Code review comment for lp:~percona-toolkit-dev/percona-toolkit/pt-kill-log-dsn

Revision history for this message
Daniel Nichter (daniel-nichter) wrote :

Changes for

bug 941469

branch https://code.launchpad.net/~percona-toolkit-dev/percona-toolkit/pt-kill-reconnect-bug-941469

merge https://code.launchpad.net/~percona-toolkit-dev/percona-toolkit/pt-kill-reconnect-bug-941469/+merge/114748

conflict with the --log-dsn code. The code needs to be updated like those ^ changes, i.e. use Retry to try doing the INSERT, if that fails, reconnect and try again. I would say: tries=20, wait 3s (i.e. 1 minute). MySQL shouldn't stay away for long if the code just observed it, and if an INSERT fails that many times, it's no big deal, but it's worth making a good effort.

Also, please standardize the tests:

* Use English and indention,

is(
   $EVAL_ERROR,
   "",
   "foo"
);

* Be more explicit, e.g.:

   my $result = shift @$results;
   $result->[7] =~ s/localhost:[0-9]+/localhost/;
   is_deeply(
      [ @{$result}[6..9, 11, 12] ],

That's cryptic. Rather:

my $row = $dbh->selectrow_hashref($sql);
is_deeply(
   $row,
   {
      Id => 123,
      user => 'foo',
      ...
   },
   "..."
) or diag(Dumper($row));

« Back to merge proposal