From 1875c926748136da8e51fae10e5d161e67cda75a Mon Sep 17 00:00:00 2001 From: Daniel Nichter Date: Fri, 27 Jul 2012 11:16:41 -0600 Subject: [PATCH 1/3] Partial bug fix: report max diffs, not just last slave's diffs. Still needs fixing to report count of all unique chunk# diffs. --- bin/pt-table-checksum | 7 ++++-- t/pt-table-checksum/bugs.t | 36 ++++++++++++++++++++++++++--- t/pt-table-checksum/samples/a-z.sql | 10 ++++++++ 3 files changed, 48 insertions(+), 5 deletions(-) create mode 100644 t/pt-table-checksum/samples/a-z.sql diff --git a/bin/pt-table-checksum b/bin/pt-table-checksum index bb75b65a..ac4ee8c5 100755 --- a/bin/pt-table-checksum +++ b/bin/pt-table-checksum @@ -7690,6 +7690,7 @@ sub main { ); # Check each slave for checksum diffs. + my $max_diffs = 0; foreach my $slave ( @$slaves ) { eval { my $diffs = $rc->find_replication_differences( @@ -7699,8 +7700,9 @@ sub main { ); PTDEBUG && _d(scalar @$diffs, 'checksum diffs on', $slave->name()); - if ( @$diffs ) { - $tbl->{checksum_results}->{diffs} = scalar @$diffs; + if ( my $n_diffs = scalar @$diffs ) { + # https://bugs.launchpad.net/percona-toolkit/+bug/1030031 + $max_diffs = $n_diffs if $n_diffs > $max_diffs; } }; if ($EVAL_ERROR) { @@ -7714,6 +7716,7 @@ sub main { $tbl->{checksum_results}->{errors}++; } } + $tbl->{checksum_results}->{diffs} = $max_diffs; } # Print table's checksum results if we're not being quiet, diff --git a/t/pt-table-checksum/bugs.t b/t/pt-table-checksum/bugs.t index 431a9e18..e0149c9a 100644 --- a/t/pt-table-checksum/bugs.t +++ b/t/pt-table-checksum/bugs.t @@ -29,16 +29,20 @@ require "$trunk/bin/pt-table-checksum"; my $dp = new DSNParser(opts=>$dsn_opts); my $sb = new Sandbox(basedir => '/tmp', DSNParser => $dp); my $master_dbh = $sb->get_dbh_for('master'); -my $slave_dbh = $sb->get_dbh_for('slave1'); +my $slave1_dbh = $sb->get_dbh_for('slave1'); +my $slave2_dbh = $sb->get_dbh_for('slave2'); if ( !$master_dbh ) { plan skip_all => 'Cannot connect to sandbox master'; } -elsif ( !$slave_dbh ) { +elsif ( !$slave1_dbh ) { plan skip_all => 'Cannot connect to sandbox slave1'; } +elsif ( !$slave2_dbh ) { + plan skip_all => 'Cannot connect to sandbox slave2'; +} else { - plan tests => 8; + plan tests => 9; } # The sandbox servers run with lock_wait_timeout=3 and it's not dynamic @@ -134,6 +138,32 @@ is( "Bug 987393 (Perl 5.8 scoping): checksummed table" ); +# ############################################################################# +# https://bugs.launchpad.net/percona-toolkit/+bug/1030031 +# pt-table-checksum reports wrong number of DIFFS +# ############################################################################# +$sb->load_file('master', "$sample/a-z.sql"); +$sb->wait_for_slaves(); + +# Create 2 diffs on slave1 and 1 diff on slave2. +$slave1_dbh->do("UPDATE test.t SET c='' WHERE id=5"); # diff on slave1 & 2 +$slave1_dbh->do("SET SQL_LOG_BIN=0"); +$slave1_dbh->do("UPDATE test.t SET c='' WHERE id=20"); # diff only on slave1 + +# Restore sql_log_bin on slave1 in case later tests use it. +$slave1_dbh->do("SET SQL_LOG_BIN=1"); + +$output = output( + sub { pt_table_checksum::main(@args, qw(-t test.t --chunk-size 10)) }, + stderr => 1, +); + +is( + PerconaTest::count_checksum_results($output, 'diffs'), + 2, + "Bug 1030031 (wrong DIFFS): 2 diffs" +); + # ############################################################################# # Done. # ############################################################################# diff --git a/t/pt-table-checksum/samples/a-z.sql b/t/pt-table-checksum/samples/a-z.sql new file mode 100644 index 00000000..48045580 --- /dev/null +++ b/t/pt-table-checksum/samples/a-z.sql @@ -0,0 +1,10 @@ +drop database if exists test; +create database test; +use test; + +create table t ( + id int auto_increment primary key, + c varchar(16) not null +) engine=innodb; + +insert into t values (null, 'a'),(null, 'b'),(null, 'c'),(null, 'd'),(null, 'e'),(null, 'f'),(null, 'g'),(null, 'h'),(null, 'i'),(null, 'j'),(null, 'k'),(null, 'l'),(null, 'm'),(null, 'n'),(null, 'o'),(null, 'p'),(null, 'q'),(null, 'r'),(null, 's'),(null, 't'),(null, 'u'),(null, 'v'),(null, 'w'),(null, 'x'),(null, 'y'),(null, 'z'); From 203b0a9cb6f645c84c8fe62214baf7b63beb276f Mon Sep 17 00:00:00 2001 From: Daniel Nichter Date: Fri, 27 Jul 2012 11:30:33 -0600 Subject: [PATCH 2/3] Report DIFFS as the true number of unique chunks that differ across all slaves. --- bin/pt-table-checksum | 11 +++++------ t/pt-table-checksum/bugs.t | 18 ++++++++++++++++-- 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/bin/pt-table-checksum b/bin/pt-table-checksum index ac4ee8c5..42029680 100755 --- a/bin/pt-table-checksum +++ b/bin/pt-table-checksum @@ -7690,7 +7690,7 @@ sub main { ); # Check each slave for checksum diffs. - my $max_diffs = 0; + my %diff_chunks; foreach my $slave ( @$slaves ) { eval { my $diffs = $rc->find_replication_differences( @@ -7700,10 +7700,9 @@ sub main { ); PTDEBUG && _d(scalar @$diffs, 'checksum diffs on', $slave->name()); - if ( my $n_diffs = scalar @$diffs ) { - # https://bugs.launchpad.net/percona-toolkit/+bug/1030031 - $max_diffs = $n_diffs if $n_diffs > $max_diffs; - } + # Save unique chunks that differ. + # https://bugs.launchpad.net/percona-toolkit/+bug/1030031 + map { $diff_chunks{ $_->{chunk} }++ } @$diffs; }; if ($EVAL_ERROR) { if ( $o->get('quiet') < 2 ) { @@ -7716,7 +7715,7 @@ sub main { $tbl->{checksum_results}->{errors}++; } } - $tbl->{checksum_results}->{diffs} = $max_diffs; + $tbl->{checksum_results}->{diffs} = scalar keys %diff_chunks; } # Print table's checksum results if we're not being quiet, diff --git a/t/pt-table-checksum/bugs.t b/t/pt-table-checksum/bugs.t index e0149c9a..8c6e0fe7 100644 --- a/t/pt-table-checksum/bugs.t +++ b/t/pt-table-checksum/bugs.t @@ -42,7 +42,7 @@ elsif ( !$slave2_dbh ) { plan skip_all => 'Cannot connect to sandbox slave2'; } else { - plan tests => 9; + plan tests => 10; } # The sandbox servers run with lock_wait_timeout=3 and it's not dynamic @@ -155,7 +155,6 @@ $slave1_dbh->do("SET SQL_LOG_BIN=1"); $output = output( sub { pt_table_checksum::main(@args, qw(-t test.t --chunk-size 10)) }, - stderr => 1, ); is( @@ -164,6 +163,21 @@ is( "Bug 1030031 (wrong DIFFS): 2 diffs" ); +# Restore slave2, but then give it 1 diff that's not the same chunk# +# as slave1, so there's 3 unique chunk that differ. +$slave2_dbh->do("UPDATE test.t SET c='e' WHERE id=5"); +$slave2_dbh->do("UPDATE test.t SET c='' WHERE id=26"); + +$output = output( + sub { pt_table_checksum::main(@args, qw(-t test.t --chunk-size 10)) }, +); + +is( + PerconaTest::count_checksum_results($output, 'diffs'), + 3, + "Bug 1030031 (wrong DIFFS): 3 diffs" +); + # ############################################################################# # Done. # ############################################################################# From c25c79e119c904cc2698b04e069c4771273e2207 Mon Sep 17 00:00:00 2001 From: Daniel Nichter Date: Fri, 27 Jul 2012 11:35:27 -0600 Subject: [PATCH 3/3] Only iterate diffs if there are some. --- bin/pt-table-checksum | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/bin/pt-table-checksum b/bin/pt-table-checksum index 42029680..8131bde0 100755 --- a/bin/pt-table-checksum +++ b/bin/pt-table-checksum @@ -7702,7 +7702,9 @@ sub main { $slave->name()); # Save unique chunks that differ. # https://bugs.launchpad.net/percona-toolkit/+bug/1030031 - map { $diff_chunks{ $_->{chunk} }++ } @$diffs; + if ( scalar @$diffs ) { + map { $diff_chunks{ $_->{chunk} }++ } @$diffs; + } }; if ($EVAL_ERROR) { if ( $o->get('quiet') < 2 ) {