diff --git a/bin/pt-table-checksum b/bin/pt-table-checksum index 135f9888..472f29f9 100755 --- a/bin/pt-table-checksum +++ b/bin/pt-table-checksum @@ -3858,12 +3858,18 @@ sub can_nibble { } 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 @@ sub main { 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, diff --git a/lib/NibbleIterator.pm b/lib/NibbleIterator.pm index 2730fe6a..6e446f4c 100644 --- a/lib/NibbleIterator.pm +++ b/lib/NibbleIterator.pm @@ -428,13 +428,26 @@ sub can_nibble { } 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} diff --git a/t/pt-table-checksum/chunk_index.t b/t/pt-table-checksum/chunk_index.t index 3aae20f7..48bab4e5 100644 --- a/t/pt-table-checksum/chunk_index.t +++ b/t/pt-table-checksum/chunk_index.t @@ -25,7 +25,7 @@ if ( !$dbh ) { 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 @@ -141,6 +141,22 @@ is( "14 rows checksummed (bug 925855)" ); +# ############################################################################# +# 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. # ############################################################################# diff --git a/t/pt-table-checksum/samples/not-using-pk-bug.out b/t/pt-table-checksum/samples/not-using-pk-bug.out new file mode 100644 index 00000000..f9640d33 --- /dev/null +++ b/t/pt-table-checksum/samples/not-using-pk-bug.out @@ -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 + diff --git a/t/pt-table-checksum/samples/not-using-pk-bug.sql b/t/pt-table-checksum/samples/not-using-pk-bug.sql new file mode 100644 index 00000000..b299513e --- /dev/null +++ b/t/pt-table-checksum/samples/not-using-pk-bug.sql @@ -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);