Merge lp:~percona-toolkit-dev/percona-toolkit/fix-sync-ignore-bug-1002365 into lp:percona-toolkit/2.1

[Daniel Nichter]

[Daniel Nichter (daniel-nichter)]

review: Approve

=== modified file 'bin/pt-table-sync'
--- bin/pt-table-sync	2012-05-17 14:54:36 +0000
+++ bin/pt-table-sync	2012-05-21 21:57:26 +0000
@@ -4248,10 +4248,9 @@
       . "FROM $table "
       . "WHERE master_cnt <> this_cnt OR master_crc <> this_crc "
       . "OR ISNULL(master_crc) <> ISNULL(this_crc)";
-
    PTDEBUG && _d($sql);
    my $diffs = $dbh->selectall_arrayref($sql, { Slice => {} });
-   return @$diffs;
+   return $diffs;
 }
 
 sub _d {
@@ -7988,11 +7987,16 @@
       tbl      => undef,  # set later
    };
 
-   # Filters for --databases and --tables.  We have to do these manually
-   # since we don't use MySQLFind for --replicate.
-   my $databases = $o->get('databases');
-   my $tables    = $o->get('tables');
+   # Used to filter which tables are synced.
+   # https://bugs.launchpad.net/percona-toolkit/+bug/1002365
+   my $schema_iter = new SchemaIterator(
+      dbh          => $src->{dbh},
+      OptionParser => $o,
+      TableParser  => $args{TableParser},
+      Quoter       => $args{Quoter},
+   );
 
+   my %skip_table;
    my $exit_status = 0;
 
    # Connect to the master and treat it as the source, then find
@@ -8008,30 +8012,29 @@
 
       # First, check that the master (source) has no discrepancies itself,
       # and ignore tables that do.
-      my %skip_table;
-      map { $skip_table{$_->{db}}->{$_->{tbl}}++ }
-         $checksum->find_replication_differences(
-            $src->{dbh}, $o->get('replicate'));
+      my $src_diffs = $checksum->find_replication_differences(
+         $src->{dbh}, $o->get('replicate'));
+      map { $skip_table{lc $_->{db}}->{lc $_->{tbl}}++ } @$src_diffs;
 
       # Now check the slave for differences and sync them if necessary.
-      my @diffs =  filter_diffs(
-         \%skip_table,
-         $databases,
-         $tables,
-         $checksum->find_replication_differences(
-            $dst->{dbh}, $o->get('replicate'))
+      my $dst_diffs = $checksum->find_replication_differences(
+         $dst->{dbh}, $o->get('replicate'));
+      my $diffs = filter_diffs(
+         diffs          => $dst_diffs,
+         SchemaIterator => $schema_iter,
+         skip_table     => \%skip_table,
       );
 
       if ( $o->get('verbose') ) {
-         print_header("# Syncing via replication " . $dp->as_string($dst->{dsn})
+         print_header("# Syncing via replication " .$dp->as_string($dst->{dsn})
             . ($o->get('dry-run') ?
                ' in dry-run mode, without accessing or comparing data' : ''));
       }
 
-      if ( @diffs ) {
+      if ( $diffs && scalar @$diffs ) {
          lock_server(src => $src, dst => $dst, %args);
 
-         foreach my $diff ( @diffs ) {
+         foreach my $diff ( @$diffs ) {
             $src->{db}  = $dst->{db}  = $diff->{db};
             $src->{tbl} = $dst->{tbl} = $diff->{tbl};
 
@@ -8056,7 +8059,6 @@
    # The DSN is the master.  Connect to each slave, find differences,
    # then sync them.
    else {
-      my %skip_table;
       $ms->recurse_to_slaves(
          {  dbh        => $src->{dbh},
             dsn        => $src->{dsn},
@@ -8064,20 +8066,20 @@
             recurse    => 1,
             callback   => sub {
                my ( $dsn, $dbh, $level, $parent ) = @_;
-               my @diffs = $checksum
-                  ->find_replication_differences($dbh, $o->get('replicate'));
+               my $all_diffs = $checksum->find_replication_differences(
+                  $dbh, $o->get('replicate'));
                if ( !$level ) {
                   # This is the master; don't sync any tables that are wrong
                   # here, for obvious reasons.
-                  map { $skip_table{$_->{db}}->{$_->{tbl}}++ } @diffs;
+                  map { $skip_table{lc $_->{db}}->{lc $_->{tbl}}++ }
+                     @$all_diffs;
                }
                else {
                   # This is a slave.
-                  @diffs = filter_diffs(
-                     \%skip_table,
-                     $databases,
-                     $tables,
-                     @diffs
+                  my $diffs = filter_diffs(
+                     diffs          => $all_diffs,
+                     SchemaIterator => $schema_iter,
+                     skip_table     => \%skip_table,
                   );
 
                   if ( $o->get('verbose') ) {
@@ -8089,7 +8091,7 @@
                            : ''));
                   }
 
-                  if ( @diffs ) {
+                  if ( $diffs && scalar @$diffs ) {
                      my $dst = {
                         dsn      => $dsn,
                         dbh      => $dbh,
@@ -8100,7 +8102,7 @@
 
                      lock_server(src => $src, dst => $dst, %args);
 
-                     foreach my $diff ( @diffs ) {
+                     foreach my $diff ( @$diffs ) {
                         $src->{db}  = $dst->{db}  = $diff->{db};
                         $src->{tbl} = $dst->{tbl} = $diff->{tbl};
 
@@ -8794,22 +8796,28 @@
 #   filters.  This sub is called in <sync_via_replication()> to implement
 #   schema object filters like --databases and --tables.
 #
-# Parameters:
-#   $skip_table - Hashref of databases and tables to skip
-#   $databases  - Hashref of databases to skip
-#   $tables     - Hashref of tables to skip
-#   @diffs      - Array of hashrefs, one for each different slave table
-#
 # Returns:
-#   Array of different slave tables that pass the filters
+#   Arrayref of different slave tables that pass the filters
 sub filter_diffs {
-   my ( $skip_table, $databases, $tables, @diffs ) = @_;
-   return grep {
-      my ($db, $tbl) = $q->split_unquote($_->{table});
-      !$skip_table->{$db}->{$tbl}
-      && (!$databases ||  $databases->{$db})
-      && (!$tables    || ($tables->{$tbl} || $tables->{$_->{table}}))
-   } @diffs;
+   my ( %args ) = @_;
+   my @required_args = qw(diffs SchemaIterator skip_table);
+   foreach my $arg ( @required_args ) {
+      die "I need a $arg argument" unless $args{$arg};
+   }
+   my ($diffs, $si, $skip_table) = @args{@required_args};
+
+   my @filtered_diffs;
+   foreach my $diff ( @$diffs ) {
+      my $db  = lc $diff->{db};
+      my $tbl = lc $diff->{tbl};
+      if (   !$skip_table->{$db}->{$tbl}
+           && $si->database_is_allowed($db)
+           && $si->table_is_allowed($db, $tbl) ) {
+         push @filtered_diffs, $diff;
+      }
+   }
+
+   return \@filtered_diffs;
 }
 
 # Sub: disconnect

=== modified file 'lib/TableChecksum.pm'
--- lib/TableChecksum.pm	2012-01-19 19:46:56 +0000
+++ lib/TableChecksum.pm	2012-05-21 21:57:26 +0000
@@ -480,10 +480,9 @@
       . "FROM $table "
       . "WHERE master_cnt <> this_cnt OR master_crc <> this_crc "
       . "OR ISNULL(master_crc) <> ISNULL(this_crc)";
-
    PTDEBUG && _d($sql);
    my $diffs = $dbh->selectall_arrayref($sql, { Slice => {} });
-   return @$diffs;
+   return $diffs;
 }
 
 sub _d {

=== modified file 't/pt-table-sync/filters.t'
--- t/pt-table-sync/filters.t	2011-12-22 22:43:15 +0000
+++ t/pt-table-sync/filters.t	2012-05-21 21:57:26 +0000
@@ -29,7 +29,7 @@
    plan skip_all => 'Cannot connect to sandbox slave';
 }
 else {
-   plan tests => 4;
+   plan tests => 8;
 }
 
 # Previous tests slave 12347 to 12346 which makes pt-table-checksum
@@ -106,6 +106,79 @@
 );
 
 # #############################################################################
+# pt-table-sync --ignore-* options don't work with --replicate 
+# https://bugs.launchpad.net/percona-toolkit/+bug/1002365
+# #############################################################################
+$sb->wipe_clean($master_dbh);
+
+$sb->load_file("master", "t/pt-table-sync/samples/simple-tbls.sql");
+PerconaTest::wait_for_table($slave_dbh, "test.mt1", "id=10");
+
+# Create a checksum diff in a table that we're going to ignore
+# when we sync.
+$slave_dbh->do("INSERT INTO test.empty_it VALUES (null,11,11,'eleven')");
+
+# Create the checksums.
+diag(`$trunk/bin/pt-table-checksum h=127.1,P=12345,u=msandbox,p=msandbox -d test --quiet --quiet --lock-wait-timeout 3 --max-load ''`);
+
+# Make sure all the tables were checksummed.
+my $rows = $master_dbh->selectall_arrayref("SELECT DISTINCT db, tbl FROM percona.checksums ORDER BY db, tbl");
+is_deeply(
+   $rows,
+   [ [qw(test empty_it) ],
+     [qw(test empty_mt) ],
+     [qw(test it1) ],
+     [qw(test it2) ],
+     [qw(test mt1) ],
+     [qw(test mt2) ],
+   ],
+   "Six checksum tables (bug 1002365)"
+);
+
+# Sync the checksummed tables, but ignore the table with the diff we created.
+$output = output(
+   sub { pt_table_sync::main("h=127.1,P=12346,u=msandbox,p=msandbox",
+      qw(--print --sync-to-master --replicate percona.checksums),
+      "--ignore-tables", "test.empty_it") },
+   stderr => 1,
+);
+
+is(
+   $output,
+   "",
+   "Table ignored, nothing to sync (bug 1002365)"
+);
+
+# Sync the checksummed tables, but ignore the database.
+$output = output(
+   sub { pt_table_sync::main("h=127.1,P=12346,u=msandbox,p=msandbox",
+      qw(--print --sync-to-master --replicate percona.checksums),
+      "--ignore-databases", "test") },
+   stderr => 1,
+);
+
+is(
+   $output,
+   "",
+   "Database ignored, nothing to sync (bug 1002365)"
+);
+
+# The same should work for just --sync-to-master.
+$output = output(
+   sub { pt_table_sync::main("h=127.1,P=12346,u=msandbox,p=msandbox",
+      qw(--print --sync-to-master),
+      "--ignore-tables", "test.empty_it",
+      "--ignore-databases", "percona") },
+   stderr => 1,
+);
+
+unlike(
+   $output,
+   qr/empty_it/,
+   "Table ignored, nothing to sync-to-master (bug 1002365)"
+);
+
+# #############################################################################
 # Done.
 # #############################################################################
 $sb->wipe_clean($master_dbh);