Merge lp:~percona-toolkit-dev/percona-toolkit/fix-osc-repl-bug-933232.

bin/pt-online-schema-change

@@ -4110,6 +4110,13 @@ sub main {
    # #####################################################################
    # Do the online alter.
    # #####################################################################
+   if ( !$o->get('execute') ) {
+      msg("Exiting without altering $db.$tbl because you did not "
+        . "specify --execute.  Please read the tool's documentation "
+        . "carefully before using this tool.");
+      return $exit_status;
+   }
+
    msg("Starting online schema change");
    eval {
       my $sql = "";
@@ -4218,7 +4225,7 @@ sub main {
             msg($sql);
             $dbh->do($sql) unless $o->get('print');
             
-            $sql = "DROP TABLE `$db`.`$tbl`";
+            $sql = "DROP TABLE IF EXISTS `$db`.`$tbl`";
             msg($sql);
             $dbh->do($sql) unless $o->get('print');
 
@@ -4241,7 +4248,7 @@ sub main {
       $capture_sync->cleanup(%plugin_args);
 
       if ( $o->get('rename-tables') &&  $o->get('drop-old-table') ) {
-         $sql = "DROP TABLE `$db`.`$old_tbl`";
+         $sql = "DROP TABLE IF EXISTS `$db`.`$old_tbl`";
          msg($sql);
          $dbh->do($sql) unless $o->get('print');
       }
@@ -4510,12 +4517,12 @@ whether known or unknown, of using this tool.  The two main categories of risks
 are those created by the nature of the tool (e.g. read-only tools vs. read-write
 tools) and those created by bugs.
 
-pt-online-schema-change reads, writes, alters and drops tables.  Although
+pt-online-schema-change alters tables and data, therefore it is a high-risk
-it is tested, do not use it in production until you have thoroughly tested it
+tool.  Although the tool is tested and is known to work, you should not use
-in your environment!
+it in production until you have thoroughly tested it in your environment!
 
-This tool has not been tested with replication; it may break replication.
+This tool has not been tested with replication, and it can break replication
-See L<"REPLICATION">.
+if not used properly.  See L<"REPLICATION">.
 
 At the time of this release there are no known bugs that pose a serious risk.
 
@@ -4604,8 +4611,24 @@ the tool might do before actually doing it.
 
 =head1 REPLICATION
 
-In brief: update slaves first if columns are added or removed.  Certain
+If you use pt-online-schema-change to alter a table on a server with slaves,
-ALTER changes like ENGINE may not affect replication.
+be aware that:
+
+=over
+
+=item * The tool is not tested with replication
+
+=item * The tool can break replication if not used properly
+
+=item * Although the tool sets SQL_BIN_LOG=0 by default (unless L<"--bin-log"> is specified), triggers which track changes to the table being altered still write statements to the binary log
+
+=item * Replicaiton will break if you alter a table on a master that does not exist on a slave
+
+=item * Update slaves first if columns are being added or removed
+
+=item * Do not use this tool in production before testing it
+
+=back
 
 =head1 OUTPUT
 
@@ -4737,6 +4760,13 @@ swapped for the original table), then the old table can be restored.
 If altering a table with foreign key constraints, you may need to specify
 this option depending on which L<"--update-foreign-keys-method"> you choose.
 
+=item --execute
+
+Alter the table.  This option must be specified to alter the table, else
+the tool will only check the table and exit.  This helps ensure that the
+user has read the documentation and is aware of the inherent risks of using
+this tool.
+
 =item --[no]foreign-key-checks
 
 default: yes

t/pt-online-schema-change/alter_active_table.t

@@ -33,7 +33,7 @@ else {
 
 my $output  = "";
 my $cnf     = '/tmp/12345/my.sandbox.cnf';
-my @args    = ('-F', $cnf);
+my @args    = ('-F', $cnf, '--execute');
 my $exit    = 0;
 my $rows;
 

t/pt-online-schema-change/basics.t

@@ -30,13 +30,26 @@ else {
 
 my $output  = "";
 my $cnf     = '/tmp/12345/my.sandbox.cnf';
-my @args    = ('-F', $cnf);
+my @args    = ('-F', $cnf, '--execute');
 my $exit    = 0;
 my $rows;
 
 $sb->load_file('master', "t/pt-online-schema-change/samples/small_table.sql");
 $dbh->do('use mkosc');
 
+# #############################################################################
+# Tool shouldn't run without --execute (bug 933232).
+# #############################################################################
+$output = output(
+   sub { pt_online_schema_change::main('h=127.1,P=12345,u=msandbox,p=msandbox,D=mkosc,t=a') },
+);
+
+like(
+   $output,
+   qr/you did not specify --execute/,
+   "Doesn't run without --execute"
+);
+
 # #############################################################################
 # --check-tables-and-exit
 # #############################################################################

t/pt-online-schema-change/option_sanity.t

@@ -9,7 +9,7 @@ BEGIN {
 use strict;
 use warnings FATAL => 'all';
 use English qw(-no_match_vars);
-use Test::More tests => 4;
+use Test::More tests => 5;
 
 use PerconaTest;
 
@@ -44,6 +44,13 @@ like(
    "Either DSN D part or --database required"
 );
 
+$output = `$cmd --help`;
+like(
+   $output,
+   qr/--execute\s+FALSE/,
+   "--execute FALSE by default"
+);
+
 # #############################################################################
 # Done.
 # #############################################################################