From 3369a6c37d7c9e36ecf660c892ea1b451aec82a4 Mon Sep 17 00:00:00 2001 From: Daniel Nichter Date: Wed, 15 Aug 2012 16:24:19 -0600 Subject: [PATCH 1/2] Don't auto-vivify clustered_key hashref in PTDEBUG. --- bin/pt-duplicate-key-checker | 5 ++- lib/DuplicateKeyFinder.pm | 5 ++- t/lib/DuplicateKeyFinder.t | 3 +- t/pt-duplicate-key-checker/clustered_keys.t | 41 +++++++++++++++++-- .../samples/idb-no-uniques-bug-894140.sql | 9 ++++ 5 files changed, 55 insertions(+), 8 deletions(-) create mode 100644 t/pt-duplicate-key-checker/samples/idb-no-uniques-bug-894140.sql diff --git a/bin/pt-duplicate-key-checker b/bin/pt-duplicate-key-checker index 3e0b9130..73537da6 100755 --- a/bin/pt-duplicate-key-checker +++ b/bin/pt-duplicate-key-checker @@ -2188,8 +2188,9 @@ sub get_duplicate_keys { my $clustered_key = $args{clustered_key} ? $keys{$args{clustered_key}} : undef; - PTDEBUG && _d('clustered key:', $clustered_key->{name}, - $clustered_key->{colnames}); + PTDEBUG && _d('clustered key:', + $clustered_key ? ($clustered_key->{name}, $clustered_key->{colnames}) + : 'none'); if ( $clustered_key && $args{clustered} && $args{tbl_info}->{engine} diff --git a/lib/DuplicateKeyFinder.pm b/lib/DuplicateKeyFinder.pm index e43f9a43..b36b72a2 100644 --- a/lib/DuplicateKeyFinder.pm +++ b/lib/DuplicateKeyFinder.pm @@ -148,8 +148,9 @@ sub get_duplicate_keys { # Remove clustered duplicates. my $clustered_key = $args{clustered_key} ? $keys{$args{clustered_key}} : undef; - PTDEBUG && _d('clustered key:', $clustered_key->{name}, - $clustered_key->{colnames}); + PTDEBUG && _d('clustered key:', + $clustered_key ? ($clustered_key->{name}, $clustered_key->{colnames}) + : 'none'); if ( $clustered_key && $args{clustered} && $args{tbl_info}->{engine} diff --git a/t/lib/DuplicateKeyFinder.t b/t/lib/DuplicateKeyFinder.t index e39ff00d..224e480b 100644 --- a/t/lib/DuplicateKeyFinder.t +++ b/t/lib/DuplicateKeyFinder.t @@ -9,7 +9,7 @@ BEGIN { use strict; use warnings FATAL => 'all'; use English qw(-no_match_vars); -use Test::More tests => 38; +use Test::More; use VersionParser; use DuplicateKeyFinder; @@ -786,4 +786,5 @@ like( qr/Complete test coverage/, '_d() works' ); +done_testing; exit; diff --git a/t/pt-duplicate-key-checker/clustered_keys.t b/t/pt-duplicate-key-checker/clustered_keys.t index 403c699a..456fd24a 100644 --- a/t/pt-duplicate-key-checker/clustered_keys.t +++ b/t/pt-duplicate-key-checker/clustered_keys.t @@ -22,9 +22,6 @@ my $dbh = $sb->get_dbh_for('master'); if ( !$dbh ) { plan skip_all => 'Cannot connect to sandbox master'; } -else { - plan tests => 2; -} my $cnf = "/tmp/12345/my.sandbox.cnf"; my $sample = "t/pt-duplicate-key-checker/samples/"; @@ -46,9 +43,47 @@ ok( "Shorten, not remove, clustered dupes" ); +# ############################################################################# +# Error if InnoDB table has no PK or unique indexes +# https://bugs.launchpad.net/percona-toolkit/+bug/1036804 +# ############################################################################# +$sb->load_file('master', "t/pt-duplicate-key-checker/samples/idb-no-uniques-bug-894140.sql"); + +# PTDEBUG was auto-vivifying $clustered_key: +# +# PTDEBUG && _d('clustered key:', $clustered_key->{name}, +# $clustered_key->{colnames}); +# +# if ( $clustered_key +# && $args{clustered} +# && $args{tbl_info}->{engine} +# && $args{tbl_info}->{engine} =~ m/InnoDB/i ) +# { +# push @dupes, $self->remove_clustered_duplicates($clustered_key... +# +# sub remove_clustered_duplicates { +# my ( $self, $ck, $keys, %args ) = @_; +# die "I need a ck argument" unless $ck; +# die "I need a keys argument" unless $keys; +# my $ck_cols = $ck->{colnames}; +# my @dupes; +# KEY: +# for my $i ( 0 .. @$keys - 1 ) { +# my $key = $keys->[$i]->{colnames}; +# if ( $key =~ m/$ck_cols$/ ) { + +my $output = `PTDEBUG=1 $trunk/bin/pt-duplicate-key-checker F=$cnf -d bug_1036804 2>&1`; + +unlike( + $output, + qr/Use of uninitialized value/, + 'PTDEBUG doesn\'t auto-vivify cluster key hashref (bug 1036804)' +); + # ############################################################################# # Done. # ############################################################################# $sb->wipe_clean($dbh); ok($sb->ok(), "Sandbox servers") or BAIL_OUT(__FILE__ . " broke the sandbox"); +done_testing; exit; diff --git a/t/pt-duplicate-key-checker/samples/idb-no-uniques-bug-894140.sql b/t/pt-duplicate-key-checker/samples/idb-no-uniques-bug-894140.sql new file mode 100644 index 00000000..4eb0b6a3 --- /dev/null +++ b/t/pt-duplicate-key-checker/samples/idb-no-uniques-bug-894140.sql @@ -0,0 +1,9 @@ +DROP DATABASE IF EXISTS bug_1036804; +CREATE DATABASE bug_1036804; +USE bug_1036804; +CREATE TABLE `t` ( + `col1` int(11) DEFAULT NULL, + `col2` int(11) DEFAULT NULL, + KEY `col1` (`col1`), + KEY `col2` (`col2`) +) ENGINE=InnoDB DEFAULT CHARSET=latin1; From 310105b976840b6d80f167a9a49bff76b5d22ff9 Mon Sep 17 00:00:00 2001 From: Daniel Nichter Date: Wed, 15 Aug 2012 16:35:45 -0600 Subject: [PATCH 2/2] Eval checking each table in case the table goes away. --- bin/pt-duplicate-key-checker | 130 +++++++++++++++++------------------ 1 file changed, 65 insertions(+), 65 deletions(-) diff --git a/bin/pt-duplicate-key-checker b/bin/pt-duplicate-key-checker index 73537da6..64165559 100755 --- a/bin/pt-duplicate-key-checker +++ b/bin/pt-duplicate-key-checker @@ -3405,74 +3405,74 @@ sub main { ); TABLE: while ( my $tbl = $schema_itr->next() ) { - $tbl->{engine} = $tbl->{tbl_struct}->{engine}; + eval { + $tbl->{engine} = $tbl->{tbl_struct}->{engine}; - my ($keys, $clustered_key, $fks); - if ( $get_keys ) { - ($keys, $clustered_key) - = $tp->get_keys($tbl->{ddl}, {}); - } - if ( $get_fks ) { - $fks = $tp->get_fks($tbl->{ddl}, {database => $tbl->{db}}); - } - - next TABLE unless ($keys && %$keys) || ($fks && %$fks); - - if ( $o->got('verbose') ) { - print_all_keys($keys, $tbl, \%seen_tbl) if $keys; - print_all_keys($fks, $tbl, \%seen_tbl) if $fks; - } - else { - PTDEBUG && _d('Getting duplicate keys on', $tbl->{db}, $tbl->{tbl}); - eval { - if ( $keys ) { - $dk->get_duplicate_keys( - $keys, - clustered_key => $clustered_key, - tbl_info => $tbl, - callback => \&print_duplicate_key, - %tp_opts, - # get_duplicate_keys() ignores these args but passes them - # to the callback: - dbh => $dbh, - is_fk => 0, - o => $o, - ks => $ks, - tp => $tp, - q => $q, - seen_tbl => \%seen_tbl, - summary => \%summary, - ); - } - if ( $fks ) { - $dk->get_duplicate_fks( - $fks, - tbl_info => $tbl, - callback => \&print_duplicate_key, - %tp_opts, - # get_duplicate_fks() ignores these args but passes them - # to the callback: - dbh => $dbh, - is_fk => 1, - o => $o, - ks => $ks, - tp => $tp, - q => $q, - seen_tbl => \%seen_tbl, - summary => \%summary, - ); - } - }; - if ( $EVAL_ERROR ) { - warn "Error checking `$tbl->{db}`.`$tbl->{tbl}` for duplicate keys: " - . $EVAL_ERROR; - next TABLE; + my ($keys, $clustered_key, $fks); + if ( $get_keys ) { + ($keys, $clustered_key) + = $tp->get_keys($tbl->{ddl}, {}); + } + if ( $get_fks ) { + $fks = $tp->get_fks($tbl->{ddl}, {database => $tbl->{db}}); } - } - # Always count Total Keys so print_key_summary won't die - # because %summary is empty. - $summary{'Total Indexes'} += (scalar keys %$keys) + (scalar keys %$fks) + if ( ($keys && %$keys) || ($fks && %$fks) ) { + if ( $o->got('verbose') ) { + print_all_keys($keys, $tbl, \%seen_tbl) if $keys; + print_all_keys($fks, $tbl, \%seen_tbl) if $fks; + } + else { + PTDEBUG && _d('Getting duplicate keys on', + $tbl->{db}, $tbl->{tbl}); + if ( $keys ) { + $dk->get_duplicate_keys( + $keys, + clustered_key => $clustered_key, + tbl_info => $tbl, + callback => \&print_duplicate_key, + %tp_opts, + # get_duplicate_keys() ignores these args but passes them + # to the callback: + dbh => $dbh, + is_fk => 0, + o => $o, + ks => $ks, + tp => $tp, + q => $q, + seen_tbl => \%seen_tbl, + summary => \%summary, + ); + } + if ( $fks ) { + $dk->get_duplicate_fks( + $fks, + tbl_info => $tbl, + callback => \&print_duplicate_key, + %tp_opts, + # get_duplicate_fks() ignores these args but passes them + # to the callback: + dbh => $dbh, + is_fk => 1, + o => $o, + ks => $ks, + tp => $tp, + q => $q, + seen_tbl => \%seen_tbl, + summary => \%summary, + ); + } + } + + # Always count Total Keys so print_key_summary won't die + # because %summary is empty. + $summary{'Total Indexes'} += (scalar keys %$keys) + + (scalar keys %$fks) + } + }; + if ( $EVAL_ERROR ) { + warn "Error checking $tbl->{db}.$tbl->{tbl}: $EVAL_ERROR"; + } } # TABLE print_key_summary(%summary) if $o->get('summary');