Merge lp:~percona-toolkit-dev/percona-toolkit/fix-ptc-pk-bug-978432 into lp:percona-toolkit/2.1

[Daniel Nichter]

[Daniel Nichter (daniel-nichter)]

review: Approve

=== modified file 'bin/pt-table-checksum'
--- bin/pt-table-checksum	2012-05-04 16:17:01 +0000
+++ bin/pt-table-checksum	2012-05-08 18:46:21 +0000
@@ -3858,12 +3858,18 @@
    }
    my ($cxn, $tbl, $chunk_size, $o) = @args{@required_args};
 
+   my $where = $o->has('where') ? $o->get('where') : '';
+
    my ($row_est, $mysql_index) = get_row_estimate(
       Cxn   => $cxn,
       tbl   => $tbl,
-      where => $o->has('where') ? $o->get('where') : '',
+      where => $where,
    );
 
+   if ( !$where ) {
+      $mysql_index = undef;
+   }
+
    my $one_nibble = !defined $args{one_nibble} || $args{one_nibble}
                   ? $row_est <= $chunk_size * $o->get('chunk-size-limit')
                   : 0;
@@ -6387,9 +6393,6 @@
                my $chunk_size_limit = $o->get('chunk-size-limit');
                my @too_large;
                foreach my $slave ( @$slaves ) {
-                  # get_row_estimate() returns (row_est, index), but
-                  # we only need the row_est.  Maybe in the future we'll
-                  # care what index MySQL will use on a slave.
                   my ($n_rows) = NibbleIterator::get_row_estimate(
                      Cxn   => $slave,
                      tbl   => $tbl,

=== modified file 'lib/NibbleIterator.pm'
--- lib/NibbleIterator.pm	2012-04-03 15:33:10 +0000
+++ lib/NibbleIterator.pm	2012-05-08 18:46:21 +0000
@@ -428,13 +428,26 @@
    }
    my ($cxn, $tbl, $chunk_size, $o) = @args{@required_args};
 
+   my $where = $o->has('where') ? $o->get('where') : '';
+
    # About how many rows are there?
    my ($row_est, $mysql_index) = get_row_estimate(
       Cxn   => $cxn,
       tbl   => $tbl,
-      where => $o->has('where') ? $o->get('where') : '',
+      where => $where,
    );
 
+   # MySQL's chosen index is only something we should prefer
+   # if --where is used.  Else, we can chose our own index
+   # and disregard the MySQL index from the row estimate.
+   # If there's a --where, however, then MySQL's chosen index
+   # is used because it tells us how MySQL plans to optimize
+   # for the --where.
+   # https://bugs.launchpad.net/percona-toolkit/+bug/978432
+   if ( !$where ) {
+      $mysql_index = undef;
+   }
+
    # Can all those rows be nibbled in one chunk?  If one_nibble is defined,
    # then do as it says; else, look at the chunk size limit.
    my $one_nibble = !defined $args{one_nibble} || $args{one_nibble}

=== modified file 't/pt-table-checksum/chunk_index.t'
--- t/pt-table-checksum/chunk_index.t	2012-02-03 18:38:20 +0000
+++ t/pt-table-checksum/chunk_index.t	2012-05-08 18:46:21 +0000
@@ -25,7 +25,7 @@
    plan skip_all => 'Cannot connect to sandbox master';
 }
 else {
-   plan tests => 10;
+   plan tests => 11;
 }
 
 # The sandbox servers run with lock_wait_timeout=3 and it's not dynamic
@@ -142,6 +142,22 @@
 );
 
 # #############################################################################
+# Bug 978432: PK is ignored
+# #############################################################################
+$sb->load_file('master', "t/pt-table-checksum/samples/not-using-pk-bug.sql");
+PerconaTest::wait_for_table($dbh, "test.multi_resource_apt", "apt_id=4 AND res_id=4");
+
+ok(
+   no_diff(
+      sub { pt_table_checksum::main(@args,
+         qw(-t test.multi_resource_apt --chunk-size 2 --explain --explain))
+      },
+      "t/pt-table-checksum/samples/not-using-pk-bug.out",
+   ),
+   "Smarter chunk index selection (bug 978432)"
+);
+
+# #############################################################################
 # Done.
 # #############################################################################
 $sb->wipe_clean($dbh);

=== added file 't/pt-table-checksum/samples/not-using-pk-bug.out'
--- t/pt-table-checksum/samples/not-using-pk-bug.out	1970-01-01 00:00:00 +0000
+++ t/pt-table-checksum/samples/not-using-pk-bug.out	2012-05-08 18:46:21 +0000
@@ -0,0 +1,20 @@
+--
+-- test.multi_resource_apt
+--
+
+REPLACE INTO `percona`.`checksums` (db, tbl, chunk, chunk_index, lower_boundary, upper_boundary, this_cnt, this_crc) SELECT ?, ?, ?, ?, ?, ?, COUNT(*) AS cnt, COALESCE(LOWER(CONV(BIT_XOR(CAST(CRC32(CONCAT_WS('#', `apt_id`, `res_id`)) AS UNSIGNED)), 10, 16)), 0) AS crc FROM `test`.`multi_resource_apt` FORCE INDEX(`PRIMARY`) WHERE ((`apt_id` > ?) OR (`apt_id` = ? AND `res_id` >= ?)) AND ((`apt_id` < ?) OR (`apt_id` = ? AND `res_id` <= ?)) /*checksum chunk*/
+
+REPLACE INTO `percona`.`checksums` (db, tbl, chunk, chunk_index, lower_boundary, upper_boundary, this_cnt, this_crc) SELECT ?, ?, ?, ?, ?, ?, COUNT(*), '0' FROM `test`.`multi_resource_apt` FORCE INDEX(`PRIMARY`) WHERE ((`apt_id` < ?) OR (`apt_id` = ? AND `res_id` < ?)) ORDER BY `apt_id`, `res_id` /*past lower chunk*/
+
+REPLACE INTO `percona`.`checksums` (db, tbl, chunk, chunk_index, lower_boundary, upper_boundary, this_cnt, this_crc) SELECT ?, ?, ?, ?, ?, ?, COUNT(*), '0' FROM `test`.`multi_resource_apt` FORCE INDEX(`PRIMARY`) WHERE ((`apt_id` > ?) OR (`apt_id` = ? AND `res_id` > ?)) ORDER BY `apt_id`, `res_id` /*past upper chunk*/
+
+SELECT /*!40001 SQL_NO_CACHE */ `apt_id`, `apt_id`, `res_id` FROM `test`.`multi_resource_apt` FORCE INDEX(`PRIMARY`) WHERE ((`apt_id` > ?) OR (`apt_id` = ? AND `res_id` >= ?)) ORDER BY `apt_id`, `res_id` LIMIT ?, 2 /*next chunk boundary*/
+
+1 1,1,1 2,2,1
+2 2,2,2 3,3,1
+3 3,3,2 3,3,3
+4 4,4,1 4,4,2
+5 4,4,3 4,4,4
+6  1,1,1
+7 4,4,4 
+

=== added file 't/pt-table-checksum/samples/not-using-pk-bug.sql'
--- t/pt-table-checksum/samples/not-using-pk-bug.sql	1970-01-01 00:00:00 +0000
+++ t/pt-table-checksum/samples/not-using-pk-bug.sql	2012-05-08 18:46:21 +0000
@@ -0,0 +1,20 @@
+DROP DATABASE IF EXISTS test;
+CREATE DATABASE test;
+USE test;
+CREATE TABLE `multi_resource_apt` (
+  `apt_id` int(10) unsigned NOT NULL DEFAULT '0',
+  `res_id` int(10) unsigned NOT NULL DEFAULT '0',
+  PRIMARY KEY (`apt_id`,`res_id`),
+  KEY `resid` (`res_id`)
+) ENGINE=InnoDB;
+INSERT INTO multi_resource_apt VALUES
+  (1, 1),
+  (2, 1),
+  (2, 2),
+  (3, 1),
+  (3, 2),
+  (3, 3),
+  (4, 1),
+  (4, 2),
+  (4, 3),
+  (4, 4);