Merge lp:~sergei.glushchenko/percona-xtrabackup/20-bug856400 into lp:percona-xtrabackup/2.0

Proposed by Sergei Glushchenko
Status: Merged
Approved by: Alexey Kopytov
Approved revision: 529
Merged at revision: 537
Proposed branch: lp:~sergei.glushchenko/percona-xtrabackup/20-bug856400
Merge into: lp:percona-xtrabackup/2.0
Diff against target: 717 lines (+373/-100)
3 files modified
innobackupex (+96/-37)
src/xtrabackup.cc (+157/-63)
test/t/bug856400.sh (+120/-0)
To merge this branch: bzr merge lp:~sergei.glushchenko/percona-xtrabackup/20-bug856400
Reviewer Review Type Date Requested Status
Alexey Kopytov (community) Approve
George Ormond Lorch III g2 Approve
Review via email: mp+152361@code.launchpad.net

Description of the change

Bug 856400
Remove dropped tables from a full backup when merging an incremental one
The problem is that between full and incremental backups such a DDL
operations as DROP TABLE and ALTER TABLE DROP PARTITION are possible.
Incremental backup doesn't contain dropped tablespaces, while full
backup still does.
Solution is to enumerate all tablespaces in datadir (which present on
filesystem) and drop those which doesn't present in fil_system directory.
This is done once incremental backup is applied to full backup and
prepared.
Minor code refactoring has been done to avoid code duplication for
enumerating files in data directory.

To post a comment you must log in.
Revision history for this message
Sergei Glushchenko (sergei.glushchenko) wrote :
Revision history for this message
Sergei Glushchenko (sergei.glushchenko) wrote :
Revision history for this message
George Ormond Lorch III (gl-az) wrote :

Introduced/moved code that had compilation warnings and should be addressed:

xtrabackup.cc: In function ‘ulint rm_if_not_found(const char*, const char*, const char*, void*)’:
xtrabackup.cc:6383: warning: unused variable ‘space’ [-Wunused-variable]

Other than the warning, the fix seems reasonable, I like the use of the fn pointer to try to keep the code reasonably factored.

One minor thing is that around diff line 346, if xtrabackup_apply_deltas fails, inc_dir_tables_hash is not freed.

review: Needs Fixing (g2)
Revision history for this message
Sergei Glushchenko (sergei.glushchenko) wrote :

Issue with warning has been fixed as long as memory deallocation.

http://jenkins.percona.com/view/XtraBackup/job/percona-xtrabackup-2.0-param/379/

Revision history for this message
George Ormond Lorch III (gl-az) wrote :

Nice work.

review: Approve (g2)
Revision history for this message
Alexey Kopytov (akopytov) wrote :

As discussed on IRC, this also has to handle non-InnoDB files (frm, MyISAM, metainfo, etc.)

review: Needs Fixing
Revision history for this message
Sergei Glushchenko (sergei.glushchenko) wrote :
Revision history for this message
Alexey Kopytov (akopytov) wrote :

Sergei,

Some comments and questions:
  - conflict in xtrabackup.cc
  - typo in the comments for cleanup_dir_recursively() (double "directory")
  - wrong bug reference in the test case comments
  - there's no need for "\/+" in the following code:

:+ (my $rel_path = $File::Find::name) =~ s/^$copy_dir_src\/+//;

  - can you clarify the purpose of the code in lines 50-52?
  - error handling for unlink() would be nice
  - did you take the comment in lines 125-128 into account?
  - a few lines are longer than 80 chars, try:
: bzr diff -c-1 | grep -0 '^+' | expand | awk 'length > 80'
    or better yet, try to use a good text editor that can be configured
    to highlight such lines
  - the check_newer argument is not used. does it really make sense to
    add some _new_ code to support it? Wouldn't it be better to get rid
    of it?
  - in the test, do we really have to restart the server between full
    and incremental backup, or it's just a copy-paste from another test?

review: Needs Fixing
Revision history for this message
Sergei Glushchenko (sergei.glushchenko) wrote :

Hi Alexey,

* changes
  - rebased on latest trunk
  - typo in cleanup_dir_recursively fixed
  - bug reference in the test case comments fixed
  - added error handling for unlink
  - removed restart of server in testcase
  - parse innodb_data_file_path to avoid copying of
    system tablespace
* other things
  - there was a need for "\/+"
    because otherwise lines 50-52 wont give expected result
  - lines 50-52 were needed to restrict copying of contents of
    top level of datadir; now it is not needed as I start using
    value of innodb_data_file_path to prevent copying of
    system tablespace
  - I will never configure my text editor to highlight long lines, thanks;
    this is not a good place to discuss text editors and their settings
  - I think to have a possibility to pass user argument to callback
    is always a good idea, no matter whether this needed at the
    moment or not; but I changed the code to get rid of check_newer
* Jenkins
  - http://jenkins.percona.com/view/XtraBackup/job/percona-xtrabackup-2.0-param/393/

Thanks,
Sergei

Revision history for this message
Alexey Kopytov (akopytov) wrote :

Sergei,

  - I was referring to the check_newer argument of
    xtrabackup_apply_deltas(). It's not user provided data, and it's
    unused. It's good that you changed the argument to
    xtrabackup_apply_delta() from my_bool to 'void*' as it's less
    confusing this way. Please also remove the argument of
    xtrabackup_apply_deltas(), as it's rather confusing to see
    something like "xtrabackup_apply_deltas(TRUE)" and then discover
    that TRUE is there just in case.

  - I don't understand this code around parsing
    innodb_data_file_path. So you want to exclude system tablespace
    files from cleaning up. You loop through paths returned by
    parse_innodb_data_file_path(), replace multiple slashes with a
    single one and add them to skip_list. Don't you want to add
    filenames instead of paths? And what's the goal of removing
    multiple slashes again?

  - I also see you replaced "strcpy(table->name, dest_space_name);"
    with "sprintf(table->name, dest_space_name)". Please revert that,
    you don't really need sprintf() there.

review: Needs Fixing
Revision history for this message
Sergei Glushchenko (sergei.glushchenko) wrote :

Alexey,

Indeed, I forgot to cleanup things like xtrabackup_apply_deltas(TRUE).

What if I have MyISAM table named test_table and innodb_data_file_path=test_table.MYD:10M:autoextend? In this case incorrect backup will be taken. In fact I think one should also add "./" to innodb_data_file_path component to avoid disambiguations. So it can be considered as an attempt to workaround (now officially reported) bug https://bugs.launchpad.net/percona-xtrabackup/+bug/1169726.

s/strcpy/sprintf/ is really an accident, sorry :)

Revision history for this message
Sergei Glushchenko (sergei.glushchenko) wrote :

However for this specific case using just a filename from innodb_data_file_path will not lead to catastrophe. It can lead only to some files will not be cleaned up.

If our tablespace is named say xtrabackup_tmp#1234.idb, and we removed tablespace with ID 1234 between full and incremental backup, the tmp# tablespace will remain in full backup directory. It will not be copied back on restore however because of https://bugs.launchpad.net/percona-xtrabackup/+bug/1169726. That's when two bugs eventually give almost correct result.

The reason why I started to think in that side was just a discomfort when I write a code that decide whether to remove file or not by using just it's name instead of fully qualified path.

Tree is updated, jenkins run is started, but supposedly it will be finished by tomorrow
http://jenkins.percona.com/view/XtraBackup/job/percona-xtrabackup-2.0-param/399/

Revision history for this message
Alexey Kopytov (akopytov) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'innobackupex'
2--- innobackupex 2013-04-03 07:04:25 +0000
3+++ innobackupex 2013-04-17 12:03:25 +0000
4@@ -225,10 +225,12 @@
5 my $xtrabackup_binary_file = 'xtrabackup_binary';
6 my $xtrabackup_pid_file = 'xtrabackup_pid';
7 my %rsync_files_hash;
8+my %processed_files;
9
10 my $copy_dir_src;
11 my $copy_dir_dst;
12-my $copy_dir_exclude_regexp;
13+my $cleanup_dir_dst;
14+my $process_dir_exclude_regexp;
15 my $copy_dir_overwrite;
16
17 ######################################################################
18@@ -619,7 +621,7 @@
19 sub process_file {
20 my $process_func = shift;
21
22- if (/$copy_dir_exclude_regexp/) {
23+ if (/$process_dir_exclude_regexp/) {
24 return;
25 }
26 (my $dst_path = $File::Find::name) =~ s/^$copy_dir_src/$copy_dir_dst/;
27@@ -639,6 +641,8 @@
28 }
29
30 &$process_func($File::Find::name, $dst_path);
31+ (my $rel_path = $File::Find::name) =~ s/^$copy_dir_src//;
32+ $processed_files{$rel_path} = 1;
33 }
34 }
35
36@@ -657,9 +661,30 @@
37 }
38
39 #
40+# find() callback to remove files
41+#
42+sub remove_file_callback {
43+ if (/$process_dir_exclude_regexp/) {
44+ return;
45+ }
46+ if (-d "$File::Find::name") {
47+ return;
48+ }
49+ (my $rel_path = $File::Find::name) =~ s/^$cleanup_dir_dst//;
50+ if (!($rel_path =~ m/\//)) {
51+ return;
52+ }
53+ if (! exists $processed_files{$rel_path}) {
54+ unlink($File::Find::name) or
55+ Die("Cannot remove file $File::Find::name");
56+ }
57+}
58+
59+#
60 # copy_dir_recursively subroutine does a recursive copy of a specified
61 # directory excluding files matching a specifies regexp. If $overwrite
62-# is 1, it overwrites the existing files.
63+# is 1, it overwrites the existing files. It also updates processed_files
64+# hash with the relative paths of files been copied.
65 #
66 # SYNOPSIS
67 #
68@@ -674,9 +699,10 @@
69 # Clean paths and remove trailing slashes if any
70 $copy_dir_src = File::Spec->canonpath(shift);
71 $copy_dir_dst = File::Spec->canonpath(shift);
72- $copy_dir_exclude_regexp = shift;
73+ $process_dir_exclude_regexp = shift;
74 $copy_dir_overwrite = shift;
75
76+ undef %processed_files;
77 find(\&copy_file_callback, $copy_dir_src);
78 }
79
80@@ -696,13 +722,50 @@
81 # Clean paths and remove trailing slashes if any
82 $copy_dir_src = File::Spec->canonpath(shift);
83 $copy_dir_dst = File::Spec->canonpath(shift);
84- $copy_dir_exclude_regexp = shift;
85+ $process_dir_exclude_regexp = shift;
86 $copy_dir_overwrite = shift;
87
88+ undef %processed_files;
89 find(\&move_file_callback, $copy_dir_src);
90 }
91
92 #
93+# cleanup_dir_recursively subroutine removes files from directory
94+# excluding files matching a specifies regexp and files listed in
95+# processed_files.
96+#
97+# SYNOPSIS
98+#
99+# cleanup_dir_recursively($dir, $exclude_regexp);
100+#
101+sub cleanup_dir_recursively {
102+ # Clean paths and remove trailing slashes if any
103+ $cleanup_dir_dst = File::Spec->canonpath(shift);
104+ $process_dir_exclude_regexp = shift;
105+ find(\&remove_file_callback, $cleanup_dir_dst);
106+}
107+
108+#
109+# parse_innodb_data_file_path parse innodb_data_file_path and returns
110+# it components as array of hash refs. Each hash ref has two components
111+# the one named 'path' is the data file path as specified in the
112+# innodb-data-file-path, second one named 'filename' is the data file name
113+#
114+sub parse_innodb_data_file_path {
115+ my $innodb_data_file_path = shift;
116+ my @data_file_paths = ();
117+
118+ foreach my $data_file (split(/;/, $innodb_data_file_path)) {
119+ my $data_file_path = (split(/:/,$data_file))[0];
120+ my $data_file_name = (split(/\/+/, $data_file_path))[-1];
121+ push(@data_file_paths, {path => $data_file_path,
122+ filename => $data_file_name});
123+ }
124+
125+ return @data_file_paths;
126+}
127+
128+#
129 # copy_back subroutine copies data and index files from backup directory
130 # back to their original locations.
131 #
132@@ -760,24 +823,26 @@
133
134 # make a list of all ibdata files in the backup directory and all
135 # directories in the backup directory under which there are ibdata files
136- foreach my $a (split(/;/, $orig_innodb_data_file_path)) {
137- my $path = (split(/:/,$a))[0];
138- my $filename = (split(/\/+/, $path))[-1];
139+ foreach my $c (parse_innodb_data_file_path($orig_innodb_data_file_path)) {
140
141 # check that the backup data file exists
142- if (! -e "$backup_dir/$filename") {
143- if (-e "$backup_dir/${filename}.ibz") {
144- Die "Backup data file '$backup_dir/$filename' does not exist, but "
145- . "its compressed copy '${path}.ibz' exists. Check "
146- . "that you have run '$innobackup_script --apply-log --uncompress "
147- . "...' before attempting '$innobackup_script --copy-back ...' "
148+ if (! -e "$backup_dir/$c->{filename}") {
149+ if (-e "$backup_dir/$c->{filename}.ibz") {
150+ Die "Backup data file '$backup_dir/$c->{filename}' "
151+ . "does not exist, but "
152+ . "its compressed copy '$c->{path}.ibz' exists. Check "
153+ . "that you have run "
154+ . "'$innobackup_script --apply-log --uncompress "
155+ . "...' before attempting "
156+ . "'$innobackup_script --copy-back ...' "
157 . "or '$innobackup_script --move-back ...' !";
158 } else {
159- Die "Backup data file '$backup_dir/$filename' does not exist.";
160+ Die "Backup data file '$backup_dir/$c->{filename}' "
161+ . "does not exist.";
162 }
163 }
164
165- $excluded_files .= "|\Q$filename\E";
166+ $excluded_files .= "|\Q$c->{filename}\E";
167 }
168
169 # copy files from backup dir to their original locations
170@@ -792,12 +857,10 @@
171 print STDERR "\n$prefix Starting to $operation InnoDB system tablespace\n";
172 print STDERR "$prefix in '$backup_dir'\n";
173 print STDERR "$prefix back to original InnoDB data directory '$orig_ibdata_dir'\n";
174- foreach my $a (split(/;/, $orig_innodb_data_file_path)) {
175+ foreach my $c (parse_innodb_data_file_path($orig_innodb_data_file_path)) {
176 # get the relative pathname of a data file
177- my $path = (split(/:/,$a))[0];
178- my $filename = (split(/\/+/, $path))[-1];
179- $src_name = escape_path("$backup_dir/$filename");
180- $dst_name = escape_path("$orig_ibdata_dir/$path");
181+ $src_name = escape_path("$backup_dir/$c->{filename}");
182+ $dst_name = escape_path("$orig_ibdata_dir/$c->{path}");
183 &$move_or_copy_file($src_name, $dst_name);
184 }
185
186@@ -866,6 +929,9 @@
187 $options .= " --tmpdir=$option_tmpdir";
188 }
189
190+ my $innodb_data_file_path =
191+ get_option(\%config, $option_defaults_group, 'innodb_data_file_path');
192+
193 # run ibbackup as a child process
194 $cmdline = "$option_ibbackup_binary $options";
195 $now = current_time();
196@@ -894,23 +960,16 @@
197 if ( $option_incremental_dir ) {
198 print STDERR "$prefix Starting to copy non-InnoDB files in '$option_incremental_dir'\n";
199 print STDERR "$prefix to the full backup directory '$backup_dir'\n";
200- copy_dir_recursively($option_incremental_dir, $backup_dir,
201- '^(\.\.?|backup-my\.cnf|xtrabackup_logfile|' .
202- 'xtrabackup_binary|xtrabackup_checkpoints|' .
203- '.*\.delta|.*\.meta|ib_logfile.*)$', 1);
204- # If the latest backup has no file, we need to remove the old
205- # xtrabackup_slave_info file, because it is out of date
206- # TODO: this will not be needed when bug #856400 is fixed.
207- if ( -e "$backup_dir/xtrabackup_slave_info" ) {
208- print STDERR "\n$now $prefix No updated xtrabackup_slave_info found in incremental dir, removing stale xtrabackup_slave_info file from target dir.";
209- $cmdline = "rm $backup_dir/xtrabackup_slave_info";
210- $rcode = system("$cmdline");
211- if ($rcode) {
212- # failure
213- Die "\n$prefix Failed to remove stale xtrabackup_slave_info in $backup_dir";
214- }
215-
216+ my @skip_list = ('\.\.?','backup-my\.cnf','xtrabackup_logfile',
217+ 'xtrabackup_binary','xtrabackup_checkpoints',
218+ '.*\.delta','.*\.meta','ib_logfile.*');
219+ copy_dir_recursively($option_incremental_dir, $backup_dir,
220+ '^(' . join('|', @skip_list) . ')$', 1);
221+ foreach my $c (parse_innodb_data_file_path($innodb_data_file_path)) {
222+ push(@skip_list, "\Q$c->{filename}\E");
223 }
224+ cleanup_dir_recursively($backup_dir,
225+ '^(' . join('|', @skip_list, '.*\.ibd') . ')$');
226 }
227
228 }
229
230=== modified file 'src/xtrabackup.cc'
231--- src/xtrabackup.cc 2013-04-03 22:53:49 +0000
232+++ src/xtrabackup.cc 2013-04-17 12:03:25 +0000
233@@ -1334,6 +1334,8 @@
234 char *xtrabackup_tables_file = NULL;
235 hash_table_t* tables_hash;
236
237+hash_table_t* inc_dir_tables_hash;
238+
239 struct xtrabackup_tables_struct{
240 char* name;
241 hash_node_t name_hash;
242@@ -5006,7 +5008,7 @@
243 }
244
245 table = static_cast<xtrabackup_tables_t *>
246- (malloc(sizeof(xtrabackup_tables_t)
247+ (ut_malloc(sizeof(xtrabackup_tables_t)
248 + strlen(name_buf) + 1));
249 memset(table, '\0', sizeof(xtrabackup_tables_t) + strlen(name_buf) + 1);
250 table->name = ((char*)table) + sizeof(xtrabackup_tables_t);
251@@ -5018,6 +5020,34 @@
252 }
253 }
254
255+static
256+void
257+xb_tables_hash_free(hash_table_t* hash)
258+{
259+ ulint i;
260+
261+ /* free the hash elements */
262+ for (i = 0; i < hash_get_n_cells(hash); i++) {
263+ xtrabackup_tables_t* table;
264+
265+ table = static_cast<xtrabackup_tables_t *>
266+ (HASH_GET_FIRST(hash, i));
267+
268+ while (table) {
269+ xtrabackup_tables_t* prev_table = table;
270+
271+ table = static_cast<xtrabackup_tables_t *>
272+ (HASH_GET_NEXT(name_hash, prev_table));
273+
274+ HASH_DELETE(xtrabackup_tables_t, name_hash, hash,
275+ ut_fold_string(prev_table->name), prev_table);
276+ ut_free(prev_table);
277+ }
278+ }
279+
280+ /* free hash */
281+ hash_table_free(hash);
282+}
283
284 /************************************************************************
285 Destroy table filters for partial backup. */
286@@ -5028,29 +5058,7 @@
287 {
288
289 if (xtrabackup_tables_file) {
290- ulint i;
291-
292- /* free the hash elements */
293- for (i = 0; i < hash_get_n_cells(tables_hash); i++) {
294- xtrabackup_tables_t* table;
295-
296- table = static_cast<xtrabackup_tables_t *>
297- (HASH_GET_FIRST(tables_hash, i));
298-
299- while (table) {
300- xtrabackup_tables_t* prev_table = table;
301-
302- table = static_cast<xtrabackup_tables_t *>
303- (HASH_GET_NEXT(name_hash, prev_table));
304-
305- HASH_DELETE(xtrabackup_tables_t, name_hash, tables_hash,
306- ut_fold_string(prev_table->name), prev_table);
307- free(prev_table);
308- }
309- }
310-
311- /* free tables_hash */
312- hash_table_free(tables_hash);
313+ xb_tables_hash_free(tables_hash);
314 }
315
316 }
317@@ -6625,12 +6633,13 @@
318 size_t real_name_len, /* out: buffer size for real_name */
319 ibool* success) /* out: indicates error. TRUE = success */
320 {
321- char dest_dir[FN_REFLEN];
322- char dest_space_name[FN_REFLEN];
323- ibool ok;
324- fil_space_t* fil_space;
325- os_file_t file = 0;
326- ulint tablespace_flags;
327+ char dest_dir[FN_REFLEN];
328+ char dest_space_name[FN_REFLEN];
329+ ibool ok;
330+ fil_space_t* fil_space;
331+ os_file_t file = 0;
332+ ulint tablespace_flags;
333+ xtrabackup_tables_t* table;
334
335 ut_a(dbname != NULL ||
336 trx_sys_sys_space(space_id) ||
337@@ -6669,6 +6678,16 @@
338 goto found;
339 }
340
341+ /* remember space name for further reference */
342+ table = static_cast<xtrabackup_tables_t *>
343+ (ut_malloc(sizeof(xtrabackup_tables_t) +
344+ strlen(dest_space_name) + 1));
345+
346+ table->name = ((char*)table) + sizeof(xtrabackup_tables_t);
347+ strcpy(table->name, dest_space_name);
348+ HASH_INSERT(xtrabackup_tables_t, name_hash, inc_dir_tables_hash,
349+ ut_fold_string(table->name), table);
350+
351 mutex_enter(&fil_system->mutex);
352 fil_space = xb_space_get_by_name(dest_space_name);
353 mutex_exit(&fil_system->mutex);
354@@ -6792,7 +6811,7 @@
355 const char* dbname, /* in: database name (ibdata: NULL) */
356 const char* filename, /* in: file name (not a path),
357 including the .delta extension */
358- my_bool check_newer __attribute__((unused)))
359+ void* /*data*/)
360 {
361 os_file_t src_file = XB_FILE_UNDEFINED;
362 os_file_t dst_file = XB_FILE_UNDEFINED;
363@@ -6989,11 +7008,61 @@
364 }
365
366 /************************************************************************
367-Applies all .delta files from incremental_dir to the full backup.
368-@return TRUE on success. */
369-static
370-ibool
371-xtrabackup_apply_deltas(my_bool check_newer)
372+Callback to handle datadir entry. Function of this type will be called
373+for each entry which matches the mask by xb_process_datadir.
374+@return should return TRUE on success */
375+typedef ibool (*handle_datadir_entry_func_t)(
376+/*=========================================*/
377+ const char* data_home_dir, /*!<in: path to datadir */
378+ const char* db_name, /*!<in: database name */
379+ const char* file_name, /*!<in: file name with suffix */
380+ void* arg); /*!<in: caller-provided data */
381+
382+/************************************************************************
383+Callback to handle datadir entry. Deletes entry if it has no matching
384+fil_space in fil_system directory.
385+@return FALSE if delete attempt was unsuccessful */
386+static
387+ibool
388+rm_if_not_found(
389+ const char* data_home_dir, /*!<in: path to datadir */
390+ const char* db_name, /*!<in: database name */
391+ const char* file_name, /*!<in: file name with suffix */
392+ void* arg __attribute__((unused)))
393+{
394+ char name[FN_REFLEN];
395+ xtrabackup_tables_t* table;
396+
397+ snprintf(name, FN_REFLEN, "%s%s/%s",
398+ xb_dict_prefix, db_name, file_name);
399+ name[strlen(name) - XB_DICT_SUFFIX_LEN] = '\0';
400+
401+ XB_HASH_SEARCH(name_hash, inc_dir_tables_hash, ut_fold_string(name),
402+ table, (void) 0,
403+ !strcmp(table->name, name));
404+
405+ if (!table) {
406+ snprintf(name, FN_REFLEN, "%s/%s/%s", data_home_dir,
407+ db_name, file_name);
408+ return os_file_delete(name);
409+ }
410+
411+ return(TRUE);
412+}
413+
414+/************************************************************************
415+Function enumerates files in datadir (provided by path) which are matched
416+by provided suffix. For each entry callback is called.
417+@return FALSE if callback for some entry returned FALSE */
418+static
419+ibool
420+xb_process_datadir(
421+ const char* path, /*!<in: datadir path */
422+ const char* suffix, /*!<in: suffix to match
423+ against */
424+ handle_datadir_entry_func_t func, /*!<in: callback */
425+ void* data) /*!<in: additional argument for
426+ callback */
427 {
428 ulint ret;
429 char dbpath[FN_REFLEN];
430@@ -7001,56 +7070,59 @@
431 os_file_dir_t dbdir;
432 os_file_stat_t dbinfo;
433 os_file_stat_t fileinfo;
434- dberr_t err = DB_SUCCESS;
435+ ulint suffix_len;
436+ dberr_t err = DB_SUCCESS;
437 static char current_dir[2];
438
439 current_dir[0] = FN_CURLIB;
440 current_dir[1] = 0;
441 srv_data_home = current_dir;
442
443+ suffix_len = strlen(suffix);
444+
445 /* datafile */
446- dbdir = os_file_opendir(xtrabackup_incremental_dir, FALSE);
447+ dbdir = os_file_opendir(path, FALSE);
448
449 if (dbdir != NULL) {
450- ret = fil_file_readdir_next_file(&err, xtrabackup_incremental_dir, dbdir,
451+ ret = fil_file_readdir_next_file(&err, path, dbdir,
452 &fileinfo);
453 while (ret == 0) {
454 if (fileinfo.type == OS_FILE_TYPE_DIR) {
455 goto next_file_item_1;
456 }
457
458- if (strlen(fileinfo.name) > 6
459+ if (strlen(fileinfo.name) > suffix_len
460 && 0 == strcmp(fileinfo.name +
461- strlen(fileinfo.name) - 6,
462- ".delta")) {
463- if (!xtrabackup_apply_delta(
464- xtrabackup_incremental_dir, NULL,
465- fileinfo.name, check_newer))
466+ strlen(fileinfo.name) - suffix_len,
467+ suffix)) {
468+ if (!func(
469+ path, NULL,
470+ fileinfo.name, data))
471 {
472- return FALSE;
473+ return(FALSE);
474 }
475 }
476 next_file_item_1:
477 ret = fil_file_readdir_next_file(&err,
478- xtrabackup_incremental_dir, dbdir,
479+ path, dbdir,
480 &fileinfo);
481 }
482
483 os_file_closedir(dbdir);
484 } else {
485 msg("xtrabackup: Cannot open dir %s\n",
486- xtrabackup_incremental_dir);
487+ path);
488 }
489
490 /* single table tablespaces */
491- dir = os_file_opendir(xtrabackup_incremental_dir, FALSE);
492+ dir = os_file_opendir(path, FALSE);
493
494 if (dir == NULL) {
495 msg("xtrabackup: Cannot open dir %s\n",
496- xtrabackup_incremental_dir);
497+ path);
498 }
499
500- ret = fil_file_readdir_next_file(&err, xtrabackup_incremental_dir, dir,
501+ ret = fil_file_readdir_next_file(&err, path, dir,
502 &dbinfo);
503 while (ret == 0) {
504 if (dbinfo.type == OS_FILE_TYPE_FILE
505@@ -7059,7 +7131,7 @@
506 goto next_datadir_item;
507 }
508
509- sprintf(dbpath, "%s/%s", xtrabackup_incremental_dir,
510+ sprintf(dbpath, "%s/%s", path,
511 dbinfo.name);
512 srv_normalize_path_for_win(dbpath);
513
514@@ -7076,18 +7148,19 @@
515 goto next_file_item_2;
516 }
517
518- if (strlen(fileinfo.name) > 6
519+ if (strlen(fileinfo.name) > suffix_len
520 && 0 == strcmp(fileinfo.name +
521- strlen(fileinfo.name) - 6,
522- ".delta")) {
523- /* The name ends in .delta; try opening
524+ strlen(fileinfo.name) -
525+ suffix_len,
526+ suffix)) {
527+ /* The name ends in suffix; process
528 the file */
529- if (!xtrabackup_apply_delta(
530- xtrabackup_incremental_dir,
531+ if (!func(
532+ path,
533 dbinfo.name,
534- fileinfo.name, check_newer))
535+ fileinfo.name, data))
536 {
537- return FALSE;
538+ return(FALSE);
539 }
540 }
541 next_file_item_2:
542@@ -7100,13 +7173,24 @@
543 }
544 next_datadir_item:
545 ret = fil_file_readdir_next_file(&err,
546- xtrabackup_incremental_dir,
547+ path,
548 dir, &dbinfo);
549 }
550
551 os_file_closedir(dir);
552
553- return TRUE;
554+ return(TRUE);
555+}
556+
557+/************************************************************************
558+Applies all .delta files from incremental_dir to the full backup.
559+@return TRUE on success. */
560+static
561+ibool
562+xtrabackup_apply_deltas()
563+{
564+ return xb_process_datadir(xtrabackup_incremental_dir, ".delta",
565+ xtrabackup_apply_delta, NULL);
566 }
567
568 static my_bool
569@@ -7289,12 +7373,22 @@
570 goto error;
571 }
572
573- if(!xtrabackup_apply_deltas(TRUE)) {
574+ inc_dir_tables_hash = hash_create(1000);
575+
576+ if(!xtrabackup_apply_deltas()) {
577 xb_data_files_close();
578+ xb_tables_hash_free(inc_dir_tables_hash);
579 goto error;
580 }
581
582 xb_data_files_close();
583+
584+ /* Cleanup datadir from tablespaces deleted between full and
585+ incremental backups */
586+
587+ xb_process_datadir("./", ".ibd", rm_if_not_found, NULL);
588+
589+ xb_tables_hash_free(inc_dir_tables_hash);
590 }
591 sync_close();
592 sync_initialized = FALSE;
593
594=== added file 'test/t/bug856400.sh'
595--- test/t/bug856400.sh 1970-01-01 00:00:00 +0000
596+++ test/t/bug856400.sh 2013-04-17 12:03:25 +0000
597@@ -0,0 +1,120 @@
598+########################################################################
599+# Bug #856400: RENAME TABLE causes incremental prepare to fail
600+########################################################################
601+
602+. inc/common.sh
603+. inc/ib_part.sh
604+
605+start_server --innodb_file_per_table
606+
607+require_partitioning
608+
609+run_cmd $MYSQL $MYSQL_ARGS test <<EOF
610+CREATE TABLE t1(a INT) ENGINE=InnoDB;
611+INSERT INTO t1 VALUES (1), (2), (3);
612+
613+CREATE TABLE t2(a INT) ENGINE=InnoDB;
614+INSERT INTO t2 VALUES (4), (5), (6);
615+
616+CREATE TABLE p (
617+ a int
618+) ENGINE=InnoDB
619+PARTITION BY RANGE (a)
620+(PARTITION p0 VALUES LESS THAN (100),
621+ PARTITION p1 VALUES LESS THAN (200),
622+ PARTITION p2 VALUES LESS THAN (300),
623+ PARTITION p3 VALUES LESS THAN (400));
624+
625+INSERT INTO p VALUES (1), (101), (201), (301);
626+
627+
628+CREATE TABLE isam_t1(a INT) ENGINE=MyISAM;
629+INSERT INTO isam_t1 VALUES (1), (2), (3);
630+
631+CREATE TABLE isam_t2(a INT) ENGINE=MyISAM;
632+INSERT INTO isam_t2 VALUES (4), (5), (6);
633+
634+CREATE TABLE isam_p (
635+ a int
636+) ENGINE=MyISAM
637+PARTITION BY RANGE (a)
638+(PARTITION p0 VALUES LESS THAN (100),
639+ PARTITION p1 VALUES LESS THAN (200),
640+ PARTITION p2 VALUES LESS THAN (300),
641+ PARTITION p3 VALUES LESS THAN (400));
642+
643+INSERT INTO isam_p VALUES (1), (101), (201), (301);
644+
645+EOF
646+
647+# Full backup
648+vlog "Creating full backup"
649+innobackupex --no-timestamp $topdir/full
650+
651+vlog "Making changes"
652+
653+run_cmd $MYSQL $MYSQL_ARGS test <<EOF
654+
655+DROP TABLE t1;
656+
657+DROP TABLE t2;
658+CREATE TABLE t2(a INT) ENGINE=InnoDB;
659+INSERT INTO t2 VALUES (40), (50), (60);
660+
661+ALTER TABLE p DROP PARTITION p0;
662+ALTER TABLE p DROP PARTITION p1;
663+ALTER TABLE p ADD PARTITION (PARTITION p4 VALUES LESS THAN (500));
664+ALTER TABLE p ADD PARTITION (PARTITION p5 VALUES LESS THAN (600));
665+
666+INSERT INTO p VALUES (401), (501);
667+
668+
669+DROP TABLE isam_t1;
670+DROP TABLE isam_t2;
671+CREATE TABLE isam_t2(a INT) ENGINE=MyISAM;
672+
673+INSERT INTO isam_t2 VALUES (40), (50), (60);
674+
675+ALTER TABLE isam_p DROP PARTITION p0;
676+ALTER TABLE isam_p DROP PARTITION p1;
677+ALTER TABLE isam_p ADD PARTITION (PARTITION p4 VALUES LESS THAN (500));
678+ALTER TABLE isam_p ADD PARTITION (PARTITION p5 VALUES LESS THAN (600));
679+
680+INSERT INTO isam_p VALUES (401), (501);
681+
682+EOF
683+
684+vlog "Creating incremental backup"
685+
686+innobackupex --incremental --no-timestamp \
687+ --incremental-basedir=$topdir/full $topdir/inc
688+
689+vlog "Preparing backup"
690+
691+innobackupex --apply-log --redo-only $topdir/full
692+vlog "Log applied to full backup"
693+
694+innobackupex --apply-log --redo-only --incremental-dir=$topdir/inc \
695+ $topdir/full
696+vlog "Delta applied to full backup"
697+
698+innobackupex --apply-log $topdir/full
699+vlog "Data prepared for restore"
700+
701+ls -al $topdir/full/test/*
702+
703+# we expect to see
704+# 5 InnoDB tablespaces
705+count=`ls $topdir/full/test/*.ibd | wc -l`
706+vlog "$count .ibd in restore, expecting 5"
707+test $count -eq 5
708+
709+# 5 MyISAM data files
710+count=`ls $topdir/full/test/*.MYD | wc -l`
711+vlog "$count .MYD in restore, expecting 5"
712+test $count -eq 5
713+
714+# and 10 tables overall
715+count=`ls $topdir/full/test/*.frm | wc -l`
716+vlog "$count .frm in restore, expecting 4"
717+test $count -eq 4

Subscribers

People subscribed via source and target branches