diff --git a/bin/pt-online-schema-change b/bin/pt-online-schema-change index 321b078e..7630547c 100755 --- a/bin/pt-online-schema-change +++ b/bin/pt-online-schema-change @@ -2815,7 +2815,7 @@ sub name { sub DESTROY { my ($self) = @_; - if ( $self->{dbh} ) { + if ( $self->{dbh} && ref($self->{dbh}) ) { PTDEBUG && _d('Disconnecting dbh', $self->{dbh}, $self->{name}); $self->{dbh}->disconnect(); } @@ -4126,8 +4126,8 @@ sub next { while ( $self->{have_rows} || $self->_next_boundaries() ) { if ( !$self->{have_rows} ) { $self->{nibbleno}++; - PTDEBUG && _d($self->{nibble_sth}->{Statement}, 'params:', - join(', ', (@{$self->{lower}}, @{$self->{upper}}))); + PTDEBUG && _d('Nibble:', $self->{nibble_sth}->{Statement}, 'params:', + join(', ', (@{$self->{lower} || []}, @{$self->{upper} || []}))); if ( my $callback = $self->{callbacks}->{exec_nibble} ) { $self->{have_rows} = $callback->(%callback_args); } @@ -4257,14 +4257,21 @@ sub can_nibble { } my ($cxn, $tbl, $chunk_size, $o) = @args{@required_args}; + my $where = $o->has('where') ? $o->get('where') : ''; + my ($row_est, $mysql_index) = get_row_estimate( Cxn => $cxn, tbl => $tbl, - where => $o->has('where') ? $o->get('where') : '', + where => $where, ); + if ( !$where ) { + $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 * $o->get('chunk-size-limit') + ? $row_est <= $chunk_size * $chunk_size_limit : 0; PTDEBUG && _d('One nibble:', $one_nibble ? 'yes' : 'no'); @@ -4392,7 +4399,11 @@ sub get_row_estimate { PTDEBUG && _d($sql); my $expl = $cxn->dbh()->selectrow_hashref($sql); PTDEBUG && _d(Dumper($expl)); - return ($expl->{rows} || 0), $expl->{key}; + my $mysql_index = $expl->{key} || ''; + if ( $mysql_index ne 'PRIMARY' ) { + $mysql_index = lc($mysql_index); + } + return ($expl->{rows} || 0), $mysql_index; } sub _prepare_sths { @@ -4445,12 +4456,10 @@ sub _get_bounds { if ( !$self->{next_lower} ) { PTDEBUG && _d('At end of table, or no more boundaries to resume'); + $self->{no_more_boundaries} = 1; $self->{last_upper} = $dbh->selectrow_arrayref($self->{last_ub_sql}); PTDEBUG && _d('Last upper boundary:', Dumper($self->{last_upper})); - $self->{no_more_boundaries} = 1; - - $self->{no_more_boundaries} = 1; } return; @@ -4470,12 +4479,15 @@ sub _next_boundaries { return 1; # continue nibbling } + + if ( $self->identical_boundaries($self->{lower}, $self->{next_lower}) ) { PTDEBUG && _d('Infinite loop detected'); my $tbl = $self->{tbl}; my $index = $tbl->{tbl_struct}->{keys}->{$self->{index}}; my $n_cols = scalar @{$index->{cols}}; my $chunkno = $self->{nibbleno}; + die "Possible infinite loop detected! " . "The lower boundary for chunk $chunkno is " . "<" . join(', ', @{$self->{lower}}) . "> and the lower " @@ -4502,6 +4514,7 @@ sub _next_boundaries { } } + PTDEBUG && _d($self->{ub_sth}->{Statement}, 'params:', join(', ', @{$self->{lower}}), $self->{limit}); $self->{ub_sth}->execute(@{$self->{lower}}, $self->{limit}); @@ -4860,8 +4873,13 @@ sub new { sub DESTROY { my ($self) = @_; my $task = $self->{task}; - PTDEBUG && _d('Calling cleanup task', $task); - $task->(); + if ( ref $task ) { + PTDEBUG && _d('Calling cleanup task', $task); + $task->(); + } + else { + warn "Lost cleanup task"; + } return; } diff --git a/t/pt-online-schema-change/bugs.t b/t/pt-online-schema-change/bugs.t new file mode 100644 index 00000000..ee6c9191 --- /dev/null +++ b/t/pt-online-schema-change/bugs.t @@ -0,0 +1,73 @@ +#!/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 Data::Dumper; +use PerconaTest; +use Sandbox; + +require "$trunk/bin/pt-online-schema-change"; + +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'); + +if ( !$master_dbh ) { + plan skip_all => 'Cannot connect to sandbox master'; +} +elsif ( !$slave_dbh ) { + plan skip_all => 'Cannot connect to sandbox slave1'; +} +else { + plan tests => 2; +} + +# 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. +my $master_dsn = 'h=127.1,P=12345,u=msandbox,p=msandbox'; +my @args = (qw(--lock-wait-timeout 3)); +my $output; +my $exit_status; +my $sample = "t/pt-online-schema-change/samples/"; + +# ############################################################################ +# https://bugs.launchpad.net/percona-toolkit/+bug/994002 +# pt-online-schema-change 2.1.1 doesn't choose the PRIMARY KEY +# ############################################################################ +$sb->load_file('master', "$sample/pk-bug-994002.sql"); + +$output = output( + sub { $exit_status = pt_online_schema_change::main(@args, + "$master_dsn,D=test,t=t", + "--alter", "add column (foo int)", + qw(--chunk-size 2 --dry-run --print)) }, +); + +# Must chunk the table to detect the next test correctly. +like( + $output, + qr/next chunk boundary/, + "Bug 994002: chunks the table" +); + +unlike( + $output, + qr/FORCE INDEX\(`guest_language`\)/, + "Bug 994002: doesn't choose non-PK" +); + +# ############################################################################# +# Done. +# ############################################################################# +$sb->wipe_clean($master_dbh); +exit; diff --git a/t/pt-online-schema-change/samples/pk-bug-994002.sql b/t/pt-online-schema-change/samples/pk-bug-994002.sql new file mode 100644 index 00000000..43926632 --- /dev/null +++ b/t/pt-online-schema-change/samples/pk-bug-994002.sql @@ -0,0 +1,29 @@ +drop database if exists test; +create database test; +use test; + +CREATE TABLE t ( + `ufi` int(11) NOT NULL, + `guest_language` char(2) NOT NULL, + `guest_country` char(2) NOT NULL, + `score` int(10) unsigned NOT NULL, + PRIMARY KEY (`ufi`,`guest_language`,`guest_country`), + KEY `guest_language` (`guest_language`,`guest_country`,`score`) +) ENGINE=InnoDB; + +INSERT INTO t VALUES + (1, 'en', 'en', 1), + (2, 'fr', 'fr', 1), + (3, 'es', 'es', 1), + (4, 'ru', 'ru', 1), + (5, 'sl', 'sl', 1), + (6, 'ch', 'ch', 1), + (7, 'en', 'en', 1), + (8, 'fr', 'fr', 1), + (9, 'es', 'es', 1), + (10,'ru', 'ru', 1), + (11,'sl', 'sl', 1), + (12,'aa', 'ch', 1), + (10,'ab', 'ru', 1), + (11,'ac', 'sl', 1), + (12,'ad', 'ch', 1);