Merge lp:~akopytov/percona-xtrabackup/bug870119-2.0 into lp:percona-xtrabackup/2.0

[Alexey Kopytov]

Bug #870119: assertion failure while trying to read InnoDB partition

The problem was that the InnoDB file I/O subsystem may reuse file
descriptors by closing the old ones when the number of open files hits
innodb_open_files. Which works for InnoDB, because if InnoDB needs to
access a table which has been closed, it would just reopen it.However,
that didn't work for XtraBackup, since it only keeps a file descriptor
when copying a file. So when the --parallel option was used, there was a
chance that another thread wanted to open a file and hit
innodb_open_files. Therefore fil_try_to_close_file_in_LRU() might close
the file descriptor which was currently in use by another thread and
then this descriptor is shortly reused when opening another file. Which
would result in obscure failures like this.

However, most part of the problem disappeared with the fix for bug
#713267: XtraBackup no longer relies on the file descriptors created by
the 'fil' subsystem of InnoDB to get tablespace flags, and that was the
reason for exhausting the innodb_open_files pool. So bug #870119 is
essentially a duplicate, or at least became extremely hard to hit in the
wild.

The only thing that should be done to exclude even a theoretical chance
of reusing file descriptors from the 'fil' subsystem is to always create
a new file descriptor to copy the file, instead of checking if a node
has been previously open by the 'fil' subsystem and reusing the
descriptor if so. Which is what this fix implements.

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

[Sergei Glushchenko (sergei.glushchenko)]

I saw similar issue couple of times and was pretty much surprised why we reusing descriptors from fil_system. I even filed a bug https://bugs.launchpad.net/percona-xtrabackup/+bug/1059151. Marking it as duplicate of bug 870119 since it has been fixed.

Approve.

review: Approve (g2)

=== modified file 'src/xtrabackup.cc'
--- src/xtrabackup.cc	2013-03-01 06:54:51 +0000
+++ src/xtrabackup.cc	2013-03-06 04:51:20 +0000
@@ -3334,28 +3334,24 @@
 	}
 
 	/* open src_file*/
-	if (!node->open) {
-		src_file = xb_file_create_no_error_handling(node->name,
-							    OS_FILE_OPEN,
-							    OS_FILE_READ_ONLY,
-							    &success);
-		if (!success) {
-			/* The following call prints an error message */
-			os_file_get_last_error(TRUE);
-
-			msg("[%02u] xtrabackup: Warning: cannot open %s\n"
-			    "[%02u] xtrabackup: Warning: We assume the "
-			    "table was dropped or renamed during "
-			    "xtrabackup execution and ignore the file.\n",
-			    thread_n, node->name, thread_n);
-			goto skip;
-		}
-
-		xb_file_set_nocache(src_file, node->name, "OPEN");
-	} else {
-		src_file = node->handle;
+	src_file = xb_file_create_no_error_handling(node->name,
+						    OS_FILE_OPEN,
+						    OS_FILE_READ_ONLY,
+						    &success);
+	if (!success) {
+		/* The following call prints an error message */
+		os_file_get_last_error(TRUE);
+
+		msg("[%02u] xtrabackup: Warning: cannot open %s\n"
+		    "[%02u] xtrabackup: Warning: We assume the "
+		    "table was dropped or renamed during "
+		    "xtrabackup execution and ignore the file.\n",
+		    thread_n, node->name, thread_n);
+		goto skip;
 	}
 
+	xb_file_set_nocache(src_file, node->name, "OPEN");
+
 #ifdef USE_POSIX_FADVISE
 	posix_fadvise(src_file, 0, 0, POSIX_FADV_SEQUENTIAL);
 #endif

=== added file 'test/t/bug870119.sh'
--- test/t/bug870119.sh	1970-01-01 00:00:00 +0000
+++ test/t/bug870119.sh	2013-03-06 04:51:20 +0000
@@ -0,0 +1,18 @@
+########################################################################
+# Bug #870119: assertion failure while trying to read InnoDB partition 
+########################################################################
+
+. inc/common.sh
+
+start_server --innodb_file_per_table
+
+# Set the minimum value for innodb_open_files to be used by xtrabackup
+echo "innodb_open_files=10" >>$topdir/my.cnf
+
+load_dbase_schema sakila
+load_dbase_data sakila
+
+innobackupex --no-timestamp $topdir/backup --parallel=16
+
+# No need to prepare and verify the backup, as the bug was resulting in a fatal
+# error during the backup