From 7748cc765d99012ce5c173b1726e8f1bc5c903c9 Mon Sep 17 00:00:00 2001 From: Carlos Salguero Date: Tue, 8 Jan 2019 14:37:05 -0300 Subject: [PATCH 1/2] Revert "Merge pull request #380 from percona/PT-1114" This reverts commit bcbc175d0c1ee5ece379b67aca38114d3f6e9571, reversing changes made to cf5c661d46296b8a603c5a93b59a79d67e84baf5. --- bin/pt-online-schema-change | 7 --- bin/pt-table-checksum | 43 ++++++--------- lib/IndexLength.pm | 8 --- t/pt-table-checksum/pt-1114.t | 72 ------------------------- t/pt-table-checksum/samples/pt-1114.sql | 49 ----------------- 5 files changed, 16 insertions(+), 163 deletions(-) delete mode 100644 t/pt-table-checksum/pt-1114.t delete mode 100644 t/pt-table-checksum/samples/pt-1114.sql diff --git a/bin/pt-online-schema-change b/bin/pt-online-schema-change index c8ceb37e..91fd61f3 100755 --- a/bin/pt-online-schema-change +++ b/bin/pt-online-schema-change @@ -6669,13 +6669,6 @@ sub index_length { n_index_cols => $n_index_cols, ); - if (!$vals) { - my @row = $cxn->dbh()->selectrow_array('SELECT COUNT(*) FROM '.$args{tbl}->{name}); - if ($row[0] == 0) { - PTDEBUG && _d('Table '.$args{tbl}->{name}.' is empty'); - return undef, undef; - } - } my $sql = $self->_make_range_query( %args, n_index_cols => $n_index_cols, diff --git a/bin/pt-table-checksum b/bin/pt-table-checksum index 030d894b..596e7604 100755 --- a/bin/pt-table-checksum +++ b/bin/pt-table-checksum @@ -9621,13 +9621,6 @@ sub index_length { n_index_cols => $n_index_cols, ); - if (!$vals) { - my @row = $cxn->dbh()->selectrow_array('SELECT COUNT(*) FROM '.$args{tbl}->{name}); - if ($row[0] == 0) { - PTDEBUG && _d('Table '.$args{tbl}->{name}.' is empty'); - return undef, undef; - } - } my $sql = $self->_make_range_query( %args, n_index_cols => $n_index_cols, @@ -11031,27 +11024,23 @@ sub main { index => $nibble_iter->nibble_index(), n_index_cols => $o->get('chunk-index-columns'), ); - if ($key) { - if ( !$key || lc($key) ne lc($nibble_iter->nibble_index()) ) { - die "Cannot determine the key_len of the chunk index " - . "because MySQL chose " - . ($key ? "the $key" : "no") . " index " - . "instead of the " . $nibble_iter->nibble_index() - . " index for the first lower boundary statement. " - . "See --[no]check-plan in the documentation for more " - . "information."; - } elsif ( $key_len) { - $tbl->{key_len} = $key_len; - } + if ( !$key || lc($key) ne lc($nibble_iter->nibble_index()) ) { + die "Cannot determine the key_len of the chunk index " + . "because MySQL chose " + . ($key ? "the $key" : "no") . " index " + . "instead of the " . $nibble_iter->nibble_index() + . " index for the first lower boundary statement. " + . "See --[no]check-plan in the documentation for more " + . "information."; } - # If there is no key probably the table is empty - # elsif ( !$key_len ) { - # die "The key_len of the $key index is " - # . (defined $key_len ? "zero" : "NULL") - # . ", but this should not be possible. " - # . "See --[no]check-plan in the documentation for more " - # . "information."; - # } + elsif ( !$key_len ) { + die "The key_len of the $key index is " + . (defined $key_len ? "zero" : "NULL") + . ", but this should not be possible. " + . "See --[no]check-plan in the documentation for more " + . "information."; + } + $tbl->{key_len} = $key_len; } } diff --git a/lib/IndexLength.pm b/lib/IndexLength.pm index 3fe43856..32bbfc58 100644 --- a/lib/IndexLength.pm +++ b/lib/IndexLength.pm @@ -75,14 +75,6 @@ sub index_length { n_index_cols => $n_index_cols, ); - if (!$vals) { - # Maybe the table is empty ... - my @row = $cxn->dbh()->selectrow_array('SELECT COUNT(*) FROM '.$args{tbl}->{name}); - if ($row[0] == 0) { - PTDEBUG && _d('Table '.$args{tbl}->{name}.' is empty'); - return undef, undef; - } - } # Make an EXPLAIN query to scan the range and execute it. my $sql = $self->_make_range_query( %args, diff --git a/t/pt-table-checksum/pt-1114.t b/t/pt-table-checksum/pt-1114.t deleted file mode 100644 index 1ef260c6..00000000 --- a/t/pt-table-checksum/pt-1114.t +++ /dev/null @@ -1,72 +0,0 @@ -#!/usr/bin/env perl - -BEGIN { - die "The PERCONA_TOOLKIT_BRANCH environment variable is not set.\n" - unless $ENV{PERCONA_TOOLKIT_BRANCH} && -d $ENV{PERCONA_TOOLKIT_BRANCH}; - unshift @INC, "$ENV{PERCONA_TOOLKIT_BRANCH}/lib"; -}; - -use strict; -use warnings FATAL => 'all'; -use English qw(-no_match_vars); -use Test::More; - -use PerconaTest; -use Sandbox; -use SqlModes; -require "$trunk/bin/pt-table-checksum"; - -my $dp = new DSNParser(opts=>$dsn_opts); -my $sb = new Sandbox(basedir => '/tmp', DSNParser => $dp); -my $dsn = $sb->dsn_for('master'); -my $dbh = $sb->get_dbh_for('master'); - -if ( !$dbh ) { - plan skip_all => 'Cannot connect to sandbox master'; -} -else { - plan tests => 3; -} - -diag("loading samples"); -$sb->load_file('master', 't/pt-table-checksum/samples/pt-1114.sql'); - -my $master_port = $sb->port_for('master'); -my $num_rows = 40000; - -diag(`util/mysql_random_data_load --host=127.0.0.1 --port=$master_port --user=msandbox --password=msandbox test t1 $num_rows`); - -$dbh->do('set global innodb_stats_persistent=0;'); -$dbh->do('DELETE FROM test.t1'); - -my @args = ($dsn); -my $output; -my $exit_status; - -# Test #1 -$output = output( - sub { $exit_status = pt_table_checksum::main(@args) }, - stderr => 1, -); - -is( - $exit_status, - 0, - "Exit status OK", -); -diag($output); - -like( - $output, - qr/0\s+0\s+0\s+0\s+1\s+0\s+\d+\.\d+\s+test\.t1/, - "Checksumed test.t1 even when it is empty", -); - -$dbh->do('SET GLOBAL binlog_format="STATEMENT"'); - -# ############################################################################# -# Done. -# ############################################################################# -$sb->wipe_clean($dbh); -ok($sb->ok(), "Sandbox servers") or BAIL_OUT(__FILE__ . " broke the sandbox"); -exit; diff --git a/t/pt-table-checksum/samples/pt-1114.sql b/t/pt-table-checksum/samples/pt-1114.sql deleted file mode 100644 index 97a48876..00000000 --- a/t/pt-table-checksum/samples/pt-1114.sql +++ /dev/null @@ -1,49 +0,0 @@ --- MySQL dump 10.13 Distrib 5.6.26-74.0, for Linux (x86_64) --- --- Host: 127.0.0.1 Database: test --- ------------------------------------------------------ --- Server version 5.6.25-73.1-log - -/*!40101 SET @OLD_CHARACTER_SET_CLIENT=@@CHARACTER_SET_CLIENT */; -/*!40101 SET @OLD_CHARACTER_SET_RESULTS=@@CHARACTER_SET_RESULTS */; -/*!40101 SET @OLD_COLLATION_CONNECTION=@@COLLATION_CONNECTION */; -/*!40101 SET NAMES utf8 */; -/*!40103 SET @OLD_TIME_ZONE=@@TIME_ZONE */; -/*!40103 SET TIME_ZONE='+00:00' */; -/*!40014 SET @OLD_UNIQUE_CHECKS=@@UNIQUE_CHECKS, UNIQUE_CHECKS=0 */; -/*!40014 SET @OLD_FOREIGN_KEY_CHECKS=@@FOREIGN_KEY_CHECKS, FOREIGN_KEY_CHECKS=0 */; -/*!40101 SET @OLD_SQL_MODE=@@SQL_MODE, SQL_MODE='NO_AUTO_VALUE_ON_ZERO' */; -/*!40111 SET @OLD_SQL_NOTES=@@SQL_NOTES, SQL_NOTES=0 */; - --- --- Table structure for table `t1` --- - -DROP DATABASE IF EXISTS test; -CREATE DATABASE test; -USE test; - -DROP TABLE IF EXISTS `t1`; -/*!40101 SET @saved_cs_client = @@character_set_client */; -/*!40101 SET character_set_client = utf8 */; -CREATE TABLE `t1` ( - `a` int(11) NOT NULL, - `b` datetime DEFAULT NULL, - PRIMARY KEY (`a`), - KEY `b` (`b`) -) ENGINE=InnoDB DEFAULT CHARSET=latin1; -/*!40101 SET character_set_client = @saved_cs_client */; - -/*!40000 ALTER TABLE `t1` ENABLE KEYS */; -UNLOCK TABLES; -/*!40103 SET TIME_ZONE=@OLD_TIME_ZONE */; - -/*!40101 SET SQL_MODE=@OLD_SQL_MODE */; -/*!40014 SET FOREIGN_KEY_CHECKS=@OLD_FOREIGN_KEY_CHECKS */; -/*!40014 SET UNIQUE_CHECKS=@OLD_UNIQUE_CHECKS */; -/*!40101 SET CHARACTER_SET_CLIENT=@OLD_CHARACTER_SET_CLIENT */; -/*!40101 SET CHARACTER_SET_RESULTS=@OLD_CHARACTER_SET_RESULTS */; -/*!40101 SET COLLATION_CONNECTION=@OLD_COLLATION_CONNECTION */; -/*!40111 SET SQL_NOTES=@OLD_SQL_NOTES */; - --- Dump completed on 2016-04-26 14:38:53 From 424a48518cc6086ee2696b60316a9bc3b869b210 Mon Sep 17 00:00:00 2001 From: Carlos Salguero Date: Fri, 11 Jan 2019 15:43:38 -0300 Subject: [PATCH 2/2] PT-1114 test fix --- bin/pt-table-checksum | 32 ++++++++++++---- lib/NibbleIterator.pm | 26 ++++++++++++- t/pt-table-checksum/pt-1114.t | 72 +++++++++++++++++++++++++++++++++++ 3 files changed, 120 insertions(+), 10 deletions(-) create mode 100644 t/pt-table-checksum/pt-1114.t diff --git a/bin/pt-table-checksum b/bin/pt-table-checksum index 596e7604..1e1f3b62 100755 --- a/bin/pt-table-checksum +++ b/bin/pt-table-checksum @@ -6292,6 +6292,7 @@ use strict; use warnings FATAL => 'all'; use English qw(-no_match_vars); use constant PTDEBUG => $ENV{PTDEBUG} || 0; +use IndexLength; use Data::Dumper; $Data::Dumper::Indent = 1; @@ -6690,11 +6691,11 @@ sub row_estimate { sub can_nibble { my (%args) = @_; - my @required_args = qw(Cxn tbl chunk_size OptionParser TableParser); + my @required_args = qw(Cxn tbl chunk_size OptionParser TableParser Quoter); foreach my $arg ( @required_args ) { die "I need a $arg argument" unless $args{$arg}; } - my ($cxn, $tbl, $chunk_size, $o) = @args{@required_args}; + my ($cxn, $tbl, $chunk_size, $o, $q) = @args{@required_args}; my $where = $o->has('where') ? $o->get('where') : ''; @@ -6705,13 +6706,30 @@ sub can_nibble { ); if ( !$where ) { - $mysql_index = undef; - } + $mysql_index = undef; + } 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; + + if ($mysql_index) { + my $idx_len = IndexLength->new(Quoter => $q); + my ($key_len, $key) = $idx_len->index_length( + Cxn => $args{Cxn}, + tbl => $tbl, + index => $mysql_index, + n_index_cols => $o->get('chunk-index-columns'), + ); + + if ( !$key || !$key_len || lc($key) ne lc($mysql_index)) { + $one_nibble = 1; + } + } else { + $one_nibble = 1; + } + PTDEBUG && _d('One nibble:', $one_nibble ? 'yes' : 'no'); if ( $args{resume} @@ -9660,14 +9678,14 @@ sub _get_first_values { . "WHERE " . join(' AND ', @where) . " ORDER BY $index_columns " . "LIMIT 1 /*key_len*/"; # only need 1 row - PTDEBUG && _d($sql); + PTDEBUG && _d("_get_first_values: $sql"); my $vals = $cxn->dbh()->selectrow_arrayref($sql); return $vals; } sub _make_range_query { my ($self, %args) = @_; - my @required_args = qw(tbl index n_index_cols vals); + my @required_args = qw(tbl index n_index_cols); foreach my $arg ( @required_args ) { die "I need a $arg argument" unless $args{$arg}; } @@ -9682,13 +9700,11 @@ sub _make_range_query { if ( $n_index_cols > 1 ) { foreach my $n ( 0..($n_index_cols - 2) ) { my $col = $index_cols->[$n]; - my $val = $vals->[$n]; push @where, $q->quote($col) . " = ?"; } } my $col = $index_cols->[$n_index_cols - 1]; - my $val = $vals->[-1]; # should only be as many vals as cols push @where, $q->quote($col) . " >= ?"; my $sql = "EXPLAIN SELECT /*!40001 SQL_NO_CACHE */ * " diff --git a/lib/NibbleIterator.pm b/lib/NibbleIterator.pm index 285a990a..50784af9 100644 --- a/lib/NibbleIterator.pm +++ b/lib/NibbleIterator.pm @@ -26,6 +26,7 @@ use strict; use warnings FATAL => 'all'; use English qw(-no_match_vars); use constant PTDEBUG => $ENV{PTDEBUG} || 0; +use IndexLength; use Data::Dumper; $Data::Dumper::Indent = 1; @@ -487,11 +488,11 @@ sub row_estimate { sub can_nibble { my (%args) = @_; - my @required_args = qw(Cxn tbl chunk_size OptionParser TableParser); + my @required_args = qw(Cxn tbl chunk_size OptionParser TableParser Quoter); foreach my $arg ( @required_args ) { die "I need a $arg argument" unless $args{$arg}; } - my ($cxn, $tbl, $chunk_size, $o) = @args{@required_args}; + my ($cxn, $tbl, $chunk_size, $o, $q) = @args{@required_args}; my $where = $o->has('where') ? $o->get('where') : ''; @@ -502,6 +503,23 @@ sub can_nibble { where => $where, ); + my $can_get_keys; + if ($mysql_index) { + my $idx_len = IndexLength->new(Quoter => $q); + my ($key_len, $key) = $idx_len->index_length( + Cxn => $args{Cxn}, + tbl => $tbl, + index => $mysql_index, + n_index_cols => $o->get('chunk-index-columns'), + ); + + if ( !$key || !$key_len || lc($key) ne lc($mysql_index)) { + $can_get_keys = 0; + } else { + $can_get_keys = 1; + } + } + # 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. @@ -521,6 +539,10 @@ sub can_nibble { my $one_nibble = !defined $args{one_nibble} || $args{one_nibble} ? $row_est <= $chunk_size * $chunk_size_limit : 0; + if (!$can_get_keys) { + $one_nibble = 1; + } + PTDEBUG && _d('One nibble:', $one_nibble ? 'yes' : 'no'); # Special case: we're resuming and there's no boundaries, so the table diff --git a/t/pt-table-checksum/pt-1114.t b/t/pt-table-checksum/pt-1114.t new file mode 100644 index 00000000..1ef260c6 --- /dev/null +++ b/t/pt-table-checksum/pt-1114.t @@ -0,0 +1,72 @@ +#!/usr/bin/env perl + +BEGIN { + die "The PERCONA_TOOLKIT_BRANCH environment variable is not set.\n" + unless $ENV{PERCONA_TOOLKIT_BRANCH} && -d $ENV{PERCONA_TOOLKIT_BRANCH}; + unshift @INC, "$ENV{PERCONA_TOOLKIT_BRANCH}/lib"; +}; + +use strict; +use warnings FATAL => 'all'; +use English qw(-no_match_vars); +use Test::More; + +use PerconaTest; +use Sandbox; +use SqlModes; +require "$trunk/bin/pt-table-checksum"; + +my $dp = new DSNParser(opts=>$dsn_opts); +my $sb = new Sandbox(basedir => '/tmp', DSNParser => $dp); +my $dsn = $sb->dsn_for('master'); +my $dbh = $sb->get_dbh_for('master'); + +if ( !$dbh ) { + plan skip_all => 'Cannot connect to sandbox master'; +} +else { + plan tests => 3; +} + +diag("loading samples"); +$sb->load_file('master', 't/pt-table-checksum/samples/pt-1114.sql'); + +my $master_port = $sb->port_for('master'); +my $num_rows = 40000; + +diag(`util/mysql_random_data_load --host=127.0.0.1 --port=$master_port --user=msandbox --password=msandbox test t1 $num_rows`); + +$dbh->do('set global innodb_stats_persistent=0;'); +$dbh->do('DELETE FROM test.t1'); + +my @args = ($dsn); +my $output; +my $exit_status; + +# Test #1 +$output = output( + sub { $exit_status = pt_table_checksum::main(@args) }, + stderr => 1, +); + +is( + $exit_status, + 0, + "Exit status OK", +); +diag($output); + +like( + $output, + qr/0\s+0\s+0\s+0\s+1\s+0\s+\d+\.\d+\s+test\.t1/, + "Checksumed test.t1 even when it is empty", +); + +$dbh->do('SET GLOBAL binlog_format="STATEMENT"'); + +# ############################################################################# +# Done. +# ############################################################################# +$sb->wipe_clean($dbh); +ok($sb->ok(), "Sandbox servers") or BAIL_OUT(__FILE__ . " broke the sandbox"); +exit;