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: no longer in the source branch.
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 (community) 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
=== modified file 'innobackupex'
--- innobackupex 2013-04-03 07:04:25 +0000
+++ innobackupex 2013-04-17 12:03:25 +0000
@@ -225,10 +225,12 @@
225my $xtrabackup_binary_file = 'xtrabackup_binary';225my $xtrabackup_binary_file = 'xtrabackup_binary';
226my $xtrabackup_pid_file = 'xtrabackup_pid';226my $xtrabackup_pid_file = 'xtrabackup_pid';
227my %rsync_files_hash;227my %rsync_files_hash;
228my %processed_files;
228229
229my $copy_dir_src;230my $copy_dir_src;
230my $copy_dir_dst;231my $copy_dir_dst;
231my $copy_dir_exclude_regexp;232my $cleanup_dir_dst;
233my $process_dir_exclude_regexp;
232my $copy_dir_overwrite;234my $copy_dir_overwrite;
233235
234######################################################################236######################################################################
@@ -619,7 +621,7 @@
619sub process_file {621sub process_file {
620 my $process_func = shift;622 my $process_func = shift;
621623
622 if (/$copy_dir_exclude_regexp/) {624 if (/$process_dir_exclude_regexp/) {
623 return;625 return;
624 }626 }
625 (my $dst_path = $File::Find::name) =~ s/^$copy_dir_src/$copy_dir_dst/;627 (my $dst_path = $File::Find::name) =~ s/^$copy_dir_src/$copy_dir_dst/;
@@ -639,6 +641,8 @@
639 }641 }
640642
641 &$process_func($File::Find::name, $dst_path);643 &$process_func($File::Find::name, $dst_path);
644 (my $rel_path = $File::Find::name) =~ s/^$copy_dir_src//;
645 $processed_files{$rel_path} = 1;
642 }646 }
643}647}
644648
@@ -657,9 +661,30 @@
657}661}
658662
659#663#
664# find() callback to remove files
665#
666sub remove_file_callback {
667 if (/$process_dir_exclude_regexp/) {
668 return;
669 }
670 if (-d "$File::Find::name") {
671 return;
672 }
673 (my $rel_path = $File::Find::name) =~ s/^$cleanup_dir_dst//;
674 if (!($rel_path =~ m/\//)) {
675 return;
676 }
677 if (! exists $processed_files{$rel_path}) {
678 unlink($File::Find::name) or
679 Die("Cannot remove file $File::Find::name");
680 }
681}
682
683#
660# copy_dir_recursively subroutine does a recursive copy of a specified684# copy_dir_recursively subroutine does a recursive copy of a specified
661# directory excluding files matching a specifies regexp. If $overwrite685# directory excluding files matching a specifies regexp. If $overwrite
662# is 1, it overwrites the existing files.686# is 1, it overwrites the existing files. It also updates processed_files
687# hash with the relative paths of files been copied.
663#688#
664# SYNOPSIS 689# SYNOPSIS
665#690#
@@ -674,9 +699,10 @@
674 # Clean paths and remove trailing slashes if any699 # Clean paths and remove trailing slashes if any
675 $copy_dir_src = File::Spec->canonpath(shift);700 $copy_dir_src = File::Spec->canonpath(shift);
676 $copy_dir_dst = File::Spec->canonpath(shift);701 $copy_dir_dst = File::Spec->canonpath(shift);
677 $copy_dir_exclude_regexp = shift;702 $process_dir_exclude_regexp = shift;
678 $copy_dir_overwrite = shift;703 $copy_dir_overwrite = shift;
679704
705 undef %processed_files;
680 find(\&copy_file_callback, $copy_dir_src);706 find(\&copy_file_callback, $copy_dir_src);
681}707}
682708
@@ -696,13 +722,50 @@
696 # Clean paths and remove trailing slashes if any722 # Clean paths and remove trailing slashes if any
697 $copy_dir_src = File::Spec->canonpath(shift);723 $copy_dir_src = File::Spec->canonpath(shift);
698 $copy_dir_dst = File::Spec->canonpath(shift);724 $copy_dir_dst = File::Spec->canonpath(shift);
699 $copy_dir_exclude_regexp = shift;725 $process_dir_exclude_regexp = shift;
700 $copy_dir_overwrite = shift;726 $copy_dir_overwrite = shift;
701727
728 undef %processed_files;
702 find(\&move_file_callback, $copy_dir_src);729 find(\&move_file_callback, $copy_dir_src);
703}730}
704731
705#732#
733# cleanup_dir_recursively subroutine removes files from directory
734# excluding files matching a specifies regexp and files listed in
735# processed_files.
736#
737# SYNOPSIS
738#
739# cleanup_dir_recursively($dir, $exclude_regexp);
740#
741sub cleanup_dir_recursively {
742 # Clean paths and remove trailing slashes if any
743 $cleanup_dir_dst = File::Spec->canonpath(shift);
744 $process_dir_exclude_regexp = shift;
745 find(\&remove_file_callback, $cleanup_dir_dst);
746}
747
748#
749# parse_innodb_data_file_path parse innodb_data_file_path and returns
750# it components as array of hash refs. Each hash ref has two components
751# the one named 'path' is the data file path as specified in the
752# innodb-data-file-path, second one named 'filename' is the data file name
753#
754sub parse_innodb_data_file_path {
755 my $innodb_data_file_path = shift;
756 my @data_file_paths = ();
757
758 foreach my $data_file (split(/;/, $innodb_data_file_path)) {
759 my $data_file_path = (split(/:/,$data_file))[0];
760 my $data_file_name = (split(/\/+/, $data_file_path))[-1];
761 push(@data_file_paths, {path => $data_file_path,
762 filename => $data_file_name});
763 }
764
765 return @data_file_paths;
766}
767
768#
706# copy_back subroutine copies data and index files from backup directory 769# copy_back subroutine copies data and index files from backup directory
707# back to their original locations.770# back to their original locations.
708#771#
@@ -760,24 +823,26 @@
760823
761 # make a list of all ibdata files in the backup directory and all824 # make a list of all ibdata files in the backup directory and all
762 # directories in the backup directory under which there are ibdata files825 # directories in the backup directory under which there are ibdata files
763 foreach my $a (split(/;/, $orig_innodb_data_file_path)) {826 foreach my $c (parse_innodb_data_file_path($orig_innodb_data_file_path)) {
764 my $path = (split(/:/,$a))[0];
765 my $filename = (split(/\/+/, $path))[-1];
766827
767 # check that the backup data file exists828 # check that the backup data file exists
768 if (! -e "$backup_dir/$filename") {829 if (! -e "$backup_dir/$c->{filename}") {
769 if (-e "$backup_dir/${filename}.ibz") {830 if (-e "$backup_dir/$c->{filename}.ibz") {
770 Die "Backup data file '$backup_dir/$filename' does not exist, but "831 Die "Backup data file '$backup_dir/$c->{filename}' "
771 . "its compressed copy '${path}.ibz' exists. Check "832 . "does not exist, but "
772 . "that you have run '$innobackup_script --apply-log --uncompress "833 . "its compressed copy '$c->{path}.ibz' exists. Check "
773 . "...' before attempting '$innobackup_script --copy-back ...' "834 . "that you have run "
835 . "'$innobackup_script --apply-log --uncompress "
836 . "...' before attempting "
837 . "'$innobackup_script --copy-back ...' "
774 . "or '$innobackup_script --move-back ...' !";838 . "or '$innobackup_script --move-back ...' !";
775 } else {839 } else {
776 Die "Backup data file '$backup_dir/$filename' does not exist.";840 Die "Backup data file '$backup_dir/$c->{filename}' "
841 . "does not exist.";
777 }842 }
778 }843 }
779 844
780 $excluded_files .= "|\Q$filename\E";845 $excluded_files .= "|\Q$c->{filename}\E";
781 }846 }
782847
783 # copy files from backup dir to their original locations848 # copy files from backup dir to their original locations
@@ -792,12 +857,10 @@
792 print STDERR "\n$prefix Starting to $operation InnoDB system tablespace\n";857 print STDERR "\n$prefix Starting to $operation InnoDB system tablespace\n";
793 print STDERR "$prefix in '$backup_dir'\n";858 print STDERR "$prefix in '$backup_dir'\n";
794 print STDERR "$prefix back to original InnoDB data directory '$orig_ibdata_dir'\n";859 print STDERR "$prefix back to original InnoDB data directory '$orig_ibdata_dir'\n";
795 foreach my $a (split(/;/, $orig_innodb_data_file_path)) {860 foreach my $c (parse_innodb_data_file_path($orig_innodb_data_file_path)) {
796 # get the relative pathname of a data file861 # get the relative pathname of a data file
797 my $path = (split(/:/,$a))[0];862 $src_name = escape_path("$backup_dir/$c->{filename}");
798 my $filename = (split(/\/+/, $path))[-1];863 $dst_name = escape_path("$orig_ibdata_dir/$c->{path}");
799 $src_name = escape_path("$backup_dir/$filename");
800 $dst_name = escape_path("$orig_ibdata_dir/$path");
801 &$move_or_copy_file($src_name, $dst_name);864 &$move_or_copy_file($src_name, $dst_name);
802 }865 }
803866
@@ -866,6 +929,9 @@
866 $options .= " --tmpdir=$option_tmpdir";929 $options .= " --tmpdir=$option_tmpdir";
867 }930 }
868931
932 my $innodb_data_file_path =
933 get_option(\%config, $option_defaults_group, 'innodb_data_file_path');
934
869 # run ibbackup as a child process935 # run ibbackup as a child process
870 $cmdline = "$option_ibbackup_binary $options";936 $cmdline = "$option_ibbackup_binary $options";
871 $now = current_time();937 $now = current_time();
@@ -894,23 +960,16 @@
894 if ( $option_incremental_dir ) {960 if ( $option_incremental_dir ) {
895 print STDERR "$prefix Starting to copy non-InnoDB files in '$option_incremental_dir'\n"; 961 print STDERR "$prefix Starting to copy non-InnoDB files in '$option_incremental_dir'\n";
896 print STDERR "$prefix to the full backup directory '$backup_dir'\n";962 print STDERR "$prefix to the full backup directory '$backup_dir'\n";
897 copy_dir_recursively($option_incremental_dir, $backup_dir,963 my @skip_list = ('\.\.?','backup-my\.cnf','xtrabackup_logfile',
898 '^(\.\.?|backup-my\.cnf|xtrabackup_logfile|' .964 'xtrabackup_binary','xtrabackup_checkpoints',
899 'xtrabackup_binary|xtrabackup_checkpoints|' .965 '.*\.delta','.*\.meta','ib_logfile.*');
900 '.*\.delta|.*\.meta|ib_logfile.*)$', 1);966 copy_dir_recursively($option_incremental_dir, $backup_dir,
901 # If the latest backup has no file, we need to remove the old967 '^(' . join('|', @skip_list) . ')$', 1);
902 # xtrabackup_slave_info file, because it is out of date968 foreach my $c (parse_innodb_data_file_path($innodb_data_file_path)) {
903 # TODO: this will not be needed when bug #856400 is fixed.969 push(@skip_list, "\Q$c->{filename}\E");
904 if ( -e "$backup_dir/xtrabackup_slave_info" ) {
905 print STDERR "\n$now $prefix No updated xtrabackup_slave_info found in incremental dir, removing stale xtrabackup_slave_info file from target dir.";
906 $cmdline = "rm $backup_dir/xtrabackup_slave_info";
907 $rcode = system("$cmdline");
908 if ($rcode) {
909 # failure
910 Die "\n$prefix Failed to remove stale xtrabackup_slave_info in $backup_dir";
911 }
912
913 }970 }
971 cleanup_dir_recursively($backup_dir,
972 '^(' . join('|', @skip_list, '.*\.ibd') . ')$');
914 }973 }
915974
916}975}
917976
=== modified file 'src/xtrabackup.cc'
--- src/xtrabackup.cc 2013-04-03 22:53:49 +0000
+++ src/xtrabackup.cc 2013-04-17 12:03:25 +0000
@@ -1334,6 +1334,8 @@
1334char *xtrabackup_tables_file = NULL;1334char *xtrabackup_tables_file = NULL;
1335hash_table_t* tables_hash;1335hash_table_t* tables_hash;
13361336
1337hash_table_t* inc_dir_tables_hash;
1338
1337struct xtrabackup_tables_struct{1339struct xtrabackup_tables_struct{
1338 char* name;1340 char* name;
1339 hash_node_t name_hash;1341 hash_node_t name_hash;
@@ -5006,7 +5008,7 @@
5006 }5008 }
50075009
5008 table = static_cast<xtrabackup_tables_t *>5010 table = static_cast<xtrabackup_tables_t *>
5009 (malloc(sizeof(xtrabackup_tables_t)5011 (ut_malloc(sizeof(xtrabackup_tables_t)
5010 + strlen(name_buf) + 1));5012 + strlen(name_buf) + 1));
5011 memset(table, '\0', sizeof(xtrabackup_tables_t) + strlen(name_buf) + 1);5013 memset(table, '\0', sizeof(xtrabackup_tables_t) + strlen(name_buf) + 1);
5012 table->name = ((char*)table) + sizeof(xtrabackup_tables_t);5014 table->name = ((char*)table) + sizeof(xtrabackup_tables_t);
@@ -5018,6 +5020,34 @@
5018 }5020 }
5019}5021}
50205022
5023static
5024void
5025xb_tables_hash_free(hash_table_t* hash)
5026{
5027 ulint i;
5028
5029 /* free the hash elements */
5030 for (i = 0; i < hash_get_n_cells(hash); i++) {
5031 xtrabackup_tables_t* table;
5032
5033 table = static_cast<xtrabackup_tables_t *>
5034 (HASH_GET_FIRST(hash, i));
5035
5036 while (table) {
5037 xtrabackup_tables_t* prev_table = table;
5038
5039 table = static_cast<xtrabackup_tables_t *>
5040 (HASH_GET_NEXT(name_hash, prev_table));
5041
5042 HASH_DELETE(xtrabackup_tables_t, name_hash, hash,
5043 ut_fold_string(prev_table->name), prev_table);
5044 ut_free(prev_table);
5045 }
5046 }
5047
5048 /* free hash */
5049 hash_table_free(hash);
5050}
50215051
5022/************************************************************************5052/************************************************************************
5023Destroy table filters for partial backup. */5053Destroy table filters for partial backup. */
@@ -5028,29 +5058,7 @@
5028{5058{
50295059
5030 if (xtrabackup_tables_file) {5060 if (xtrabackup_tables_file) {
5031 ulint i;5061 xb_tables_hash_free(tables_hash);
5032
5033 /* free the hash elements */
5034 for (i = 0; i < hash_get_n_cells(tables_hash); i++) {
5035 xtrabackup_tables_t* table;
5036
5037 table = static_cast<xtrabackup_tables_t *>
5038 (HASH_GET_FIRST(tables_hash, i));
5039
5040 while (table) {
5041 xtrabackup_tables_t* prev_table = table;
5042
5043 table = static_cast<xtrabackup_tables_t *>
5044 (HASH_GET_NEXT(name_hash, prev_table));
5045
5046 HASH_DELETE(xtrabackup_tables_t, name_hash, tables_hash,
5047 ut_fold_string(prev_table->name), prev_table);
5048 free(prev_table);
5049 }
5050 }
5051
5052 /* free tables_hash */
5053 hash_table_free(tables_hash);
5054 }5062 }
50555063
5056}5064}
@@ -6625,12 +6633,13 @@
6625 size_t real_name_len, /* out: buffer size for real_name */6633 size_t real_name_len, /* out: buffer size for real_name */
6626 ibool* success) /* out: indicates error. TRUE = success */6634 ibool* success) /* out: indicates error. TRUE = success */
6627{6635{
6628 char dest_dir[FN_REFLEN];6636 char dest_dir[FN_REFLEN];
6629 char dest_space_name[FN_REFLEN];6637 char dest_space_name[FN_REFLEN];
6630 ibool ok;6638 ibool ok;
6631 fil_space_t* fil_space;6639 fil_space_t* fil_space;
6632 os_file_t file = 0;6640 os_file_t file = 0;
6633 ulint tablespace_flags;6641 ulint tablespace_flags;
6642 xtrabackup_tables_t* table;
66346643
6635 ut_a(dbname != NULL ||6644 ut_a(dbname != NULL ||
6636 trx_sys_sys_space(space_id) ||6645 trx_sys_sys_space(space_id) ||
@@ -6669,6 +6678,16 @@
6669 goto found;6678 goto found;
6670 }6679 }
66716680
6681 /* remember space name for further reference */
6682 table = static_cast<xtrabackup_tables_t *>
6683 (ut_malloc(sizeof(xtrabackup_tables_t) +
6684 strlen(dest_space_name) + 1));
6685
6686 table->name = ((char*)table) + sizeof(xtrabackup_tables_t);
6687 strcpy(table->name, dest_space_name);
6688 HASH_INSERT(xtrabackup_tables_t, name_hash, inc_dir_tables_hash,
6689 ut_fold_string(table->name), table);
6690
6672 mutex_enter(&fil_system->mutex);6691 mutex_enter(&fil_system->mutex);
6673 fil_space = xb_space_get_by_name(dest_space_name);6692 fil_space = xb_space_get_by_name(dest_space_name);
6674 mutex_exit(&fil_system->mutex);6693 mutex_exit(&fil_system->mutex);
@@ -6792,7 +6811,7 @@
6792 const char* dbname, /* in: database name (ibdata: NULL) */6811 const char* dbname, /* in: database name (ibdata: NULL) */
6793 const char* filename, /* in: file name (not a path),6812 const char* filename, /* in: file name (not a path),
6794 including the .delta extension */6813 including the .delta extension */
6795 my_bool check_newer __attribute__((unused)))6814 void* /*data*/)
6796{6815{
6797 os_file_t src_file = XB_FILE_UNDEFINED;6816 os_file_t src_file = XB_FILE_UNDEFINED;
6798 os_file_t dst_file = XB_FILE_UNDEFINED;6817 os_file_t dst_file = XB_FILE_UNDEFINED;
@@ -6989,11 +7008,61 @@
6989}7008}
69907009
6991/************************************************************************7010/************************************************************************
6992Applies all .delta files from incremental_dir to the full backup.7011Callback to handle datadir entry. Function of this type will be called
6993@return TRUE on success. */7012for each entry which matches the mask by xb_process_datadir.
6994static7013@return should return TRUE on success */
6995ibool7014typedef ibool (*handle_datadir_entry_func_t)(
6996xtrabackup_apply_deltas(my_bool check_newer)7015/*=========================================*/
7016 const char* data_home_dir, /*!<in: path to datadir */
7017 const char* db_name, /*!<in: database name */
7018 const char* file_name, /*!<in: file name with suffix */
7019 void* arg); /*!<in: caller-provided data */
7020
7021/************************************************************************
7022Callback to handle datadir entry. Deletes entry if it has no matching
7023fil_space in fil_system directory.
7024@return FALSE if delete attempt was unsuccessful */
7025static
7026ibool
7027rm_if_not_found(
7028 const char* data_home_dir, /*!<in: path to datadir */
7029 const char* db_name, /*!<in: database name */
7030 const char* file_name, /*!<in: file name with suffix */
7031 void* arg __attribute__((unused)))
7032{
7033 char name[FN_REFLEN];
7034 xtrabackup_tables_t* table;
7035
7036 snprintf(name, FN_REFLEN, "%s%s/%s",
7037 xb_dict_prefix, db_name, file_name);
7038 name[strlen(name) - XB_DICT_SUFFIX_LEN] = '\0';
7039
7040 XB_HASH_SEARCH(name_hash, inc_dir_tables_hash, ut_fold_string(name),
7041 table, (void) 0,
7042 !strcmp(table->name, name));
7043
7044 if (!table) {
7045 snprintf(name, FN_REFLEN, "%s/%s/%s", data_home_dir,
7046 db_name, file_name);
7047 return os_file_delete(name);
7048 }
7049
7050 return(TRUE);
7051}
7052
7053/************************************************************************
7054Function enumerates files in datadir (provided by path) which are matched
7055by provided suffix. For each entry callback is called.
7056@return FALSE if callback for some entry returned FALSE */
7057static
7058ibool
7059xb_process_datadir(
7060 const char* path, /*!<in: datadir path */
7061 const char* suffix, /*!<in: suffix to match
7062 against */
7063 handle_datadir_entry_func_t func, /*!<in: callback */
7064 void* data) /*!<in: additional argument for
7065 callback */
6997{7066{
6998 ulint ret;7067 ulint ret;
6999 char dbpath[FN_REFLEN];7068 char dbpath[FN_REFLEN];
@@ -7001,56 +7070,59 @@
7001 os_file_dir_t dbdir;7070 os_file_dir_t dbdir;
7002 os_file_stat_t dbinfo;7071 os_file_stat_t dbinfo;
7003 os_file_stat_t fileinfo;7072 os_file_stat_t fileinfo;
7004 dberr_t err = DB_SUCCESS;7073 ulint suffix_len;
7074 dberr_t err = DB_SUCCESS;
7005 static char current_dir[2];7075 static char current_dir[2];
70067076
7007 current_dir[0] = FN_CURLIB;7077 current_dir[0] = FN_CURLIB;
7008 current_dir[1] = 0;7078 current_dir[1] = 0;
7009 srv_data_home = current_dir;7079 srv_data_home = current_dir;
70107080
7081 suffix_len = strlen(suffix);
7082
7011 /* datafile */7083 /* datafile */
7012 dbdir = os_file_opendir(xtrabackup_incremental_dir, FALSE);7084 dbdir = os_file_opendir(path, FALSE);
70137085
7014 if (dbdir != NULL) {7086 if (dbdir != NULL) {
7015 ret = fil_file_readdir_next_file(&err, xtrabackup_incremental_dir, dbdir,7087 ret = fil_file_readdir_next_file(&err, path, dbdir,
7016 &fileinfo);7088 &fileinfo);
7017 while (ret == 0) {7089 while (ret == 0) {
7018 if (fileinfo.type == OS_FILE_TYPE_DIR) {7090 if (fileinfo.type == OS_FILE_TYPE_DIR) {
7019 goto next_file_item_1;7091 goto next_file_item_1;
7020 }7092 }
70217093
7022 if (strlen(fileinfo.name) > 67094 if (strlen(fileinfo.name) > suffix_len
7023 && 0 == strcmp(fileinfo.name + 7095 && 0 == strcmp(fileinfo.name +
7024 strlen(fileinfo.name) - 6,7096 strlen(fileinfo.name) - suffix_len,
7025 ".delta")) {7097 suffix)) {
7026 if (!xtrabackup_apply_delta(7098 if (!func(
7027 xtrabackup_incremental_dir, NULL,7099 path, NULL,
7028 fileinfo.name, check_newer))7100 fileinfo.name, data))
7029 {7101 {
7030 return FALSE;7102 return(FALSE);
7031 }7103 }
7032 }7104 }
7033next_file_item_1:7105next_file_item_1:
7034 ret = fil_file_readdir_next_file(&err,7106 ret = fil_file_readdir_next_file(&err,
7035 xtrabackup_incremental_dir, dbdir,7107 path, dbdir,
7036 &fileinfo);7108 &fileinfo);
7037 }7109 }
70387110
7039 os_file_closedir(dbdir);7111 os_file_closedir(dbdir);
7040 } else {7112 } else {
7041 msg("xtrabackup: Cannot open dir %s\n",7113 msg("xtrabackup: Cannot open dir %s\n",
7042 xtrabackup_incremental_dir);7114 path);
7043 }7115 }
70447116
7045 /* single table tablespaces */7117 /* single table tablespaces */
7046 dir = os_file_opendir(xtrabackup_incremental_dir, FALSE);7118 dir = os_file_opendir(path, FALSE);
70477119
7048 if (dir == NULL) {7120 if (dir == NULL) {
7049 msg("xtrabackup: Cannot open dir %s\n",7121 msg("xtrabackup: Cannot open dir %s\n",
7050 xtrabackup_incremental_dir);7122 path);
7051 }7123 }
70527124
7053 ret = fil_file_readdir_next_file(&err, xtrabackup_incremental_dir, dir,7125 ret = fil_file_readdir_next_file(&err, path, dir,
7054 &dbinfo);7126 &dbinfo);
7055 while (ret == 0) {7127 while (ret == 0) {
7056 if (dbinfo.type == OS_FILE_TYPE_FILE7128 if (dbinfo.type == OS_FILE_TYPE_FILE
@@ -7059,7 +7131,7 @@
7059 goto next_datadir_item;7131 goto next_datadir_item;
7060 }7132 }
70617133
7062 sprintf(dbpath, "%s/%s", xtrabackup_incremental_dir,7134 sprintf(dbpath, "%s/%s", path,
7063 dbinfo.name);7135 dbinfo.name);
7064 srv_normalize_path_for_win(dbpath);7136 srv_normalize_path_for_win(dbpath);
70657137
@@ -7076,18 +7148,19 @@
7076 goto next_file_item_2;7148 goto next_file_item_2;
7077 }7149 }
70787150
7079 if (strlen(fileinfo.name) > 67151 if (strlen(fileinfo.name) > suffix_len
7080 && 0 == strcmp(fileinfo.name + 7152 && 0 == strcmp(fileinfo.name +
7081 strlen(fileinfo.name) - 6,7153 strlen(fileinfo.name) -
7082 ".delta")) {7154 suffix_len,
7083 /* The name ends in .delta; try opening7155 suffix)) {
7156 /* The name ends in suffix; process
7084 the file */7157 the file */
7085 if (!xtrabackup_apply_delta(7158 if (!func(
7086 xtrabackup_incremental_dir,7159 path,
7087 dbinfo.name,7160 dbinfo.name,
7088 fileinfo.name, check_newer))7161 fileinfo.name, data))
7089 {7162 {
7090 return FALSE;7163 return(FALSE);
7091 }7164 }
7092 }7165 }
7093next_file_item_2:7166next_file_item_2:
@@ -7100,13 +7173,24 @@
7100 }7173 }
7101next_datadir_item:7174next_datadir_item:
7102 ret = fil_file_readdir_next_file(&err,7175 ret = fil_file_readdir_next_file(&err,
7103 xtrabackup_incremental_dir,7176 path,
7104 dir, &dbinfo);7177 dir, &dbinfo);
7105 }7178 }
71067179
7107 os_file_closedir(dir);7180 os_file_closedir(dir);
71087181
7109 return TRUE;7182 return(TRUE);
7183}
7184
7185/************************************************************************
7186Applies all .delta files from incremental_dir to the full backup.
7187@return TRUE on success. */
7188static
7189ibool
7190xtrabackup_apply_deltas()
7191{
7192 return xb_process_datadir(xtrabackup_incremental_dir, ".delta",
7193 xtrabackup_apply_delta, NULL);
7110}7194}
71117195
7112static my_bool7196static my_bool
@@ -7289,12 +7373,22 @@
7289 goto error;7373 goto error;
7290 }7374 }
72917375
7292 if(!xtrabackup_apply_deltas(TRUE)) {7376 inc_dir_tables_hash = hash_create(1000);
7377
7378 if(!xtrabackup_apply_deltas()) {
7293 xb_data_files_close();7379 xb_data_files_close();
7380 xb_tables_hash_free(inc_dir_tables_hash);
7294 goto error;7381 goto error;
7295 }7382 }
72967383
7297 xb_data_files_close();7384 xb_data_files_close();
7385
7386 /* Cleanup datadir from tablespaces deleted between full and
7387 incremental backups */
7388
7389 xb_process_datadir("./", ".ibd", rm_if_not_found, NULL);
7390
7391 xb_tables_hash_free(inc_dir_tables_hash);
7298 }7392 }
7299 sync_close();7393 sync_close();
7300 sync_initialized = FALSE;7394 sync_initialized = FALSE;
73017395
=== added file 'test/t/bug856400.sh'
--- test/t/bug856400.sh 1970-01-01 00:00:00 +0000
+++ test/t/bug856400.sh 2013-04-17 12:03:25 +0000
@@ -0,0 +1,120 @@
1########################################################################
2# Bug #856400: RENAME TABLE causes incremental prepare to fail
3########################################################################
4
5. inc/common.sh
6. inc/ib_part.sh
7
8start_server --innodb_file_per_table
9
10require_partitioning
11
12run_cmd $MYSQL $MYSQL_ARGS test <<EOF
13CREATE TABLE t1(a INT) ENGINE=InnoDB;
14INSERT INTO t1 VALUES (1), (2), (3);
15
16CREATE TABLE t2(a INT) ENGINE=InnoDB;
17INSERT INTO t2 VALUES (4), (5), (6);
18
19CREATE TABLE p (
20 a int
21) ENGINE=InnoDB
22PARTITION BY RANGE (a)
23(PARTITION p0 VALUES LESS THAN (100),
24 PARTITION p1 VALUES LESS THAN (200),
25 PARTITION p2 VALUES LESS THAN (300),
26 PARTITION p3 VALUES LESS THAN (400));
27
28INSERT INTO p VALUES (1), (101), (201), (301);
29
30
31CREATE TABLE isam_t1(a INT) ENGINE=MyISAM;
32INSERT INTO isam_t1 VALUES (1), (2), (3);
33
34CREATE TABLE isam_t2(a INT) ENGINE=MyISAM;
35INSERT INTO isam_t2 VALUES (4), (5), (6);
36
37CREATE TABLE isam_p (
38 a int
39) ENGINE=MyISAM
40PARTITION BY RANGE (a)
41(PARTITION p0 VALUES LESS THAN (100),
42 PARTITION p1 VALUES LESS THAN (200),
43 PARTITION p2 VALUES LESS THAN (300),
44 PARTITION p3 VALUES LESS THAN (400));
45
46INSERT INTO isam_p VALUES (1), (101), (201), (301);
47
48EOF
49
50# Full backup
51vlog "Creating full backup"
52innobackupex --no-timestamp $topdir/full
53
54vlog "Making changes"
55
56run_cmd $MYSQL $MYSQL_ARGS test <<EOF
57
58DROP TABLE t1;
59
60DROP TABLE t2;
61CREATE TABLE t2(a INT) ENGINE=InnoDB;
62INSERT INTO t2 VALUES (40), (50), (60);
63
64ALTER TABLE p DROP PARTITION p0;
65ALTER TABLE p DROP PARTITION p1;
66ALTER TABLE p ADD PARTITION (PARTITION p4 VALUES LESS THAN (500));
67ALTER TABLE p ADD PARTITION (PARTITION p5 VALUES LESS THAN (600));
68
69INSERT INTO p VALUES (401), (501);
70
71
72DROP TABLE isam_t1;
73DROP TABLE isam_t2;
74CREATE TABLE isam_t2(a INT) ENGINE=MyISAM;
75
76INSERT INTO isam_t2 VALUES (40), (50), (60);
77
78ALTER TABLE isam_p DROP PARTITION p0;
79ALTER TABLE isam_p DROP PARTITION p1;
80ALTER TABLE isam_p ADD PARTITION (PARTITION p4 VALUES LESS THAN (500));
81ALTER TABLE isam_p ADD PARTITION (PARTITION p5 VALUES LESS THAN (600));
82
83INSERT INTO isam_p VALUES (401), (501);
84
85EOF
86
87vlog "Creating incremental backup"
88
89innobackupex --incremental --no-timestamp \
90 --incremental-basedir=$topdir/full $topdir/inc
91
92vlog "Preparing backup"
93
94innobackupex --apply-log --redo-only $topdir/full
95vlog "Log applied to full backup"
96
97innobackupex --apply-log --redo-only --incremental-dir=$topdir/inc \
98 $topdir/full
99vlog "Delta applied to full backup"
100
101innobackupex --apply-log $topdir/full
102vlog "Data prepared for restore"
103
104ls -al $topdir/full/test/*
105
106# we expect to see
107# 5 InnoDB tablespaces
108count=`ls $topdir/full/test/*.ibd | wc -l`
109vlog "$count .ibd in restore, expecting 5"
110test $count -eq 5
111
112# 5 MyISAM data files
113count=`ls $topdir/full/test/*.MYD | wc -l`
114vlog "$count .MYD in restore, expecting 5"
115test $count -eq 5
116
117# and 10 tables overall
118count=`ls $topdir/full/test/*.frm | wc -l`
119vlog "$count .frm in restore, expecting 4"
120test $count -eq 4

Subscribers

People subscribed via source and target branches