Merge lp:~percona-toolkit-dev/percona-toolkit/fix-pt-osc-drop-bug-1188002 into lp:percona-toolkit/2.2

[Daniel Nichter]

[Daniel Nichter (daniel-nichter)]

review: Approve

=== modified file 'bin/pt-online-schema-change'
--- bin/pt-online-schema-change	2013-04-19 23:26:48 +0000
+++ bin/pt-online-schema-change	2013-06-07 20:57:30 +0000
@@ -7710,16 +7710,18 @@
 my $exit_status = 0;
 my $oktorun     = 1;
 my @drop_trigger_sqls;
+my @triggers_not_dropped;
 
 $OUTPUT_AUTOFLUSH = 1;
 
 sub main {
-   local @ARGV        = @_;
+   local @ARGV = @_;
 
    # Reset global vars else tests will fail.
-   $exit_status       = 0;
-   $oktorun           = 1;
-   @drop_trigger_sqls = ();
+   $exit_status          = 0;
+   $oktorun              = 1;
+   @drop_trigger_sqls    = ();
+   @triggers_not_dropped = ();
 
    my %stats = (
       INSERT => 0,
@@ -8419,6 +8421,16 @@
              . "--no-drop-new-table was specified.  To drop the new table, "
              . "execute:\n$sql\n";
       }
+      elsif ( @triggers_not_dropped ) {
+         # https://bugs.launchpad.net/percona-toolkit/+bug/1188002
+         print "Not dropping the new table $new_tbl->{name} because "
+            . "dropping these triggers failed:\n"
+            . join("\n", map { "  $_" } @triggers_not_dropped)
+            . "\nThese triggers must be dropped before dropping "
+            . "$new_tbl->{name}, else writing to $orig_tbl->{name} will "
+            . "cause MySQL error 1146 (42S02): \"Table $new_tbl->{name} "
+            . " doesn't exist\".\n";
+      }
       else {
          print "Dropping new table...\n";
          print $sql, "\n" if $o->get('print');
@@ -10025,8 +10037,7 @@
    else {
       print "Dropping triggers...\n";
    }
-
-   my @not_dropped;
+ 
    foreach my $sql ( @drop_trigger_sqls ) {
       print $sql, "\n" if $o->get('print');
       if ( $o->get('execute') ) {
@@ -10044,19 +10055,19 @@
          };
          if ( $EVAL_ERROR ) {
             warn "Error dropping trigger: $EVAL_ERROR\n";
-            push @not_dropped, $sql;
+            push @triggers_not_dropped, $sql;
             $exit_status = 1;
          }
       }
    }
 
    if ( $o->get('execute') ) {
-      if ( !@not_dropped ) {
+      if ( !@triggers_not_dropped ) {
          print "Dropped triggers OK.\n";
       }
       else {
          warn "To try dropping the triggers again, execute:\n"
-            . join("\n", @not_dropped) . "\n";
+            . join("\n", @triggers_not_dropped) . "\n";
       }
    }
 
 
=== added file 't/pt-online-schema-change/cleanup.t'
--- t/pt-online-schema-change/cleanup.t	1970-01-01 00:00:00 +0000
+++ t/pt-online-schema-change/cleanup.t	2013-06-07 20:57:30 +0000
@@ -0,0 +1,80 @@
+#!/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;
+require "$trunk/bin/pt-online-schema-change";
+require VersionParser;
+
+use Time::HiRes qw(sleep);
+use Data::Dumper;
+$Data::Dumper::Indent    = 1;
+$Data::Dumper::Sortkeys  = 1;
+$Data::Dumper::Quotekeys = 0;
+
+my $dp         = new DSNParser(opts=>$dsn_opts);
+my $sb         = new Sandbox(basedir => '/tmp', DSNParser => $dp);
+my $dbh1 = $sb->get_dbh_for('master');
+my $dbh2 = $sb->get_dbh_for('master');
+
+if ( !$dbh1 || !$dbh2 ) {
+   plan skip_all => 'Cannot connect to sandbox master';
+}
+elsif ( $sandbox_version lt '5.5' ) {
+   plan skip_all => "Metadata locks require MySQL 5.5 and newer";
+}
+
+my $output;
+my $master_dsn = $sb->dsn_for('master');
+my $sample     = "t/pt-online-schema-change/samples";
+my $plugin     = "$trunk/$sample/plugins";
+my $exit;
+my $rows;
+
+# Loads pt_osc.t with cols id (pk), c (unique index),, d.
+$sb->load_file('master', "$sample/basic_no_fks_innodb.sql");
+
+# #############################################################################
+# Meta-block on create_triggers.
+# #############################################################################
+
+($output) = full_output(
+   sub { pt_online_schema_change::main(
+      "$master_dsn,D=pt_osc,t=t",
+      qw(--statistics --execute --tries drop_triggers:1:0.01),
+      qw(--set-vars lock_wait_timeout=1 --print --no-swap-tables),
+      '--plugin', "$plugin/make_drop_trigger_fail.pm",
+   )},
+   stderr => 1,
+);
+
+my $triggers = $dbh1->selectall_arrayref("SHOW TRIGGERS FROM pt_osc");
+is(
+   @$triggers,
+   3,
+   "Bug 1188002: triggers not dropped"
+) or diag(Dumper($triggers));
+
+my $tables = $dbh1->selectall_arrayref("SHOW TABLES FROM pt_osc");
+is_deeply(
+   $tables,
+   [ ['_t_new'], ['t'] ],
+   "Bug 1188002: new table not dropped"
+) or diag(Dumper($tables));
+
+# #############################################################################
+# Done.
+# #############################################################################
+$sb->wipe_clean($dbh1);
+ok($sb->ok(), "Sandbox servers") or BAIL_OUT(__FILE__ . " broke the sandbox");
+done_testing;
 
=== added file 't/pt-online-schema-change/samples/plugins/make_drop_trigger_fail.pm'
--- t/pt-online-schema-change/samples/plugins/make_drop_trigger_fail.pm	1970-01-01 00:00:00 +0000
+++ t/pt-online-schema-change/samples/plugins/make_drop_trigger_fail.pm	2013-06-07 20:57:30 +0000
@@ -0,0 +1,36 @@
+package pt_online_schema_change_plugin;
+
+use strict;
+use warnings FATAL => 'all';
+use English qw(-no_match_vars);
+use constant PTDEBUG => $ENV{PTDEBUG} || 0;
+
+sub new {
+   my ($class, %args) = @_;
+   my $self = { %args };
+   return bless $self, $class;
+}
+
+sub init {
+   my ($self, %args) = @_;
+   print "PLUGIN: init()\n";
+   $self->{orig_tbl} = $args{orig_tbl};
+}
+
+sub before_drop_triggers {
+   my ($self, %args) = @_;
+   print "PLUGIN: before_drop_triggers()\n";
+
+   my $dbh      = $self->{aux_cxn}->dbh;
+   my $orig_tbl = $self->{orig_tbl};
+
+   # Start a trx and get a metadata lock on the table being altered.
+   $dbh->do('SET autocommit=0');
+   $dbh->{AutoCommit} = 0;
+   $dbh->do("START TRANSACTION");
+   $dbh->do("SELECT * FROM " . $orig_tbl->{name});
+
+   return;
+}
+
+1;