From df4662ce5197062bd6298e3f5d0edd8e3d9214f1 Mon Sep 17 00:00:00 2001 From: Daniel Nichter Date: Mon, 6 Feb 2012 13:29:08 -0700 Subject: [PATCH 1/4] Compare index names lc but save them in their original case. --- bin/pt-table-checksum | 10 +++--- lib/NibbleIterator.pm | 2 +- t/pt-table-checksum/samples/no-recheck.txt | 8 ++--- t/pt-table-sync/basics.t | 37 +++++++++++++++++++++- t/pt-table-sync/samples/bug_927771.sql | 18 +++++++++++ 5 files changed, 65 insertions(+), 10 deletions(-) create mode 100644 t/pt-table-sync/samples/bug_927771.sql diff --git a/bin/pt-table-checksum b/bin/pt-table-checksum index 15da8403..37919124 100755 --- a/bin/pt-table-checksum +++ b/bin/pt-table-checksum @@ -3778,7 +3778,7 @@ sub set_nibble_number { sub nibble_index { my ($self) = @_; - return lc($self->{index}); + return $self->{index}; } sub statements { @@ -6399,7 +6399,8 @@ sub main { sth => $sth->{explain_upper_boundary}, vals => [ @{$boundary->{lower}}, $nibble_iter->chunk_size() ], ); - if ( lc($expl->{key} || '') ne $nibble_iter->nibble_index() ) { + if ( lc($expl->{key} || '') + ne lc($nibble_iter->nibble_index() || '') ) { PTDEBUG && _d('Cannot nibble next chunk, aborting table'); if ( $o->get('quiet') < 2 ) { my $msg @@ -6467,7 +6468,8 @@ sub main { : 0; # Ensure that MySQL is using the chunk index. - if ( lc($expl->{key} || '') ne $nibble_iter->nibble_index() ) { + if ( lc($expl->{key} || '') + ne lc($nibble_iter->nibble_index() || '') ) { PTDEBUG && _d('Chunk', $args{nibbleno}, 'of table', "$tbl->{db}.$tbl->{tbl} not using chunk index, skipping"); return 0; # next boundary @@ -7266,7 +7268,7 @@ sub have_more_chunks { # The previous chunk index must match the current chunk index, # else we don't know what to do. - my $chunk_index = $nibble_iter->nibble_index() || ''; + my $chunk_index = lc($nibble_iter->nibble_index() || ''); if (lc($last_chunk->{chunk_index} || '') ne $chunk_index) { warn ts("Cannot resume from table $tbl->{db}.$tbl->{tbl} chunk " . "$last_chunk->{chunk} because the chunk indexes are different: " diff --git a/lib/NibbleIterator.pm b/lib/NibbleIterator.pm index fd8a9ad4..93f9a28c 100644 --- a/lib/NibbleIterator.pm +++ b/lib/NibbleIterator.pm @@ -347,7 +347,7 @@ sub set_nibble_number { sub nibble_index { my ($self) = @_; - return lc($self->{index}); + return $self->{index}; } sub statements { diff --git a/t/pt-table-checksum/samples/no-recheck.txt b/t/pt-table-checksum/samples/no-recheck.txt index bc1eae68..e03c1a8c 100644 --- a/t/pt-table-checksum/samples/no-recheck.txt +++ b/t/pt-table-checksum/samples/no-recheck.txt @@ -1,10 +1,10 @@ Differences on h=127.0.0.1,P=12346 TABLE CHUNK CNT_DIFF CRC_DIFF CHUNK_INDEX LOWER_BOUNDARY UPPER_BOUNDARY -sakila.city 1 0 1 primary 1 100 -sakila.city 6 0 1 primary 501 600 +sakila.city 1 0 1 PRIMARY 1 100 +sakila.city 6 0 1 PRIMARY 501 600 Differences on h=127.0.0.1,P=12347 TABLE CHUNK CNT_DIFF CRC_DIFF CHUNK_INDEX LOWER_BOUNDARY UPPER_BOUNDARY -sakila.city 1 0 1 primary 1 100 -sakila.city 6 0 1 primary 501 600 +sakila.city 1 0 1 PRIMARY 1 100 +sakila.city 6 0 1 PRIMARY 501 600 diff --git a/t/pt-table-sync/basics.t b/t/pt-table-sync/basics.t index 09020277..e132b02c 100644 --- a/t/pt-table-sync/basics.t +++ b/t/pt-table-sync/basics.t @@ -28,7 +28,7 @@ elsif ( !$slave_dbh ) { plan skip_all => 'Cannot connect to sandbox slave'; } else { - plan tests => 19; + plan tests => 21; } $sb->wipe_clean($master_dbh); @@ -198,6 +198,41 @@ is( "Synced diff (bug 911996)" ); +# Fix bug 927771. +$sb->load_file('master', 't/pt-table-sync/samples/bug_927771.sql'); +PerconaTest::wait_for_table($slave_dbh, "test.t", "c='j'"); + +$slave_dbh->do("update test.t set c='z' where id>8"); + +`$trunk/bin/pt-table-checksum h=127.1,P=12345,u=msandbox,p=msandbox --max-load '' --lock-wait 3 --chunk-size 2 -t test.t --quiet`; + +PerconaTest::wait_for_table($slave_dbh, "percona.checksums", "db='test' and tbl='t' and chunk=4"); + +$output = output( + sub { + pt_table_sync::main('h=127.1,P=12345,u=msandbox,p=msandbox', + qw(--print --execute --replicate percona.checksums), + qw(--no-foreign-key-checks)) + }, + stderr => 1, +); + +like( + $output, + qr/REPLACE INTO `test`.`t`\(`id`, `c`\) VALUES \('9', 'i'\)/, + "--replicate with uc index (bug 927771)" +); + +my $rows = $slave_dbh->selectall_arrayref("select id, c from test.t where id>8 order by id"); +is_deeply( + $rows, + [ + [9, 'i'], + [10, 'j'], + ], + "Synced slaved (bug 927771)" +); + # ############################################################################# # Done. # ############################################################################# diff --git a/t/pt-table-sync/samples/bug_927771.sql b/t/pt-table-sync/samples/bug_927771.sql new file mode 100644 index 00000000..928ca429 --- /dev/null +++ b/t/pt-table-sync/samples/bug_927771.sql @@ -0,0 +1,18 @@ +drop database if exists test; +create database test; +use test; +create table t ( + id int auto_increment not null primary key, + c varchar(8) not null +) engine=innodb; +insert into test.t values + (null, 'a'), + (null, 'b'), + (null, 'c'), + (null, 'd'), + (null, 'e'), + (null, 'f'), + (null, 'g'), + (null, 'h'), + (null, 'i'), + (null, 'j'); From 2dfa2fd54fabb8b3ac4ca2ae0fba011dcec59ce5 Mon Sep 17 00:00:00 2001 From: Daniel Nichter Date: Mon, 6 Feb 2012 13:39:11 -0700 Subject: [PATCH 2/4] Wrap check for binlog_format in version directive to avoid error on MySQL 5.0. --- bin/pt-table-checksum | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/bin/pt-table-checksum b/bin/pt-table-checksum index 15da8403..66b5f7ce 100755 --- a/bin/pt-table-checksum +++ b/bin/pt-table-checksum @@ -5887,11 +5887,12 @@ sub main { # instead, it should check if it's already set to STATEMENT. # This is becase starting with MySQL 5.1.29, changing the format # requires a SUPER user. - my $sql = 'SELECT @@binlog_format'; + my $sql = '/*!50108 SELECT @@binlog_format */'; PTDEBUG && _d($dbh, $sql); my ($original_binlog_format) = $dbh->selectrow_array($sql); PTDEBUG && _d('Original binlog_format:', $original_binlog_format); - if ( $original_binlog_format !~ /STATEMENT/i ) { + if ( $original_binlog_format + && $original_binlog_format !~ /STATEMENT/i ) { $sql = q{/*!50108 SET @@binlog_format := 'STATEMENT'*/}; eval { PTDEBUG && _d($dbh, $sql); From a08afff18b1a0803056c5d1dfe3a24d8ea26ee75 Mon Sep 17 00:00:00 2001 From: Daniel Nichter Date: Mon, 6 Feb 2012 13:45:54 -0700 Subject: [PATCH 3/4] Use VersionParser to check for MySQL 5.1 before checking binlog_format. --- bin/pt-table-checksum | 46 ++++++++++++++++++++++--------------------- 1 file changed, 24 insertions(+), 22 deletions(-) diff --git a/bin/pt-table-checksum b/bin/pt-table-checksum index d29f57e1..5e949542 100755 --- a/bin/pt-table-checksum +++ b/bin/pt-table-checksum @@ -5877,36 +5877,39 @@ sub main { # ######################################################################## # Connect to the master. # ######################################################################## + my $vp = new VersionParser(); + my $set_on_connect = sub { my ($dbh) = @_; - return if $o->get('explain'); + my $sql; # https://bugs.launchpad.net/percona-toolkit/+bug/919352 # The tool shouldn't blindly attempt to change binlog_format; # instead, it should check if it's already set to STATEMENT. # This is becase starting with MySQL 5.1.29, changing the format # requires a SUPER user. - my $sql = '/*!50108 SELECT @@binlog_format */'; - PTDEBUG && _d($dbh, $sql); - my ($original_binlog_format) = $dbh->selectrow_array($sql); - PTDEBUG && _d('Original binlog_format:', $original_binlog_format); - if ( $original_binlog_format - && $original_binlog_format !~ /STATEMENT/i ) { - $sql = q{/*!50108 SET @@binlog_format := 'STATEMENT'*/}; - eval { - PTDEBUG && _d($dbh, $sql); - $dbh->do($sql); - }; - if ( $EVAL_ERROR ) { - die "Failed to $sql: $EVAL_ERROR\n" - . "This tool requires binlog_format=STATEMENT, " - . "but the current binlog_format is set to " - ."$original_binlog_format and an error occurred while " - . "attempting to change it. If running MySQL 5.1.29 or newer, " - . "setting binlog_format requires the SUPER privilege. " - . "You will need to manually set binlog_format to 'STATEMENT' " - . "before running this tool.\n"; + if ( $vp->version_ge($dbh, '5.1.5') ) { + $sql = 'SELECT @@binlog_format'; + PTDEBUG && _d($dbh, $sql); + my ($original_binlog_format) = $dbh->selectrow_array($sql); + PTDEBUG && _d('Original binlog_format:', $original_binlog_format); + if ( $original_binlog_format !~ /STATEMENT/i ) { + $sql = q{/*!50108 SET @@binlog_format := 'STATEMENT'*/}; + eval { + PTDEBUG && _d($dbh, $sql); + $dbh->do($sql); + }; + if ( $EVAL_ERROR ) { + die "Failed to $sql: $EVAL_ERROR\n" + . "This tool requires binlog_format=STATEMENT, " + . "but the current binlog_format is set to " + ."$original_binlog_format and an error occurred while " + . "attempting to change it. If running MySQL 5.1.29 or newer, " + . "setting binlog_format requires the SUPER privilege. " + . "You will need to manually set binlog_format to 'STATEMENT' " + . "before running this tool.\n"; + } } } @@ -5996,7 +5999,6 @@ sub main { my $q = new Quoter(); my $tp = new TableParser(Quoter => $q); my $rc = new RowChecksum(Quoter=> $q, OptionParser => $o); - my $vp = new VersionParser(); my $ms = new MasterSlave(VersionParser => $vp); my $slaves; # all slaves (that we can find) From 04cb40a815c3c8ae0fa3d4771af3becfdd43a719 Mon Sep 17 00:00:00 2001 From: "baron@percona.com" <> Date: Tue, 7 Feb 2012 23:42:48 -0500 Subject: [PATCH 4/4] Update pt-query-advisor documentation, especially the rule text --- bin/pt-query-advisor | 61 +++++++++++++++++++++++--------------------- 1 file changed, 32 insertions(+), 29 deletions(-) diff --git a/bin/pt-query-advisor b/bin/pt-query-advisor index 2525e6e4..d4136af6 100755 --- a/bin/pt-query-advisor +++ b/bin/pt-query-advisor @@ -6666,17 +6666,12 @@ pt-query-advisor - Analyze queries and advise on possible problems. Usage: pt-query-advisor [OPTION...] [FILE] -pt-query-advisor analyzes queries and advises on possible problems. -Queries are given either by specifying slowlog files, --query, or --review. +pt-query-advisor detects bad patterns in a SQL query from the text alone. -Analyze all queries in a slow log: +Analyze all queries in a log in MySQL's slow query log format: pt-query-advisor /path/to/slow-query.log -Analyze all queires in a general log: - - pt-query-advisor --type genlog mysql.log - Get queries from tcpdump using pt-query-digest: pt-query-digest --type tcpdump.txt --print --no-report | pt-query-advisor @@ -6691,8 +6686,7 @@ tools) and those created by bugs. pt-query-advisor simply reads queries and examines them, and is thus very low risk. -At the time of this release there is a bug that may cause an infinite (or -very long) loop when parsing very large queries. +At the time of this release there are no known bugs that could harm users. The authoritative source for updated information is always the online issue tracking system. Issues that affect this tool will be marked as such. You can @@ -6777,14 +6771,16 @@ wildcard is potentially a bug in the SQL. severity: warn -SELECT without WHERE. The SELECT statement has no WHERE clause. +SELECT without WHERE. The SELECT statement has no WHERE clause and could +examine many more rows than intended. =item CLA.002 severity: note ORDER BY RAND(). ORDER BY RAND() is a very inefficient way to -retrieve a random row from the results. +retrieve a random row from the results, because it sorts the entire result +and then throws most of it away. =item CLA.003 @@ -6792,7 +6788,8 @@ severity: note LIMIT with OFFSET. Paginating a result set with LIMIT and OFFSET is O(n^2) complexity, and will cause performance problems as the data -grows larger. +grows larger. Pagination techniques such as bookmarked scans are much more +efficient. =item CLA.004 @@ -6806,21 +6803,24 @@ query is changed. severity: warn -ORDER BY constant column. +ORDER BY constant column. This is probably a bug in your SQL; at best it is a +useless operation that does not change the query results. =item CLA.006 severity: warn -GROUP BY or ORDER BY different tables will force a temp table and filesort. +GROUP BY or ORDER BY on different tables. This will force the use of a temporary +table and filesort, which can be a huge performance problem and can consume +large amounts of memory and temporary space on disk. =item CLA.007 severity: warn -ORDER BY different directions prevents index from being used. All tables -in the ORDER BY clause must be either ASC or DESC, else MySQL cannot use -an index. +ORDER BY different directions. All tables in the ORDER BY clause must be in the +same direction, either ASC or DESC, or MySQL cannot use an index to avoid a sort +after generating results. =item COL.001 @@ -6852,8 +6852,9 @@ more efficient to store IP addresses as integers. severity: warn -Unquoted date/time literal. A query such as "WHERE col<2010-02-12" -is valid SQL but is probably a bug; the literal should be quoted. +Unquoted date/time literal. A query such as "WHERE col<2010-02-12" is valid SQL +but is probably a bug, because it will be interpreted as "WHERE col<1996"; the +literal should be quoted. =item KWR.001 @@ -6868,23 +6869,25 @@ result screens. severity: crit -Mixing comma and ANSI joins. Mixing comma joins and ANSI joins -is confusing to humans, and the behavior differs between some -MySQL versions. +Mixing comma and ANSI joins. Mixing comma joins and ANSI joins is confusing to +humans, and the behavior and precedence differs between some MySQL versions, +which can introduce bugs. =item JOI.002 severity: crit A table is joined twice. The same table appears at least twice in the -FROM clause. +FROM clause in a manner that can be reduced to a single access to the table. =item JOI.003 severity: warn -Reference to outer table column in WHERE clause prevents OUTER JOIN, -implicitly converts to INNER JOIN. +OUTER JOIN defeated. The reference to an outer table column in the WHERE clause +prevents the OUTER JOIN from returning any non-matched rows, which implicitly +converts the query to an INNER JOIN. This is probably a bug in the query or a +misunderstanding of how OUTER JOIN works. =item JOI.004 @@ -6915,7 +6918,8 @@ non-deterministic results, depending on the query execution plan. severity: note -!= is non-standard. Use the <> operator to test for inequality. +The != operator is non-standard. Use the <> operator to test for inequality +instead. =item SUB.001 @@ -6923,9 +6927,8 @@ severity: crit IN() and NOT IN() subqueries are poorly optimized. MySQL executes the subquery as a dependent subquery for each row in the outer query. This is a frequent -cause of serious performance problems. This might change version 6.0 of MySQL, -but for versions 5.1 and older, the query should be rewritten as a JOIN or a -LEFT OUTER JOIN, respectively. +cause of serious performance problems in MySQL 5.5 and older versions. The +query probably should be rewritten as a JOIN or a LEFT OUTER JOIN, respectively. =back