From 1e3cc6131c5f29891a8effb4d13c71e56c886328 Mon Sep 17 00:00:00 2001 From: "Brian Fraser fraserb@gmail.com" <> Date: Tue, 30 Oct 2012 11:55:49 -0300 Subject: [PATCH 1/5] Fix for 938660: pt-table-checksum chunk-size-limit of 0 does not disable chunk size limit checking --- bin/pt-table-checksum | 14 ++++++-------- t/pt-table-checksum/basics.t | 23 ++++++++++++++++++++--- 2 files changed, 26 insertions(+), 11 deletions(-) diff --git a/bin/pt-table-checksum b/bin/pt-table-checksum index bad2d08e..cc459877 100755 --- a/bin/pt-table-checksum +++ b/bin/pt-table-checksum @@ -5831,7 +5831,7 @@ sub can_nibble { $mysql_index = undef; } - my $chunk_size_limit = $o->get('chunk-size-limit') || 1; + my $chunk_size_limit = $o->get('chunk-size-limit'); my $one_nibble = !defined $args{one_nibble} || $args{one_nibble} ? $row_est <= $chunk_size * $chunk_size_limit : 0; @@ -8878,7 +8878,8 @@ sub main { # 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'); + my $limit = $o->get('chunk-size-limit') || 1; + my $chunk_size_limit = $o->get('chunk-size-limit'); # ######################################################################## # Callbacks for each table's nibble iterator. All checksum work is done @@ -8934,10 +8935,6 @@ sub main { if ( $nibble_iter->one_nibble() ) { PTDEBUG && _d('Getting table row estimate on replicas'); - # 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 ) { # TODO: This duplicates NibbleIterator::can_nibble(); @@ -8957,7 +8954,7 @@ sub main { push @too_large, [$slave->name(), $n_rows || 0]; } } - if ( @too_large ) { + if ( @too_large && $chunk_size_limit ) { if ( $o->get('quiet') < 2 ) { my $msg = "Skipping table $tbl->{db}.$tbl->{tbl} because" @@ -9054,7 +9051,8 @@ sub main { vals => [ @{$boundary->{lower}}, $nibble_iter->chunk_size() ], ); if ( lc($expl->{key} || '') - ne lc($nibble_iter->nibble_index() || '') ) { + ne lc($nibble_iter->nibble_index() || '') + && $chunk_size_limit) { PTDEBUG && _d('Cannot nibble next chunk, aborting table'); if ( $o->get('quiet') < 2 ) { warn ts("Aborting table $tbl->{db}.$tbl->{tbl} at chunk " diff --git a/t/pt-table-checksum/basics.t b/t/pt-table-checksum/basics.t index 585d981c..1894a79f 100644 --- a/t/pt-table-checksum/basics.t +++ b/t/pt-table-checksum/basics.t @@ -41,9 +41,6 @@ elsif ( !$slave1_dbh ) { elsif ( !@{$master_dbh->selectall_arrayref("show databases like 'sakila'")} ) { plan skip_all => 'sakila database is not loaded'; } -else { - plan tests => 38; -} # The sandbox servers run with lock_wait_timeout=3 and it's not dynamic # so we need to specify --lock-wait-timeout=3 else the tool will die. @@ -334,6 +331,24 @@ is( "52 rows checksummed" ); +# ############################################################################# +# pt-table-checksum chunk-size-limit of 0 does not disable chunk size limit checking +# https://bugs.launchpad.net/percona-toolkit/+bug/938660 +# ############################################################################# + +$output = output( + sub { + $exit_status = pt_table_checksum::main(@args, qw(-d test --chunk-size 2 --chunk-size-limit 0)) + }, + stderr => 1, +); + +unlike( + $output, + qr/Skipping table test.t1/, + "Doesn't warns about skipping a large slave table if --chunk-size-limit 0" +); + # ############################################################################# # Crash if no host in DSN. # https://bugs.launchpad.net/percona-toolkit/+bug/819450 @@ -481,4 +496,6 @@ is( # ############################################################################# $sb->wipe_clean($master_dbh); ok($sb->ok(), "Sandbox servers") or BAIL_OUT(__FILE__ . " broke the sandbox"); + +done_testing; exit; From 9424ff7003d39d917e70913212846e2a3acd979d Mon Sep 17 00:00:00 2001 From: Brian Fraser Date: Tue, 30 Oct 2012 16:12:02 -0300 Subject: [PATCH 2/5] Removed a spurious change in the previous commit --- bin/pt-table-checksum | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/pt-table-checksum b/bin/pt-table-checksum index cc459877..d0624aee 100755 --- a/bin/pt-table-checksum +++ b/bin/pt-table-checksum @@ -5831,7 +5831,7 @@ sub can_nibble { $mysql_index = undef; } - my $chunk_size_limit = $o->get('chunk-size-limit'); + my $chunk_size_limit = $o->get('chunk-size-limit') || 1; my $one_nibble = !defined $args{one_nibble} || $args{one_nibble} ? $row_est <= $chunk_size * $chunk_size_limit : 0; From ce3c80ea45b2f6599a9f7beb9e2ecd2c79e749cf Mon Sep 17 00:00:00 2001 From: Daniel Nichter Date: Wed, 31 Oct 2012 10:31:59 -0600 Subject: [PATCH 3/5] Remove --chunk-size-limit on index check. If nibbling, the indexes must match, so --chunk-size-limit isn't a factor here. --- bin/pt-table-checksum | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/bin/pt-table-checksum b/bin/pt-table-checksum index d0624aee..9876f9fc 100755 --- a/bin/pt-table-checksum +++ b/bin/pt-table-checksum @@ -9051,8 +9051,7 @@ sub main { vals => [ @{$boundary->{lower}}, $nibble_iter->chunk_size() ], ); if ( lc($expl->{key} || '') - ne lc($nibble_iter->nibble_index() || '') - && $chunk_size_limit) { + ne lc($nibble_iter->nibble_index() || '') ) { PTDEBUG && _d('Cannot nibble next chunk, aborting table'); if ( $o->get('quiet') < 2 ) { warn ts("Aborting table $tbl->{db}.$tbl->{tbl} at chunk " From b5c48f07d13d507339127b5b2d48bebeb210e807 Mon Sep 17 00:00:00 2001 From: Daniel Nichter Date: Wed, 31 Oct 2012 12:12:51 -0600 Subject: [PATCH 4/5] Remove unused $limit. Make $chunk_size_limit || 1 because it's the one used for #-of-rows check on slaves. Reverse the new test because we don't want to disable the #-of-rows check. --- bin/pt-table-checksum | 14 +++++++------- t/pt-table-checksum/basics.t | 15 +++++++++------ 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/bin/pt-table-checksum b/bin/pt-table-checksum index 9876f9fc..b22d51b1 100755 --- a/bin/pt-table-checksum +++ b/bin/pt-table-checksum @@ -8876,10 +8876,12 @@ sub main { # 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') || 1; - my $chunk_size_limit = $o->get('chunk-size-limit'); + # See: + # https://bugs.launchpad.net/percona-toolkit/+bug/987393 + # https://bugs.launchpad.net/percona-toolkit/+bug/938660 + # https://bugs.launchpad.net/percona-toolkit/+bug/987495 + # This is used for the 2nd purpose: + my $chunk_size_limit = $o->get('chunk-size-limit') || 1; # ######################################################################## # Callbacks for each table's nibble iterator. All checksum work is done @@ -9486,9 +9488,7 @@ sub nibble_is_safe { # shouldn't have changed. If chunking the table with a non-unique key, # oversize chunks are possible. if ( my $limit = $o->get('chunk-size-limit') ) { - my $oversize_chunk - = $limit ? ($expl->{rows} || 0) >= $tbl->{chunk_size} * $limit - : 0; + my $oversize_chunk = ($expl->{rows} || 0) >= $tbl->{chunk_size} * $limit; if ( $oversize_chunk && $nibble_iter->identical_boundaries($boundary->{upper}, $boundary->{next_lower}) ) { diff --git a/t/pt-table-checksum/basics.t b/t/pt-table-checksum/basics.t index 1894a79f..88af4f9b 100644 --- a/t/pt-table-checksum/basics.t +++ b/t/pt-table-checksum/basics.t @@ -332,21 +332,26 @@ is( ); # ############################################################################# -# pt-table-checksum chunk-size-limit of 0 does not disable chunk size limit checking +# pt-table-checksum chunk-size-limit of 0 does not disable chunk size limit +# checking # https://bugs.launchpad.net/percona-toolkit/+bug/938660 # ############################################################################# +# Decided _not_ to do this; we want to always check slave table size when +# single-chunking a table on the master. + $output = output( sub { - $exit_status = pt_table_checksum::main(@args, qw(-d test --chunk-size 2 --chunk-size-limit 0)) + $exit_status = pt_table_checksum::main(@args, + qw(-d test --chunk-size 2 --chunk-size-limit 0)) }, stderr => 1, ); -unlike( +like( $output, qr/Skipping table test.t1/, - "Doesn't warns about skipping a large slave table if --chunk-size-limit 0" + "--chunk-size-limit=0 does not disable #-of-rows checks on slaves" ); # ############################################################################# @@ -496,6 +501,4 @@ is( # ############################################################################# $sb->wipe_clean($master_dbh); ok($sb->ok(), "Sandbox servers") or BAIL_OUT(__FILE__ . " broke the sandbox"); - done_testing; -exit; From 7c23608603f15a5dd9dda79258fc068eb56c0149 Mon Sep 17 00:00:00 2001 From: Daniel Nichter Date: Wed, 31 Oct 2012 12:17:02 -0600 Subject: [PATCH 5/5] Always report slaves with tables too large, regardless of --chunk-size-limit. --- bin/pt-table-checksum | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/pt-table-checksum b/bin/pt-table-checksum index b22d51b1..209ad1a4 100755 --- a/bin/pt-table-checksum +++ b/bin/pt-table-checksum @@ -8956,7 +8956,7 @@ sub main { push @too_large, [$slave->name(), $n_rows || 0]; } } - if ( @too_large && $chunk_size_limit ) { + if ( @too_large ) { if ( $o->get('quiet') < 2 ) { my $msg = "Skipping table $tbl->{db}.$tbl->{tbl} because"