Merge lp:~percona-toolkit-dev/percona-toolkit/empty-table-bug-987393-2.0 into lp:percona-toolkit/2.0

[Daniel Nichter]

[Daniel Nichter (daniel-nichter)]

review: Approve

=== modified file 'bin/pt-table-checksum'
--- bin/pt-table-checksum	2012-05-08 23:21:47 +0000
+++ bin/pt-table-checksum	2012-05-10 16:49:30 +0000
@@ -3530,8 +3530,9 @@
    
    my $where = $o->get('where');
    my ($row_est, $mysql_index) = get_row_estimate(%args, where => $where);
+   my $chunk_size_limit = $o->get('chunk-size-limit') || 1;
    my $one_nibble = !defined $args{one_nibble} || $args{one_nibble}
-                  ? $row_est <= $chunk_size * $o->get('chunk-size-limit')
+                  ? $row_est <= $chunk_size * $chunk_size_limit
                   : 0;
    PTDEBUG && _d('One nibble:', $one_nibble ? 'yes' : 'no');
 
@@ -6283,10 +6284,29 @@
    my $total_rows = 0;
    my $total_time = 0;
    my $total_rate = 0;
-   my $limit      = $o->get('chunk-size-limit');
    my $tn         = new TableNibbler(TableParser => $tp, Quoter => $q);
    my $retry      = new Retry();
 
+
+   # --chunk-size-limit has two purposes.  The 1st, as documented, is
+   # to prevent oversized chunks when the chunk index is not unique.
+   # The 2nd is to determine if the table can be processed in one chunk
+   # (WHERE 1=1 instead of nibbling).  This creates a problem when
+   # the user does --chunk-size-limit=0 to disable the 1st, documented
+   # purpose because, apparently, they're using non-unique indexes and
+   # they don't care about potentially large chunks.  But disabling the
+   # 1st purpose adversely affects the 2nd purpose becuase 0 * the chunk size
+   # will always be zero, so tables will only be single-chunked if EXPLAIN
+   # says there are 0 rows, but sometimes EXPLAIN says there is 1 row
+   # even when the table is empty.  This wouldn't matter except that nibbling
+   # an empty table doesn't currently work becuase there are no boundaries,
+   # so no checksum is written for the empty table.  To fix this and
+   # preserve the two purposes of this option, usages of the 2nd purpose
+   # do || 1 so the limit is never 0 and empty tables are single-chunked.
+   # See: https://bugs.launchpad.net/percona-toolkit/+bug/987393
+   # This is used for the 1st purpose of --chunk-size-limit:
+   my $limit = $o->get('chunk-size-limit');
+
    # ########################################################################
    # Callbacks for each table's nibble iterator.  All checksum work is done
    # in these callbacks and the subs that they call.
@@ -6339,7 +6359,11 @@
          else {
             if ( $nibble_iter->one_nibble() ) {
                PTDEBUG && _d('Getting table row estimate on replicas');
-               my $chunk_size_limit = $o->get('chunk-size-limit');
+               
+               # This is used for the 2nd purpose for --chunkr-size-limit;
+               # see the large comment block above near my $limit = $o->...
+               my $chunk_size_limit = $o->get('chunk-size-limit') || 1;
+
                my @too_large;
                foreach my $slave ( @$slaves ) {
                   my ($n_rows) = NibbleIterator::get_row_estimate(

=== modified file 'lib/NibbleIterator.pm'
--- lib/NibbleIterator.pm	2012-05-08 23:21:47 +0000
+++ lib/NibbleIterator.pm	2012-05-10 16:49:30 +0000
@@ -64,8 +64,9 @@
    
    my $where = $o->get('where');
    my ($row_est, $mysql_index) = get_row_estimate(%args, where => $where);
+   my $chunk_size_limit = $o->get('chunk-size-limit') || 1;
    my $one_nibble = !defined $args{one_nibble} || $args{one_nibble}
-                  ? $row_est <= $chunk_size * $o->get('chunk-size-limit')
+                  ? $row_est <= $chunk_size * $chunk_size_limit
                   : 0;
    PTDEBUG && _d('One nibble:', $one_nibble ? 'yes' : 'no');
 

=== modified file 't/lib/OobNibbleIterator.t'
--- t/lib/OobNibbleIterator.t	2011-12-30 16:23:41 +0000
+++ t/lib/OobNibbleIterator.t	2012-05-10 16:49:30 +0000
@@ -41,7 +41,7 @@
    plan skip_all => 'Cannot connect to sandbox master';
 }
 else {
-   plan tests => 15;
+   plan tests => 16;
 }
 
 my $q   = new Quoter();
@@ -248,6 +248,29 @@
 );
 
 # #############################################################################
+# Empty table
+# https://bugs.launchpad.net/percona-toolkit/+bug/987393 
+# #############################################################################
+$sb->load_file('master', "t/pt-table-checksum/samples/empty-table-bug-987393.sql");
+PerconaTest::wait_for_table($dbh, "test.test_full", "id=1");
+
+$ni = make_nibble_iter(
+   db       => 'test',
+   tbl      => 'test_empty',
+   argv     => [qw(--databases test --chunk-size-limit 0)],
+);
+
+@rows = ();
+for (1..5) {
+   push @rows, $ni->next();
+}
+is_deeply(
+   \@rows,
+   [],
+   "Empty table"
+);
+
+# #############################################################################
 # Done.
 # #############################################################################
 {

=== modified file 't/pt-table-checksum/bugs.t'
--- t/pt-table-checksum/bugs.t	2012-05-08 23:16:36 +0000
+++ t/pt-table-checksum/bugs.t	2012-05-10 16:49:30 +0000
@@ -38,7 +38,7 @@
    plan skip_all => 'Cannot connect to sandbox slave1';
 }
 else {
-   plan tests => 2;
+   plan tests => 5;
 }
 
 # The sandbox servers run with lock_wait_timeout=3 and it's not dynamic
@@ -78,6 +78,42 @@
 );
 
 # #############################################################################
+# https://bugs.launchpad.net/percona-toolkit/+bug/987393
+# Empy tables cause "undefined value as an ARRAY" errors
+# #############################################################################
+$master_dbh->do("DROP DATABASE IF EXISTS percona");  # clear old checksums
+$sb->load_file('master', "$sample/empty-table-bug-987393.sql");
+PerconaTest::wait_for_table($slave_dbh, "test.test_full", "id=1");
+
+$output = output(
+   sub { $exit_status = pt_table_checksum::main(
+      @args, qw(-d test --chunk-size-limit 0)) },
+   stderr => 1,
+);
+
+is(
+   $exit_status,
+   0,
+   "Bug 987393 (empty table): zero exit status"
+);
+
+is(
+   PerconaTest::count_checksum_results($output, 'errors'),
+   0,
+   "Bug 987393 (empty table): no errors"
+);
+
+my $rows = $master_dbh->selectall_arrayref("SELECT db, tbl, chunk, master_crc, master_cnt FROM percona.checksums ORDER BY db, tbl, chunk");
+is_deeply(
+   $rows,
+   [
+      ['test', 'test_empty', '1', '0',        '0'],  # empty
+      ['test', 'test_full',  '1', 'ac967054', '1'],  # row
+   ],
+   "Bug 987393 (empty table): checksums"
+) or print STDERR Dumper($rows);
+
+# #############################################################################
 # Done.
 # #############################################################################
 $sb->wipe_clean($master_dbh);

=== added file 't/pt-table-checksum/samples/empty-table-bug-987393.sql'
--- t/pt-table-checksum/samples/empty-table-bug-987393.sql	1970-01-01 00:00:00 +0000
+++ t/pt-table-checksum/samples/empty-table-bug-987393.sql	2012-05-10 16:49:30 +0000
@@ -0,0 +1,18 @@
+DROP DATABASE IF EXISTS test;
+CREATE DATABASE test;
+USE test;
+
+CREATE TABLE `test_empty` (
+  `id` int(11) NOT NULL AUTO_INCREMENT,
+  PRIMARY KEY (`id`)
+) ENGINE=InnoDB DEFAULT CHARSET=utf8 AUTO_INCREMENT=1 ;
+
+
+CREATE TABLE `test_full` (
+  `id` int(11) NOT NULL AUTO_INCREMENT,
+  `d` text NOT NULL,
+  PRIMARY KEY (`id`)
+) ENGINE=InnoDB  DEFAULT CHARSET=utf8 AUTO_INCREMENT=2 ;
+
+INSERT INTO `test_full` (`id`, `d`) VALUES
+(1, '2');