Merge lp:~percona-toolkit-dev/percona-toolkit/fix-ptc-bug-1210537 into lp:~percona-toolkit-dev/percona-toolkit/release-2.2.5

[Daniel Nichter]

[Daniel Nichter (daniel-nichter)]

review: Approve

=== modified file 'bin/pt-table-checksum'
--- bin/pt-table-checksum	2013-08-08 19:28:36 +0000
+++ bin/pt-table-checksum	2013-10-02 18:43:27 +0000
@@ -9208,15 +9208,24 @@
       }
 
       my $trimmed_nodes = Cxn->remove_duplicate_cxns(
-                                   cxns => [ $master_cxn, @$slaves ],
-                                );
+         cxns => [ $master_cxn, @$slaves ],
+      );
       ($master_cxn, @$slaves) = @$trimmed_nodes;
 
+      # If no slaves or nodes were found, and a recursion method was given
+      # (implicitly or explicitly), and that method is not none, then warn
+      # and continue but exit non-zero because there won't be any diffs but
+      # this could be a false-positive from having no slaves/nodes to check.
+      # https://bugs.launchpad.net/percona-toolkit/+bug/1210537
       PTDEBUG && _d(scalar @$slaves, 'slaves found');
-      if ( !@$slaves && $o->get('recursion-method')->[0] ne 'none' ) {
+      if ( !@$slaves
+           && (($o->get('recursion-method')->[0] || '') ne 'none'
+               || $autodiscover_cluster))
+      {
          $exit_status |= 1;
          if ( $o->get('quiet') < 2 ) {
-            warn "Diffs cannot be detected because no slaves were found.  "
+            my $type = $autodiscover_cluster ? 'cluster nodes' : 'slaves';
+            warn "Diffs cannot be detected because no $type were found.  "
                . "Please read the --recursion-method documentation for "
                . "information.\n";
          }

=== modified file 'lib/MasterSlave.pm'
--- lib/MasterSlave.pm	2013-04-12 15:58:10 +0000
+++ lib/MasterSlave.pm	2013-10-02 18:43:27 +0000
@@ -32,8 +32,18 @@
 sub check_recursion_method {
    my ($methods) = @_;
 
-   if ( @$methods != 1 ) {
-      if ( grep({ !m/processlist|hosts/i } @$methods)
+   # Check that each method is valid.
+   foreach my $method ( @$methods ) {
+      die "Invalid recursion method: " . ($method || 'undef') . "\n"
+         unless $method && $method =~ m/^(?:processlist$|hosts$|none$|cluster|dsn=)/i;
+   }
+
+   # Check for invalid combination of methods.
+   if ( @$methods > 1 ) {
+      if ( grep( { m/none/ } @$methods) && grep( {! m/none/ } @$methods) ) {
+            die "--recursion-method=none cannot be combined with other methods\n";
+      }
+      elsif ( grep({ !m/processlist|hosts/i } @$methods)
             && $methods->[0] !~ /^dsn=/i )
       {
          die  "Invalid combination of recursion methods: "
@@ -41,11 +51,8 @@
             . "Only hosts and processlist may be combined.\n"
       }
    }
-   else {
-      my ($method) = @$methods;
-      die "Invalid recursion method: " . ( $method || 'undef' )
-         unless $method && $method =~ m/^(?:processlist$|hosts$|none$|dsn=)/i;
-   }
+
+   return;
 }
 
 sub new {

=== modified file 't/lib/MasterSlave.t'
--- t/lib/MasterSlave.t	2012-08-14 23:08:06 +0000
+++ t/lib/MasterSlave.t	2013-10-02 18:43:27 +0000
@@ -11,10 +11,6 @@
 use English qw(-no_match_vars);
 use Test::More;
 
-if ( !$ENV{SLOW_TESTS} ) {
-   plan skip_all => "lib/MasterSlave.t is a top 5 slowest file; set SLOW_TESTS=1 to enable it.";
-}
-
 use MasterSlave;
 use DSNParser;
 use VersionParser;
@@ -709,28 +705,51 @@
 # ############################################################################
 # Invalid recursion methods are caught
 # ############################################################################
-local $EVAL_ERROR;
 eval {
    MasterSlave::check_recursion_method([qw(stuff)])
 };
-
 like(
    $EVAL_ERROR,
    qr/Invalid recursion method: stuff/,
    "--recursion-method stuff causes error"
 );
 
-local $EVAL_ERROR;
 eval {
    MasterSlave::check_recursion_method([qw(processlist stuff)])
 };
-
 like(
    $EVAL_ERROR,
-   qr/Invalid combination of recursion methods: processlist, stuff/,
+   qr/Invalid recursion method: stuff/,
    "--recursion-method processlist,stuff causes error",
 );
 
+eval {
+   MasterSlave::check_recursion_method([qw(none hosts)])
+};
+like(
+   $EVAL_ERROR,
+   qr/none cannot be combined with other methods/,
+   "--recursion-method none,hosts"
+);
+
+eval {
+   MasterSlave::check_recursion_method([qw(cluster none)])
+};
+like(
+   $EVAL_ERROR,
+   qr/none cannot be combined with other methods/,
+   "--recursion-method cluster,none"
+);
+
+eval {
+   MasterSlave::check_recursion_method([qw(none none)])
+};
+like(
+   $EVAL_ERROR,
+   qr/Invalid combination of recursion methods: none, none/,
+   "--recursion-method none,none"
+);
+
 # #############################################################################
 # Done.
 # #############################################################################

=== modified file 't/pt-table-checksum/bugs.t'
--- t/pt-table-checksum/bugs.t	2013-03-02 02:02:13 +0000
+++ t/pt-table-checksum/bugs.t	2013-10-02 18:43:27 +0000
@@ -273,6 +273,28 @@
 }
 
 # #############################################################################
+# pt-table-checksum --recursion-method cluster crashes
+# https://bugs.launchpad.net/percona-toolkit/+bug/1210537
+# #############################################################################
+
+$output = output(sub {
+   pt_table_checksum::main($master_dsn,
+      qw(--recursion-method cluster -t mysql.user)
+   )},
+   stderr => 1,
+);
+unlike(
+   $output,
+   qr/uninitialized value/,
+   "Bug 1210537: no crash with --recursion-method cluster"
+);
+like(
+   $output,
+   qr/mysql.user/,
+   "Bug 1210537: tool ran"
+);
+
+# #############################################################################
 # Done.
 # #############################################################################
 $sb->wipe_clean($master_dbh);