From 98edc124279f428bca3594252c6f21c3e104ff65 Mon Sep 17 00:00:00 2001 From: Carlos Salguero Date: Tue, 25 Jul 2017 22:01:05 -0300 Subject: [PATCH] Revert "Merge pull request #119 from dveeden/reread_slaves_dsn" This reverts commit 68685117a6043afb53a01433f3da2d7c1dd56161, reversing changes made to 7a035cb2e748760443c2276ce84ae2744c0774c6. --- bin/pt-online-schema-change | 83 ++++++++----------------------------- lib/ReplicaLagWaiter.pm | 29 ------------- 2 files changed, 18 insertions(+), 94 deletions(-) diff --git a/bin/pt-online-schema-change b/bin/pt-online-schema-change index 99c6bc5f..32de77c8 100755 --- a/bin/pt-online-schema-change +++ b/bin/pt-online-schema-change @@ -3394,6 +3394,7 @@ sub check_table { $self->{check_table_error} = $e; return 0; } + if ( !$row->[0] || $row->[0] ne $tbl ) { PTDEBUG && _d('Table does not exist'); return 0; @@ -4869,24 +4870,6 @@ sub wait { my $worst; # most lagging slave my $pr_callback; my $pr_first_report; - - my $pr_refresh_slave_list = sub { - my ($self) = @_; - my ($slaves, $refresher) = ($self->{slaves}, $self->{get_slaves_cb}); - return if ( not defined $refresher ); - my $before = join ' ', sort map {$_->name()} @$slaves; - $slaves = $refresher->(); - my $after = join ' ', sort map {$_->name()} @$slaves; - if ($before ne $after) { - $self->{slaves} = $slaves; - printf "Slave set to watch has changed\n Was: %s\n Now: %s\n", - $before, $after; - } - return($self->{slaves}); - }; - - $slaves = $pr_refresh_slave_list->($self); - if ( $pr ) { $pr_callback = sub { my ($fraction, $elapsed, $remaining, $eta, $completed) = @_; @@ -4914,14 +4897,6 @@ sub wait { my @lagged_slaves = map { {cxn=>$_, lag=>undef} } @$slaves; while ( $oktorun->() && @lagged_slaves ) { PTDEBUG && _d('Checking slave lag'); - - $slaves = $pr_refresh_slave_list->($self); - my $watched = 0; - @lagged_slaves = grep { - my $slave_name = $_->{cxn}->name(); - grep {$slave_name eq $_->name()} @{$slaves || []} - } @lagged_slaves; - for my $i ( 0..$#lagged_slaves ) { my $lag = $get_lag->($lagged_slaves[$i]->{cxn}); PTDEBUG && _d($lagged_slaves[$i]->{cxn}->name(), @@ -5796,26 +5771,23 @@ sub _find_best_index { my $tbl_struct = $tbl->{tbl_struct}; my $indexes = $tbl_struct->{keys}; - my $best_index; my $want_index = $args{chunk_index}; if ( $want_index ) { PTDEBUG && _d('User wants to use index', $want_index); if ( !exists $indexes->{$want_index} ) { PTDEBUG && _d('Cannot use user index because it does not exist'); $want_index = undef; - } else { - $best_index = $want_index; } } - if ( !$best_index && !$want_index && $args{mysql_index} ) { + if ( !$want_index && $args{mysql_index} ) { PTDEBUG && _d('MySQL wants to use index', $args{mysql_index}); $want_index = $args{mysql_index}; } - + my $best_index; my @possible_indexes; - if ( !$best_index && $want_index ) { + if ( $want_index ) { if ( $indexes->{$want_index}->{is_unique} ) { PTDEBUG && _d('Will use wanted index'); $best_index = $want_index; @@ -5825,8 +5797,7 @@ sub _find_best_index { push @possible_indexes, $want_index; } } - - if (!$best_index) { + else { PTDEBUG && _d('Auto-selecting best index'); foreach my $index ( $tp->sort_indexes($tbl_struct) ) { if ( $index eq 'PRIMARY' || $indexes->{$index}->{is_unique} ) { @@ -8315,11 +8286,8 @@ sub main { # think that can be less confusing. Also, the $set_on_connect variable can be # inlined into this subroutine. Many of our tools have a get_dbh() subroutine # and it might be good to just make a convention of it. - # Note: args->{errok} to tolerate connection error my $make_cxn = sub { my (%args) = @_; - my $errok = $args{errok}; - delete($args{errok}); my $cxn = Cxn->new( %args, DSNParser => $dp, @@ -8328,12 +8296,7 @@ sub main { ); eval { $cxn->connect() }; # connect or die trying if ( $EVAL_ERROR ) { - if ($errok) { - printf "IGNORING CONNECTION ERROR: %s\n", - $EVAL_ERROR; - } else { - die "Cannot connect to MySQL: $EVAL_ERROR\n"; - } + die "Cannot connect to MySQL: $EVAL_ERROR\n"; } return $cxn; }; @@ -8472,21 +8435,13 @@ sub main { Quoter => $q, ); - my $get_slaves_cb = sub { - my ($intolerant) = @_; - return( $ms->get_slaves( - dbh => $cxn->dbh(), - dsn => $cxn->dsn(), - make_cxn => sub { - return $make_cxn->(@_, prev_dsn => $cxn->dsn(), - errok => (not $intolerant)); - }, - ) - ); - }; - - ### first ever call only: do not tolerate connection errors - $slaves = $get_slaves_cb->('intolerant'); + $slaves = $ms->get_slaves( + dbh => $cxn->dbh(), + dsn => $cxn->dsn(), + make_cxn => sub { + return $make_cxn->(@_, prev_dsn => $cxn->dsn()); + }, + ); PTDEBUG && _d(scalar @$slaves, 'slaves found'); if ( scalar @$slaves ) { print "Found " . scalar(@$slaves) . " slaves:\n"; @@ -8510,7 +8465,6 @@ sub main { prev_dsn => $cxn->dsn(), ); $slave_lag_cxns = [ $cxn ]; - $get_slaves_cb = undef; } else { PTDEBUG && _d('Will check slave lag on all slaves'); @@ -8641,12 +8595,11 @@ sub main { } $replica_lag = new ReplicaLagWaiter( - slaves => $slave_lag_cxns, - get_slaves_cb => $get_slaves_cb, - max_lag => $o->get('max-lag'), - oktorun => sub { return $oktorun }, - get_lag => $get_lag, - sleep => $sleep, + slaves => $slave_lag_cxns, + max_lag => $o->get('max-lag'), + oktorun => sub { return $oktorun }, + get_lag => $get_lag, + sleep => $sleep, ); my $get_status; diff --git a/lib/ReplicaLagWaiter.pm b/lib/ReplicaLagWaiter.pm index 4d0b1a35..194406e0 100644 --- a/lib/ReplicaLagWaiter.pm +++ b/lib/ReplicaLagWaiter.pm @@ -80,26 +80,6 @@ sub wait { my $worst; # most lagging slave my $pr_callback; my $pr_first_report; - - ### refresh list of slaves. In: self passed to wait() - ### Returns: new slave list - my $pr_refresh_slave_list = sub { - my ($self) = @_; - my ($slaves, $refresher) = ($self->{slaves}, $self->{get_slaves_cb}); - return if ( not defined $refresher ); - my $before = join ' ', sort map {$_->name()} @$slaves; - $slaves = $refresher->(); - my $after = join ' ', sort map {$_->name()} @$slaves; - if ($before ne $after) { - $self->{slaves} = $slaves; - printf "Slave set to watch has changed\n Was: %s\n Now: %s\n", - $before, $after; - } - return($self->{slaves}); - }; - - $slaves = $pr_refresh_slave_list->($self); - if ( $pr ) { # If you use the default Progress report callback, you'll need to # to add Transformers.pm to this tool. @@ -133,15 +113,6 @@ sub wait { my @lagged_slaves = map { {cxn=>$_, lag=>undef} } @$slaves; while ( $oktorun->() && @lagged_slaves ) { PTDEBUG && _d('Checking slave lag'); - - ### while we were waiting our list of slaves may have changed... - $slaves = $pr_refresh_slave_list->($self); - my $watched = 0; - @lagged_slaves = grep { - my $slave_name = $_->{cxn}->name(); - grep {$slave_name eq $_->name()} @{$slaves // []} - } @lagged_slaves; - for my $i ( 0..$#lagged_slaves ) { my $lag = $get_lag->($lagged_slaves[$i]->{cxn}); PTDEBUG && _d($lagged_slaves[$i]->{cxn}->name(),