Merge lp:~akopytov/percona-xtrabackup/bug1130627-2.1 into lp:percona-xtrabackup/2.1

[Alexey Kopytov]

    Bug #1130627: Can't backup individual partitions

    The bug is a regression introduced in 2.0.5 by the fix for bug
    #711166. The fix introduced partition suffix ('#P#...') trimming in
    tables names before checking them against regular expressions specified
    by the --include option in innobackupex or the --tables option in
    xtrabackup. Which made impossible partial backups of partitioned tables.

    Fixed in both innobackupex and xtrabackup by first checking the table
    name without the suffix trimmed, and then repeating the check with the
    suffix trimmed if the former check fails.

    This patch also fixes bug #1131084 "Unneccessary/debug print in
    xtrabackup output" and a few other minor issues introduced by the fix
    for bug #711166.

http://jenkins.percona.com/view/XtraBackup/job/percona-xtrabackup-2.1-param/199/

[Sergei Glushchenko (sergei.glushchenko)]

Same comments as for 2.0 version.

review: Needs Fixing (g2)

[Alexey Kopytov (akopytov)]

Jenkins build after addressing the review comments: http://jenkins.percona.com/view/XtraBackup/job/percona-xtrabackup-2.1-param/194/

[Sergei Glushchenko (sergei.glushchenko)]

See comments re: testcase on 2.0 version of MP.

review: Needs Fixing (g2)

[Alexey Kopytov (akopytov)]

No idea why LP thinks this has been merged. It hasn't. Let's see if resubmitting will fix it.

[Alexey Kopytov (akopytov)]

http://jenkins.percona.com/view/XtraBackup/job/percona-xtrabackup-2.1-param/199/

[Sergei Glushchenko (sergei.glushchenko)]

review: Approve (g2)

=== modified file 'innobackupex'
--- innobackupex	2013-02-04 07:33:56 +0000
+++ innobackupex	2013-03-07 17:52:22 +0000
@@ -2037,13 +2037,18 @@
             next unless check_if_required($database, $file);
 
 	    if($option_include) {
-	      my $table_name = get_table_name($file);
-	      if (!("$database.$table_name" =~ /$option_include/)) {
-	        print STDERR "$database.$file is skipped because it does not match $option_include.\n";
-	        next;
-	      }
+                my $table = get_table_name($file);
+                my $table_part = get_table_name_with_part_suffix($file);
+
+                if (!("$database.$table_part" =~ /$option_include/ ||
+                      "$database.$table" =~ /$option_include/)) {
+
+                    if ($print_each_file) {
+                        print STDERR "$database.$file is skipped because it does not match '$option_include'.\n";
+                    }
+                    next;
+                }
 	    }
-
                
             if ($print_each_file) {
                 print STDERR "$prefix Backing up file '$source_dir/$database/$file'\n";
@@ -2371,6 +2376,17 @@
 #       1 table name
 #
 sub get_table_name {
+   my $table;
+
+   $table = get_table_name_with_part_suffix($_[0]);
+   # trim partition suffix
+   $table = (split('#P#', $table))[0];
+
+   return $table;
+}
+
+# Like get_table_name(), but keeps the partition suffix if present
+sub get_table_name_with_part_suffix {
    my $table_path = shift;
    my $filename;
    my $table;
@@ -2379,8 +2395,6 @@
    $filename = (reverse(split(/\//, $table_path)))[0];
    # get name of the table by removing file suffix
    $table = (split(/\./, $filename))[0];
-   # and partition suffix
-   $table = (split('#P#', $table))[0];
 
    return $table;
 }
@@ -2398,14 +2412,16 @@
    my $db_count  = scalar keys %databases_list;
    my $tbl_count = scalar keys %table_list;
    my $table;
+   my $table_part;
 
    if ( $db_count == 0 && $tbl_count == 0 ) {
       # No databases defined with --databases option, include all databases,
       # and no tables defined with --tables-file option, include all tables.
-      return 1;
+       return 1;
    }
    else {
       if ( $table_path ) {
+         $table_part = get_table_name_with_part_suffix($table_path);
          $table = get_table_name($table_path);
       }
    }
@@ -2416,7 +2432,9 @@
          if (defined $table_path) {
             my $db_hash = $databases_list{$db};
             $db_count = keys %$db_hash;
-            if ($db_count > 0 && ! defined $databases_list{$db}->{$table}) {
+            if ($db_count > 0 &&
+                !defined $databases_list{$db}->{$table_part} &&
+                !defined $databases_list{$db}->{$table}) {
                # --databases option specified, but table is not included
                return 0;
             }
@@ -2433,7 +2451,8 @@
    # Filter for --tables-file.
    if ( $tbl_count ) {
       return 0 unless exists $table_list{$db};
-      return 0 if $table && !$table_list{$db}->{$table};
+      return 0 if $table && !$table_list{$db}->{$table_part} &&
+          !$table_list{$db}->{$table};
    }
 
    return 1;  # backup the table

=== modified file 'src/xtrabackup.cc'
--- src/xtrabackup.cc	2013-03-01 07:01:10 +0000
+++ src/xtrabackup.cc	2013-03-07 17:52:22 +0000
@@ -1506,6 +1506,46 @@
 }
 
 /************************************************************************
+Checks if a given table name matches any of specifications in the --tables or
+--tables-file options.
+
+@return TRUE on match. */
+static my_bool
+check_if_table_matches_filters(const char *name)
+{
+	int 			regres;
+	int 			i;
+	xtrabackup_tables_t*	table;
+
+	if (xtrabackup_tables) {
+
+		regres = REG_NOMATCH;
+
+		for (i = 0; i < tables_regex_num; i++) {
+			regres = xb_regexec(&tables_regex[i], name, 1,
+					    tables_regmatch, 0);
+			if (regres != REG_NOMATCH) {
+
+				return(TRUE);
+			}
+		}
+	}
+
+	if (xtrabackup_tables_file) {
+
+		XB_HASH_SEARCH(name_hash, tables_hash, ut_fold_string(name),
+			       table, (void) 0,
+			       !strcmp(table->name, name));
+		if (table) {
+
+			return(TRUE);
+		}
+	}
+
+	return(FALSE);
+}
+
+/************************************************************************
 Checks if a table specified as a path should be skipped from backup
 based on the --tables or --tables-file options.
 
@@ -1548,37 +1588,24 @@
 	}
 	buf[dbname_len - 1] = 0;
 
+	/* For partitioned tables first try to match against the regexp
+	without truncating the #P#... suffix so we can backup individual
+	partitions with regexps like '^test[.]t#P#p5' */
+	if (check_if_table_matches_filters(buf)) {
+
+		return(FALSE);
+	}
 	if ((eptr = strstr(buf, "#P#")) != NULL) {
+
 		*eptr = 0;
-	}
-
-	if (xtrabackup_tables) {
-		int regres = REG_NOMATCH;
-		int i;
-		for (i = 0; i < tables_regex_num; i++) {
-			regres = xb_regexec(&tables_regex[i], buf, 1,
-					    tables_regmatch, 0);
-			if (regres != REG_NOMATCH) {
-				break;
-			}
-		}
-		if (regres == REG_NOMATCH) {
-			return(TRUE);
-		}
-	}
-
-	if (xtrabackup_tables_file) {
-		xtrabackup_tables_t*	table;
-
-		XB_HASH_SEARCH(name_hash, tables_hash, ut_fold_string(buf),
-			       table, (void) 0,
-			       !strcmp(table->name, buf));
-		if (!table) {
-			return(TRUE);
-		}
-	}
-
-	return(FALSE);
+
+		if (check_if_table_matches_filters(buf)) {
+
+			return(FALSE);
+		}
+	}
+
+	return(TRUE);
 }
 
 /***********************************************************************
@@ -2383,6 +2410,8 @@
 
 		p = xtrabackup_tables;
 		for (i=0; i < tables_regex_num; i++) {
+			int	rc;
+
 			next = strchr(p, ',');
 			ut_a(next || i == tables_regex_num - 1);
 
@@ -2390,10 +2419,18 @@
 			if (i != tables_regex_num - 1)
 				*(next - 1) = '\0';
 
-			xb_regerror(xb_regcomp(&tables_regex[i], p,
-					       REG_EXTENDED),
-				    &tables_regex[i], errbuf, sizeof(errbuf));
-			msg("xtrabackup: tables regcomp(%s): %s\n", p, errbuf);
+			rc = xb_regcomp(&tables_regex[i], p,
+					REG_EXTENDED);
+
+			if (rc) {
+
+				xb_regerror(rc, &tables_regex[i], errbuf,
+					    sizeof(errbuf));
+				msg("xtrabackup: regcomp(%s) failed: %s\n",
+				    p, errbuf);
+
+				exit(EXIT_FAILURE);
+			}
 
 			if (i != tables_regex_num - 1)
 				*(next - 1) = ',';
@@ -2440,9 +2477,6 @@
 
 			HASH_INSERT(xtrabackup_tables_t, name_hash, tables_hash,
 					ut_fold_string(table->name), table);
-
-			msg("xtrabackup: table '%s' is registered to the "
-			    "list.\n", table->name);
 		}
 	}
 }

=== added file 'test/t/bug1130627.sh'
--- test/t/bug1130627.sh	1970-01-01 00:00:00 +0000
+++ test/t/bug1130627.sh	2013-03-07 17:52:22 +0000
@@ -0,0 +1,74 @@
+########################################################################
+# Bug #1130627: Can't backup individual partitions
+########################################################################
+
+. inc/common.sh
+. inc/ib_part.sh
+
+start_server --innodb_file_per_table
+
+require_partitioning
+
+run_cmd $MYSQL $MYSQL_ARGS test <<EOF
+CREATE TABLE t_innodb (
+  i INT NOT NULL AUTO_INCREMENT,
+  c CHAR(10) DEFAULT NULL,
+  PRIMARY KEY (i)
+) ENGINE=InnoDB
+/*!50100 PARTITION BY HASH (i)
+PARTITIONS 10 */;
+
+CREATE TABLE t_myisam (
+  i INT NOT NULL AUTO_INCREMENT,
+  c CHAR(10) DEFAULT NULL,
+  PRIMARY KEY (i)
+) ENGINE=MyISAM
+/*!50100 PARTITION BY HASH (i)
+PARTITIONS 10 */;
+EOF
+
+# Force a checkpoint
+stop_server
+start_server --innodb_file_per_table
+
+# Test that specifying partitions with --include works
+innobackupex --no-timestamp --include='^test.*#P#p5' $topdir/backup
+innobackupex --apply-log $topdir/backup
+
+file_cnt=`ls $topdir/backup/test | wc -l`
+
+test "$file_cnt" -eq 3
+
+test -f $topdir/backup/test/t_innodb#P#p5.ibd
+test -f $topdir/backup/test/t_myisam#P#p5.MYD
+test -f $topdir/backup/test/t_myisam#P#p5.MYI
+
+rm -rf $topdir/backup
+
+# Test that specifying partitions with --databases works
+# TODO: uncomment when bug #569387 is fixed
+
+# innobackupex --no-timestamp \
+#     --databases='test.t_myisam#P#p5 test.t_innodb#P#p5' $topdir/backup
+# innobackupex --apply-log $topdir/backup
+
+# test "$file_cnt" -eq 3
+# test -f $topdir/backup/test/t_innodb#P#p5.ibd
+# test -f $topdir/backup/test/t_myisam#P#p5.MYD
+# test -f $topdir/backup/test/t_myisam#P#p5.MYI
+
+# rm -rf $topdir/backup
+
+# Test that specifying partitions with --tables-file works
+cat >$topdir/tables_file <<EOF
+test.t_myisam#P#p5
+test.t_innodb#P#p5
+EOF
+innobackupex --no-timestamp --tables-file=$topdir/tables_file $topdir/backup
+innobackupex --apply-log $topdir/backup
+
+test "$file_cnt" -eq 3
+test -f $topdir/backup/test/t_myisam#P#p5.MYD
+test -f $topdir/backup/test/t_myisam#P#p5.MYI
+
+rm -rf $topdir/backup