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

[Sergei Glushchenko]

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.

[Sergei Glushchenko (sergei.glushchenko)]

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

[Sergei Glushchenko (sergei.glushchenko)]

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

[George Ormond Lorch III (gl-az)]

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)

[Sergei Glushchenko (sergei.glushchenko)]

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

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

[George Ormond Lorch III (gl-az)]

Nice work.

review: Approve (g2)

[Alexey Kopytov (akopytov)]

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

review: Needs Fixing

[Sergei Glushchenko (sergei.glushchenko)]

innobackupex part implemented

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

[Alexey Kopytov (akopytov)]

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

[Sergei Glushchenko (sergei.glushchenko)]

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

[Alexey Kopytov (akopytov)]

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

[Sergei Glushchenko (sergei.glushchenko)]

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 :)

[Sergei Glushchenko (sergei.glushchenko)]

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/

[Alexey Kopytov (akopytov)]

review: Approve

=== modified file 'innobackupex'
--- innobackupex	2013-04-03 07:04:25 +0000
+++ innobackupex	2013-04-17 12:03:25 +0000
@@ -225,10 +225,12 @@
 my $xtrabackup_binary_file = 'xtrabackup_binary';
 my $xtrabackup_pid_file = 'xtrabackup_pid';
 my %rsync_files_hash;
+my %processed_files;
 
 my $copy_dir_src;
 my $copy_dir_dst;
-my $copy_dir_exclude_regexp;
+my $cleanup_dir_dst;
+my $process_dir_exclude_regexp;
 my $copy_dir_overwrite;
 
 ######################################################################
@@ -619,7 +621,7 @@
 sub process_file {
     my $process_func = shift;
 
-    if (/$copy_dir_exclude_regexp/) {
+    if (/$process_dir_exclude_regexp/) {
 	return;
     }
     (my $dst_path = $File::Find::name) =~ s/^$copy_dir_src/$copy_dir_dst/;
@@ -639,6 +641,8 @@
 	}
 
 	&$process_func($File::Find::name, $dst_path);
+        (my $rel_path = $File::Find::name) =~ s/^$copy_dir_src//;
+        $processed_files{$rel_path} = 1;
     }
 }
 
@@ -657,9 +661,30 @@
 }
 
 #
+# find() callback to remove files
+#
+sub remove_file_callback {
+    if (/$process_dir_exclude_regexp/) {
+        return;
+    }
+    if (-d "$File::Find::name") {
+        return;
+    }
+    (my $rel_path = $File::Find::name) =~ s/^$cleanup_dir_dst//;
+    if (!($rel_path =~ m/\//)) {
+        return;
+    }
+    if (! exists $processed_files{$rel_path}) {
+        unlink($File::Find::name) or
+            Die("Cannot remove file $File::Find::name");
+    }
+}
+
+#
 # copy_dir_recursively subroutine does a recursive copy of a specified
 # directory excluding files matching a specifies regexp. If $overwrite
-# is 1, it overwrites the existing files.
+# is 1, it overwrites the existing files. It also updates processed_files
+# hash with the relative paths of files been copied.
 #
 # SYNOPSIS 
 #
@@ -674,9 +699,10 @@
     # Clean paths and remove trailing slashes if any
     $copy_dir_src = File::Spec->canonpath(shift);
     $copy_dir_dst = File::Spec->canonpath(shift);
-    $copy_dir_exclude_regexp = shift;
+    $process_dir_exclude_regexp = shift;
     $copy_dir_overwrite = shift;
 
+    undef %processed_files;
     find(\&copy_file_callback, $copy_dir_src);
 }
 
@@ -696,13 +722,50 @@
     # Clean paths and remove trailing slashes if any
     $copy_dir_src = File::Spec->canonpath(shift);
     $copy_dir_dst = File::Spec->canonpath(shift);
-    $copy_dir_exclude_regexp = shift;
+    $process_dir_exclude_regexp = shift;
     $copy_dir_overwrite = shift;
 
+    undef %processed_files;
     find(\&move_file_callback, $copy_dir_src);
 }
 
 #
+# cleanup_dir_recursively subroutine removes files from directory
+# excluding files matching a specifies regexp and files listed in
+# processed_files.
+#
+# SYNOPSIS 
+#
+#     cleanup_dir_recursively($dir, $exclude_regexp);
+#
+sub cleanup_dir_recursively {
+    # Clean paths and remove trailing slashes if any
+    $cleanup_dir_dst = File::Spec->canonpath(shift);
+    $process_dir_exclude_regexp = shift;
+    find(\&remove_file_callback, $cleanup_dir_dst);
+}
+
+#
+# parse_innodb_data_file_path parse innodb_data_file_path and returns
+# it components as array of hash refs. Each hash ref has two components
+# the one named 'path' is the data file path as specified in the
+# innodb-data-file-path, second one named 'filename' is the data file name
+#
+sub parse_innodb_data_file_path {
+    my $innodb_data_file_path = shift;
+    my @data_file_paths = ();
+
+    foreach my $data_file (split(/;/, $innodb_data_file_path)) {
+        my $data_file_path = (split(/:/,$data_file))[0];
+        my $data_file_name = (split(/\/+/, $data_file_path))[-1];
+        push(@data_file_paths, {path => $data_file_path,
+                                filename => $data_file_name});
+    }
+
+    return @data_file_paths;
+}
+
+#
 # copy_back subroutine copies data and index files from backup directory 
 # back to their original locations.
 #
@@ -760,24 +823,26 @@
 
     # make a list of all ibdata files in the backup directory and all
     # directories in the backup directory under which there are ibdata files
-    foreach my $a (split(/;/, $orig_innodb_data_file_path)) {
-        my $path = (split(/:/,$a))[0];
-        my $filename = (split(/\/+/, $path))[-1];
+    foreach my $c (parse_innodb_data_file_path($orig_innodb_data_file_path)) {
 
         # check that the backup data file exists
-        if (! -e "$backup_dir/$filename") {
-            if (-e "$backup_dir/${filename}.ibz") {
-                Die "Backup data file '$backup_dir/$filename' does not exist, but "
-                  . "its compressed copy '${path}.ibz' exists. Check "
-                  . "that you have run '$innobackup_script --apply-log --uncompress "
-                  . "...' before attempting '$innobackup_script --copy-back ...'  "
+        if (! -e "$backup_dir/$c->{filename}") {
+            if (-e "$backup_dir/$c->{filename}.ibz") {
+                Die "Backup data file '$backup_dir/$c->{filename}' "
+                  . "does not exist, but "
+                  . "its compressed copy '$c->{path}.ibz' exists. Check "
+                  . "that you have run "
+                  . "'$innobackup_script --apply-log --uncompress "
+                  . "...' before attempting "
+                  . "'$innobackup_script --copy-back ...'  "
                   . "or '$innobackup_script --move-back ...' !";
             } else {
-                Die "Backup data file '$backup_dir/$filename' does not exist.";
+                Die "Backup data file '$backup_dir/$c->{filename}' "
+                    . "does not exist.";
             }
         }
 	
-        $excluded_files .= "|\Q$filename\E";
+        $excluded_files .= "|\Q$c->{filename}\E";
     }
 
     # copy files from backup dir to their original locations
@@ -792,12 +857,10 @@
     print STDERR "\n$prefix Starting to $operation InnoDB system tablespace\n";
     print STDERR "$prefix in '$backup_dir'\n";
     print STDERR "$prefix back to original InnoDB data directory '$orig_ibdata_dir'\n";
-    foreach my $a (split(/;/, $orig_innodb_data_file_path)) {
+    foreach my $c (parse_innodb_data_file_path($orig_innodb_data_file_path)) {
         # get the relative pathname of a data file
-        my $path = (split(/:/,$a))[0];
-        my $filename = (split(/\/+/, $path))[-1];
-        $src_name = escape_path("$backup_dir/$filename");
-        $dst_name = escape_path("$orig_ibdata_dir/$path");
+        $src_name = escape_path("$backup_dir/$c->{filename}");
+        $dst_name = escape_path("$orig_ibdata_dir/$c->{path}");
         &$move_or_copy_file($src_name, $dst_name);
     }
 
@@ -866,6 +929,9 @@
         $options .= " --tmpdir=$option_tmpdir";
     }
 
+    my $innodb_data_file_path = 
+        get_option(\%config, $option_defaults_group, 'innodb_data_file_path');
+
     # run ibbackup as a child process
     $cmdline = "$option_ibbackup_binary $options";
     $now = current_time();
@@ -894,23 +960,16 @@
     if ( $option_incremental_dir ) {
 	print STDERR "$prefix Starting to copy non-InnoDB files in '$option_incremental_dir'\n"; 
 	print STDERR "$prefix to the full backup directory '$backup_dir'\n";
-	copy_dir_recursively($option_incremental_dir, $backup_dir,
-			     '^(\.\.?|backup-my\.cnf|xtrabackup_logfile|' .
-			     'xtrabackup_binary|xtrabackup_checkpoints|' .
-			     '.*\.delta|.*\.meta|ib_logfile.*)$', 1);
-        # If the latest backup has no file, we need to remove the old
-        # xtrabackup_slave_info file, because it is out of date
-        # TODO: this will not be needed when bug #856400 is fixed.
-        if ( -e "$backup_dir/xtrabackup_slave_info" ) {
-            print STDERR "\n$now  $prefix No updated xtrabackup_slave_info found in incremental dir, removing stale xtrabackup_slave_info file from target dir.";
-            $cmdline = "rm $backup_dir/xtrabackup_slave_info";
-            $rcode = system("$cmdline");
-            if ($rcode) {
-                # failure
-                Die "\n$prefix Failed to remove stale xtrabackup_slave_info in $backup_dir";
-            }
-
+        my @skip_list = ('\.\.?','backup-my\.cnf','xtrabackup_logfile',
+                        'xtrabackup_binary','xtrabackup_checkpoints',
+                        '.*\.delta','.*\.meta','ib_logfile.*');
+        copy_dir_recursively($option_incremental_dir, $backup_dir,
+                             '^(' . join('|', @skip_list) . ')$', 1);
+    foreach my $c (parse_innodb_data_file_path($innodb_data_file_path)) {
+            push(@skip_list, "\Q$c->{filename}\E");
         }
+        cleanup_dir_recursively($backup_dir,
+                             '^(' . join('|', @skip_list, '.*\.ibd') . ')$');
     }
 
 }

=== 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 @@
 char *xtrabackup_tables_file = NULL;
 hash_table_t* tables_hash;
 
+hash_table_t* inc_dir_tables_hash;
+
 struct xtrabackup_tables_struct{
 	char*		name;
 	hash_node_t	name_hash;
@@ -5006,7 +5008,7 @@
 			}
 
 			table = static_cast<xtrabackup_tables_t *>
-				(malloc(sizeof(xtrabackup_tables_t)
+				(ut_malloc(sizeof(xtrabackup_tables_t)
 					+ strlen(name_buf) + 1));
 			memset(table, '\0', sizeof(xtrabackup_tables_t) + strlen(name_buf) + 1);
 			table->name = ((char*)table) + sizeof(xtrabackup_tables_t);
@@ -5018,6 +5020,34 @@
 	}
 }
 
+static
+void
+xb_tables_hash_free(hash_table_t* hash)
+{
+	ulint	i;
+
+	/* free the hash elements */
+	for (i = 0; i < hash_get_n_cells(hash); i++) {
+		xtrabackup_tables_t*	table;
+
+		table = static_cast<xtrabackup_tables_t *>
+			(HASH_GET_FIRST(hash, i));
+
+		while (table) {
+			xtrabackup_tables_t*	prev_table = table;
+
+			table = static_cast<xtrabackup_tables_t *>
+				(HASH_GET_NEXT(name_hash, prev_table));
+
+			HASH_DELETE(xtrabackup_tables_t, name_hash, hash,
+				ut_fold_string(prev_table->name), prev_table);
+			ut_free(prev_table);
+		}
+	}
+
+	/* free hash */
+	hash_table_free(hash);
+}
 
 /************************************************************************
 Destroy table filters for partial backup. */
@@ -5028,29 +5058,7 @@
 {
 
 	if (xtrabackup_tables_file) {
-		ulint	i;
-
-		/* free the hash elements */
-		for (i = 0; i < hash_get_n_cells(tables_hash); i++) {
-			xtrabackup_tables_t*	table;
-
-			table = static_cast<xtrabackup_tables_t *>
-				(HASH_GET_FIRST(tables_hash, i));
-
-			while (table) {
-				xtrabackup_tables_t*	prev_table = table;
-
-				table = static_cast<xtrabackup_tables_t *>
-					(HASH_GET_NEXT(name_hash, prev_table));
-
-				HASH_DELETE(xtrabackup_tables_t, name_hash, tables_hash,
-						ut_fold_string(prev_table->name), prev_table);
-				free(prev_table);
-			}
-		}
-
-		/* free tables_hash */
-		hash_table_free(tables_hash);
+		xb_tables_hash_free(tables_hash);
 	}
 
 }
@@ -6625,12 +6633,13 @@
 	size_t		real_name_len,	/* out: buffer size for real_name */
 	ibool* 		success)	/* out: indicates error. TRUE = success */
 {
-	char		dest_dir[FN_REFLEN];
-	char		dest_space_name[FN_REFLEN];
-	ibool		ok;
-	fil_space_t*	fil_space;
-	os_file_t	file	= 0;
-	ulint		tablespace_flags;
+	char			dest_dir[FN_REFLEN];
+	char			dest_space_name[FN_REFLEN];
+	ibool			ok;
+	fil_space_t*		fil_space;
+	os_file_t		file	= 0;
+	ulint			tablespace_flags;
+	xtrabackup_tables_t*	table;
 
 	ut_a(dbname != NULL ||
 		trx_sys_sys_space(space_id) ||
@@ -6669,6 +6678,16 @@
 		goto found;
 	}
 
+	/* remember space name for further reference */
+	table = static_cast<xtrabackup_tables_t *>
+		(ut_malloc(sizeof(xtrabackup_tables_t) +
+			strlen(dest_space_name) + 1));
+
+	table->name = ((char*)table) + sizeof(xtrabackup_tables_t);
+	strcpy(table->name, dest_space_name);
+	HASH_INSERT(xtrabackup_tables_t, name_hash, inc_dir_tables_hash,
+			ut_fold_string(table->name), table);
+
 	mutex_enter(&fil_system->mutex);
 	fil_space = xb_space_get_by_name(dest_space_name);
 	mutex_exit(&fil_system->mutex);
@@ -6792,7 +6811,7 @@
 	const char*	dbname,		/* in: database name (ibdata: NULL) */
 	const char*	filename,	/* in: file name (not a path),
 					including the .delta extension */
-	my_bool check_newer __attribute__((unused)))
+	void*		/*data*/)
 {
 	os_file_t	src_file = XB_FILE_UNDEFINED;
 	os_file_t	dst_file = XB_FILE_UNDEFINED;
@@ -6989,11 +7008,61 @@
 }
 
 /************************************************************************
-Applies all .delta files from incremental_dir to the full backup.
-@return TRUE on success. */
-static
-ibool
-xtrabackup_apply_deltas(my_bool check_newer)
+Callback to handle datadir entry. Function of this type will be called
+for each entry which matches the mask by xb_process_datadir.
+@return should return TRUE on success */
+typedef ibool (*handle_datadir_entry_func_t)(
+/*=========================================*/
+	const char*	data_home_dir,		/*!<in: path to datadir */
+	const char*	db_name,		/*!<in: database name */
+	const char*	file_name,		/*!<in: file name with suffix */
+	void*		arg);			/*!<in: caller-provided data */
+
+/************************************************************************
+Callback to handle datadir entry. Deletes entry if it has no matching
+fil_space in fil_system directory.
+@return FALSE if delete attempt was unsuccessful */
+static
+ibool
+rm_if_not_found(
+	const char*	data_home_dir,		/*!<in: path to datadir */
+	const char*	db_name,		/*!<in: database name */
+	const char*	file_name,		/*!<in: file name with suffix */
+	void*		arg __attribute__((unused)))
+{
+	char			name[FN_REFLEN];
+	xtrabackup_tables_t*	table;
+
+	snprintf(name, FN_REFLEN, "%s%s/%s",
+				xb_dict_prefix, db_name, file_name);
+	name[strlen(name) - XB_DICT_SUFFIX_LEN] = '\0';
+
+	XB_HASH_SEARCH(name_hash, inc_dir_tables_hash, ut_fold_string(name),
+		       table, (void) 0,
+		       !strcmp(table->name, name));
+
+	if (!table) {
+		snprintf(name, FN_REFLEN, "%s/%s/%s", data_home_dir,
+						      db_name, file_name);
+		return os_file_delete(name);
+	}
+
+	return(TRUE);
+}
+
+/************************************************************************
+Function enumerates files in datadir (provided by path) which are matched
+by provided suffix. For each entry callback is called.
+@return FALSE if callback for some entry returned FALSE */
+static
+ibool
+xb_process_datadir(
+	const char*			path,	/*!<in: datadir path */
+	const char*			suffix,	/*!<in: suffix to match
+						against */
+	handle_datadir_entry_func_t	func,	/*!<in: callback */
+	void*				data)	/*!<in: additional argument for
+						callback */
 {
 	ulint		ret;
 	char		dbpath[FN_REFLEN];
@@ -7001,56 +7070,59 @@
 	os_file_dir_t	dbdir;
 	os_file_stat_t	dbinfo;
 	os_file_stat_t	fileinfo;
-	dberr_t		err		= DB_SUCCESS;
+	ulint		suffix_len;
+	dberr_t		err 		= DB_SUCCESS;
 	static char	current_dir[2];
 
 	current_dir[0] = FN_CURLIB;
 	current_dir[1] = 0;
 	srv_data_home = current_dir;
 
+	suffix_len = strlen(suffix);
+
 	/* datafile */
-	dbdir = os_file_opendir(xtrabackup_incremental_dir, FALSE);
+	dbdir = os_file_opendir(path, FALSE);
 
 	if (dbdir != NULL) {
-		ret = fil_file_readdir_next_file(&err, xtrabackup_incremental_dir, dbdir,
+		ret = fil_file_readdir_next_file(&err, path, dbdir,
 							&fileinfo);
 		while (ret == 0) {
 			if (fileinfo.type == OS_FILE_TYPE_DIR) {
 				goto next_file_item_1;
 			}
 
-			if (strlen(fileinfo.name) > 6
+			if (strlen(fileinfo.name) > suffix_len
 			    && 0 == strcmp(fileinfo.name + 
-					strlen(fileinfo.name) - 6,
-					".delta")) {
-				if (!xtrabackup_apply_delta(
-					    xtrabackup_incremental_dir, NULL,
-					    fileinfo.name, check_newer))
+					strlen(fileinfo.name) - suffix_len,
+					suffix)) {
+				if (!func(
+					    path, NULL,
+					    fileinfo.name, data))
 				{
-					return FALSE;
+					return(FALSE);
 				}
 			}
 next_file_item_1:
 			ret = fil_file_readdir_next_file(&err,
-							xtrabackup_incremental_dir, dbdir,
+							path, dbdir,
 							&fileinfo);
 		}
 
 		os_file_closedir(dbdir);
 	} else {
 		msg("xtrabackup: Cannot open dir %s\n",
-		    xtrabackup_incremental_dir);
+		    path);
 	}
 
 	/* single table tablespaces */
-	dir = os_file_opendir(xtrabackup_incremental_dir, FALSE);
+	dir = os_file_opendir(path, FALSE);
 
 	if (dir == NULL) {
 		msg("xtrabackup: Cannot open dir %s\n",
-		    xtrabackup_incremental_dir);
+		    path);
 	}
 
-		ret = fil_file_readdir_next_file(&err, xtrabackup_incremental_dir, dir,
+		ret = fil_file_readdir_next_file(&err, path, dir,
 								&dbinfo);
 	while (ret == 0) {
 		if (dbinfo.type == OS_FILE_TYPE_FILE
@@ -7059,7 +7131,7 @@
 		        goto next_datadir_item;
 		}
 
-		sprintf(dbpath, "%s/%s", xtrabackup_incremental_dir,
+		sprintf(dbpath, "%s/%s", path,
 								dbinfo.name);
 		srv_normalize_path_for_win(dbpath);
 
@@ -7076,18 +7148,19 @@
 				        goto next_file_item_2;
 				}
 
-				if (strlen(fileinfo.name) > 6
+				if (strlen(fileinfo.name) > suffix_len
 				    && 0 == strcmp(fileinfo.name + 
-						strlen(fileinfo.name) - 6,
-						".delta")) {
-					/* The name ends in .delta; try opening
+						strlen(fileinfo.name) -
+								suffix_len,
+						suffix)) {
+					/* The name ends in suffix; process
 					the file */
-					if (!xtrabackup_apply_delta(
-						    xtrabackup_incremental_dir,
+					if (!func(
+						    path,
 						    dbinfo.name,
-						    fileinfo.name, check_newer))
+						    fileinfo.name, data))
 					{
-						return FALSE;
+						return(FALSE);
 					}
 				}
 next_file_item_2:
@@ -7100,13 +7173,24 @@
 		}
 next_datadir_item:
 		ret = fil_file_readdir_next_file(&err,
-						xtrabackup_incremental_dir,
+						path,
 								dir, &dbinfo);
 	}
 
 	os_file_closedir(dir);
 
-	return TRUE;
+	return(TRUE);
+}
+
+/************************************************************************
+Applies all .delta files from incremental_dir to the full backup.
+@return TRUE on success. */
+static
+ibool
+xtrabackup_apply_deltas()
+{
+	return xb_process_datadir(xtrabackup_incremental_dir, ".delta",
+		xtrabackup_apply_delta, NULL);
 }
 
 static my_bool
@@ -7289,12 +7373,22 @@
 			goto error;
 		}
 
-		if(!xtrabackup_apply_deltas(TRUE)) {
+		inc_dir_tables_hash = hash_create(1000);
+
+		if(!xtrabackup_apply_deltas()) {
 			xb_data_files_close();
+			xb_tables_hash_free(inc_dir_tables_hash);
 			goto error;
 		}
 
 		xb_data_files_close();
+
+		/* Cleanup datadir from tablespaces deleted between full and
+		incremental backups */
+
+		xb_process_datadir("./", ".ibd", rm_if_not_found, NULL);
+
+		xb_tables_hash_free(inc_dir_tables_hash);
 	}
 	sync_close();
 	sync_initialized = FALSE;

=== 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 @@
+########################################################################
+# Bug #856400: RENAME TABLE causes incremental prepare to fail
+########################################################################
+
+. inc/common.sh
+. inc/ib_part.sh
+
+start_server --innodb_file_per_table
+
+require_partitioning
+
+run_cmd $MYSQL $MYSQL_ARGS test <<EOF
+CREATE TABLE t1(a INT) ENGINE=InnoDB;
+INSERT INTO t1 VALUES (1), (2), (3);
+
+CREATE TABLE t2(a INT) ENGINE=InnoDB;
+INSERT INTO t2 VALUES (4), (5), (6);
+
+CREATE TABLE p (
+  a int
+) ENGINE=InnoDB
+PARTITION BY RANGE (a)
+(PARTITION p0 VALUES LESS THAN (100),
+ PARTITION p1 VALUES LESS THAN (200),
+ PARTITION p2 VALUES LESS THAN (300),
+ PARTITION p3 VALUES LESS THAN (400));
+
+INSERT INTO p VALUES (1), (101), (201), (301);
+
+
+CREATE TABLE isam_t1(a INT) ENGINE=MyISAM;
+INSERT INTO isam_t1 VALUES (1), (2), (3);
+
+CREATE TABLE isam_t2(a INT) ENGINE=MyISAM;
+INSERT INTO isam_t2 VALUES (4), (5), (6);
+
+CREATE TABLE isam_p (
+  a int
+) ENGINE=MyISAM
+PARTITION BY RANGE (a)
+(PARTITION p0 VALUES LESS THAN (100),
+ PARTITION p1 VALUES LESS THAN (200),
+ PARTITION p2 VALUES LESS THAN (300),
+ PARTITION p3 VALUES LESS THAN (400));
+
+INSERT INTO isam_p VALUES (1), (101), (201), (301);
+
+EOF
+
+# Full backup
+vlog "Creating full backup"
+innobackupex  --no-timestamp $topdir/full
+
+vlog "Making changes"
+
+run_cmd $MYSQL $MYSQL_ARGS test <<EOF
+
+DROP TABLE t1;
+
+DROP TABLE t2;
+CREATE TABLE t2(a INT) ENGINE=InnoDB;
+INSERT INTO t2 VALUES (40), (50), (60);
+
+ALTER TABLE p DROP PARTITION p0;
+ALTER TABLE p DROP PARTITION p1;
+ALTER TABLE p ADD PARTITION (PARTITION p4 VALUES LESS THAN (500));
+ALTER TABLE p ADD PARTITION (PARTITION p5 VALUES LESS THAN (600));
+
+INSERT INTO p VALUES (401), (501);
+
+
+DROP TABLE isam_t1;
+DROP TABLE isam_t2;
+CREATE TABLE isam_t2(a INT) ENGINE=MyISAM;
+
+INSERT INTO isam_t2 VALUES (40), (50), (60);
+
+ALTER TABLE isam_p DROP PARTITION p0;
+ALTER TABLE isam_p DROP PARTITION p1;
+ALTER TABLE isam_p ADD PARTITION (PARTITION p4 VALUES LESS THAN (500));
+ALTER TABLE isam_p ADD PARTITION (PARTITION p5 VALUES LESS THAN (600));
+
+INSERT INTO isam_p VALUES (401), (501);
+
+EOF
+
+vlog "Creating incremental backup"
+
+innobackupex --incremental --no-timestamp \
+    --incremental-basedir=$topdir/full $topdir/inc
+
+vlog "Preparing backup"
+
+innobackupex --apply-log --redo-only $topdir/full
+vlog "Log applied to full backup"
+
+innobackupex --apply-log --redo-only --incremental-dir=$topdir/inc \
+    $topdir/full
+vlog "Delta applied to full backup"
+
+innobackupex --apply-log $topdir/full
+vlog "Data prepared for restore"
+
+ls -al $topdir/full/test/*
+
+# we expect to see
+# 5 InnoDB tablespaces
+count=`ls $topdir/full/test/*.ibd | wc -l`
+vlog "$count .ibd in restore, expecting 5"
+test $count -eq 5
+
+# 5 MyISAM data files
+count=`ls $topdir/full/test/*.MYD | wc -l`
+vlog "$count .MYD in restore, expecting 5"
+test $count -eq 5
+
+# and 10 tables overall
+count=`ls $topdir/full/test/*.frm | wc -l`
+vlog "$count .frm in restore, expecting 4"
+test $count -eq 4