diff --git a/bin/pt-online-schema-change b/bin/pt-online-schema-change index fd7af631..22faf2e8 100755 --- a/bin/pt-online-schema-change +++ b/bin/pt-online-schema-change @@ -8437,25 +8437,26 @@ my %ignore_code = ( $OUTPUT_AUTOFLUSH = 1; use constant { - INVALID_PARAMETERS => 1, - UNSUPORTED_MYSQL_VERSION => 2, - NO_MINIMUM_REQUIREMENTS => 3, - NO_PRIMARY_OR_UNIQUE_KEY => 4, - INVALID_PLUGIN_FILE => 5, - INVALID_ALTER_FK_METHOD => 6, - INVALID_KEY_SIZE => 7, - CANNOT_DETERMINE_KEY_SIZE => 9, - NOT_SAFE_TO_ASCEND => 9, - ERROR_CREATING_NEW_TABLE => 10, - ERROR_ALTERING_TABLE => 11, - ERROR_CREATING_TRIGGERS => 12, - ERROR_RESTORING_TRIGGERS => 13, - ERROR_SWAPPING_TABLES => 14, - ERROR_UPDATING_FKS => 15, - ERROR_DROPPING_OLD_TABLE => 16, - UNSUPORTED_OPERATION => 17, - MYSQL_CONNECTION_ERROR => 18, - LOST_MYSQL_CONNECTION => 19, + INVALID_PARAMETERS => 1, + UNSUPORTED_MYSQL_VERSION => 2, + NO_MINIMUM_REQUIREMENTS => 3, + NO_PRIMARY_OR_UNIQUE_KEY => 4, + INVALID_PLUGIN_FILE => 5, + INVALID_ALTER_FK_METHOD => 6, + INVALID_KEY_SIZE => 7, + CANNOT_DETERMINE_KEY_SIZE => 9, + NOT_SAFE_TO_ASCEND => 9, + ERROR_CREATING_NEW_TABLE => 10, + ERROR_ALTERING_TABLE => 11, + ERROR_CREATING_TRIGGERS => 12, + ERROR_RESTORING_TRIGGERS => 13, + ERROR_SWAPPING_TABLES => 14, + ERROR_UPDATING_FKS => 15, + ERROR_DROPPING_OLD_TABLE => 16, + UNSUPORTED_OPERATION => 17, + MYSQL_CONNECTION_ERROR => 18, + LOST_MYSQL_CONNECTION => 19, + ERROR_CREATING_REVERSE_TRIGGERS => 20, }; sub _die { @@ -8568,6 +8569,21 @@ sub main { $o->set('drop-triggers', 1); } + if ( $o->get('reverse-triggers') ) { + if ($o->get('drop-old-table')) { + my $msg = '--reverse-triggers needs --no-drop-old-table'; + _die($msg, INVALID_PARAMETERS); + } + # if (!$o->get('swap-tables')) { + # my $msg = 'Cannot use --reverse-triggers with --no-swap-tables'; + # _die($msg, INVALID_PARAMETERS); + # } + if ($o->get('preserve-triggers')) { + my $msg = 'Cannot use --reverse-triggers with --preserve-triggers'; + _die($msg, INVALID_PARAMETERS); + } + } + if ( !$o->get('help') ) { if ( @ARGV ) { $o->save_error('Specify only one DSN on the command line'); @@ -9676,6 +9692,41 @@ sub main { _die("Error creating triggers: $EVAL_ERROR", ERROR_CREATING_TRIGGERS); }; + if ( $o->get('reverse-triggers') ) { + print "Adding reverse triggers\n"; + eval { + my $old_tbl_name = '_'.$orig_tbl->{tbl}.'_old'; + my $new_tbl_name = '_'.$orig_tbl->{tbl}.'_new'; + + my $old_tbl = { + db => $orig_tbl->{db}, + name => '`'.$orig_tbl->{db}.'`.`'.$old_tbl_name.'`', + tbl => $old_tbl_name, + }; + my $new_tbl = { + db => $orig_tbl->{db}, + name => '`'.$orig_tbl->{db}.'`.`'.$new_tbl_name.'`', + tbl => $new_tbl_name, + }; + my $triggers=create_triggers( + orig_tbl => $new_tbl, + new_tbl => $old_tbl, + del_tbl => $orig_tbl, + columns => \@common_cols, + Cxn => $cxn, + Quoter => $q, + OptionParser => $o, + Retry => $retry, + tries => $tries, + stats => \%stats, + reverse_triggers => 1, + ); + }; + if ( $EVAL_ERROR ) { + _die("Error creating reverse triggers: $EVAL_ERROR", ERROR_CREATING_REVERSE_TRIGGERS); + }; + } + # --plugin hook if ( $plugin && $plugin->can('after_create_triggers') ) { $plugin->after_create_triggers(); @@ -10125,7 +10176,6 @@ sub main { my $old_tbl; if ( $o->get('swap-tables') ) { - eval { $old_tbl = swap_tables( orig_tbl => $orig_tbl, @@ -10149,6 +10199,7 @@ sub main { ERROR_SWAPPING_TABLES); } } + $orig_tbl->{swapped} = 1; # flag for cleanup tasks PTDEBUG && _d('Old table:', Dumper($old_tbl)); @@ -10857,7 +10908,7 @@ sub swap_tables { my $sql = "RENAME TABLE $orig_tbl->{name} " . "TO " . $q->quote($orig_tbl->{db}, $table_name) - . ", $new_tbl->{name} TO $orig_tbl->{name}"; + . ", $new_tbl->{name} TO $orig_tbl->{name}" ; eval { osc_retry( @@ -11271,7 +11322,8 @@ sub create_triggers { # Create a unique trigger name prefix based on the orig table name # so multiple instances of the tool can run on different tables. - my $prefix = 'pt_osc_' . $orig_tbl->{db} . '_' . $orig_tbl->{tbl}; + my $pp = $args{reverse_triggers} ? "rt_" : ''; + my $prefix = $pp.'pt_osc_' . $orig_tbl->{db} . '_' . $orig_tbl->{tbl}; $prefix =~ s/\W/_/g; if ( length($prefix) > 60 ) { @@ -11301,8 +11353,11 @@ sub create_triggers { my $delete_trigger = "CREATE TRIGGER `${prefix}_del` AFTER DELETE ON $orig_tbl->{name} " . "FOR EACH ROW " + . "BEGIN " + . "DECLARE CONTINUE HANDLER FOR 1146 begin end; " . "DELETE IGNORE FROM $new_tbl->{name} " - . "WHERE $del_index_cols"; + . "WHERE $del_index_cols; " + . "END "; # --------------------------------------------------------------------------------------- my $qcols = join(', ', map { $q->quote($_->{new}) } @$cols); @@ -11311,7 +11366,10 @@ sub create_triggers { my $insert_trigger = "CREATE TRIGGER `${prefix}_ins` AFTER INSERT ON $orig_tbl->{name} " . "FOR EACH ROW " - . "REPLACE INTO $new_tbl->{name} ($qcols) VALUES ($new_vals)"; + . "BEGIN " + . "DECLARE CONTINUE HANDLER FOR 1146 begin end; " + . "REPLACE INTO $new_tbl->{name} ($qcols) VALUES ($new_vals);" + . "END "; # --------------------------------------------------------------------------------------- my $upd_index_cols = join(" AND ", map { @@ -11327,8 +11385,9 @@ sub create_triggers { = "CREATE TRIGGER `${prefix}_upd` AFTER UPDATE ON $orig_tbl->{name} " . "FOR EACH ROW " . "BEGIN " - . "DELETE IGNORE FROM $new_tbl->{name} WHERE !($upd_index_cols) AND $del_index_cols;" - . "REPLACE INTO $new_tbl->{name} ($qcols) VALUES ($new_vals);" + . "DECLARE CONTINUE HANDLER FOR 1146 begin end; " + . "DELETE IGNORE FROM $new_tbl->{name} WHERE !($upd_index_cols) AND $del_index_cols; " + . "REPLACE INTO $new_tbl->{name} ($qcols) VALUES ($new_vals); " . "END "; $triggers_info = [ @@ -11344,19 +11403,6 @@ sub create_triggers { suffix => 'ins', event => 'INSERT', time => 'AFTER', orig_triggers => [], new_trigger_sql => $insert_trigger, new_trigger_name => "${prefix}_ins", }, - - { - suffix => 'delb', event => 'DELETE', time => 'BEFORE', orig_triggers => [], - new_trigger_sql => '', new_trigger_name => '' - }, - { - suffix => 'updb', event => 'UPDATE', time => 'BEFORE', orig_triggers => [], - new_trigger_sql => '', new_trigger_name => '' - }, - { - suffix => 'insb', event => 'INSERT', time => 'BEFORE', orig_triggers => [], - new_trigger_sql => '', new_trigger_name => '' - }, ]; $cxn->connect(); @@ -11413,28 +11459,39 @@ sub create_triggers { } my @trigger_names; - @drop_trigger_sqls = (); foreach my $trigger_info ( @$triggers_info ) { - next if !$trigger_info->{new_trigger_sql}; - if ( $o->get('execute') ) { - osc_retry( - Cxn => $cxn, - Retry => $retry, - tries => $tries->{create_triggers}, - stats => $stats, - code => sub { - PTDEBUG && _d($trigger_info->{new_trigger_sql}); - $cxn->dbh()->do($trigger_info->{new_trigger_sql}); - }, - ); + if ($o->get('execute') && !$args{dont}) { + osc_retry( + Cxn => $cxn, + Retry => $retry, + tries => $tries->{create_triggers}, + stats => $stats, + code => sub { + PTDEBUG && _d($trigger_info->{new_trigger_sql}); + $cxn->dbh()->do($trigger_info->{new_trigger_sql}); + }, + ); } # Only save the trigger once it has been created # (or faked to be created) so if the 2nd trigger # fails to create, we know to only drop the 1st. push @trigger_names, $trigger_info->{new_trigger_name}; - push @drop_trigger_sqls, - "DROP TRIGGER IF EXISTS " . $q->quote($orig_tbl->{db}, $trigger_info->{new_trigger_name}); + + if (!$args{'reverse_triggers'}) { + push @drop_trigger_sqls, + "DROP TRIGGER IF EXISTS " . $q->quote($orig_tbl->{db}, $trigger_info->{new_trigger_name}); + } + if ($o->get('print')) { + print "-----------------------------------------------------------\n"; + print "Skipped trigger creation: \n" if $o->get('dry-run'); + print "Event : $trigger_info->{event} \n"; + print "Name : $trigger_info->{new_trigger_name} \n"; + print "SQL : $trigger_info->{new_trigger_sql} \n"; + print "Suffix: $trigger_info->{suffix} \n"; + print "Time : $trigger_info->{time} \n"; + print "-----------------------------------------------------------\n"; + } } if ( $o->get('execute') ) { @@ -12817,6 +12874,16 @@ ignored. You can change the list of hosts while OSC is executing: if you change the contents of the DSN table, OSC will pick it up very soon. +=item --reverse-triggers + +Copy the triggers added during the copy in reverse order. Commands in the new table will be +reflected in the old table. This is for safety in case something fails, you can make the old +table to receive the updates. This option requires C<--no-drop-old-table>. +Reverse triggers are being added AFTER the table rename/swap and after all the clean up +processes. This means there is a short period of time between the working triggers deletion and +the new triggers are being put in place. During that time, inserts, updates and deletes to the +new table won't be replicated to the old one. + =item --skip-check-slave-lag type: DSN; repeatable: yes @@ -13270,25 +13337,26 @@ of output. =head1 EXIT STATUS - INVALID_PARAMETERS = 1 - UNSUPORTED_MYSQL_VERSION = 2 - NO_MINIMUM_REQUIREMENTS = 3 - NO_PRIMARY_OR_UNIQUE_KEY = 4 - INVALID_PLUGIN_FILE = 5 - INVALID_ALTER_FK_METHOD = 6 - INVALID_KEY_SIZE = 7 - CANNOT_DETERMINE_KEY_SIZE = 9 - NOT_SAFE_TO_ASCEND = 9 - ERROR_CREATING_NEW_TABLE = 10 - ERROR_ALTERING_TABLE = 11 - ERROR_CREATING_TRIGGERS = 12 - ERROR_RESTORING_TRIGGERS = 13 - ERROR_SWAPPING_TABLES = 14 - ERROR_UPDATING_FKS = 15 - ERROR_DROPPING_OLD_TABLE = 16 - UNSUPORTED_OPERATION = 17 - MYSQL_CONNECTION_ERROR = 18 - LOST_MYSQL_CONNECTION = 19 + INVALID_PARAMETERS = 1 + UNSUPORTED_MYSQL_VERSION = 2 + NO_MINIMUM_REQUIREMENTS = 3 + NO_PRIMARY_OR_UNIQUE_KEY = 4 + INVALID_PLUGIN_FILE = 5 + INVALID_ALTER_FK_METHOD = 6 + INVALID_KEY_SIZE = 7 + CANNOT_DETERMINE_KEY_SIZE = 9 + NOT_SAFE_TO_ASCEND = 9 + ERROR_CREATING_NEW_TABLE = 10 + ERROR_ALTERING_TABLE = 11 + ERROR_CREATING_TRIGGERS = 12 + ERROR_RESTORING_TRIGGERS = 13 + ERROR_SWAPPING_TABLES = 14 + ERROR_UPDATING_FKS = 15 + ERROR_DROPPING_OLD_TABLE = 16 + UNSUPORTED_OPERATION = 17 + MYSQL_CONNECTION_ERROR = 18 + LOST_MYSQL_CONNECTION = 19 + ERROR_CREATING_REVERSE_TRIGGERS = 20 =head1 SYSTEM REQUIREMENTS diff --git a/t/pt-online-schema-change/PT-1905_reverse_pt_osc_triggers.t b/t/pt-online-schema-change/PT-1905_reverse_pt_osc_triggers.t new file mode 100644 index 00000000..4b3eb10a --- /dev/null +++ b/t/pt-online-schema-change/PT-1905_reverse_pt_osc_triggers.t @@ -0,0 +1,171 @@ +#!/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 threads; + +use English qw(-no_match_vars); +use Test::More; + +use Data::Dumper; +use PerconaTest; +use Sandbox; +use SqlModes; +use File::Temp qw/ tempdir /; +use threads; +use Time::HiRes qw( usleep ); +use constant PTDEBUG => $ENV{PTDEBUG} || 0; + +plan tests => 5; + +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 $master_dsn = 'h=127.1,P=12345,u=msandbox,p=msandbox'; + +if ( !$master_dbh ) { + plan skip_all => 'Cannot connect to sandbox master'; +} + +$sb->load_file('master', "t/pt-online-schema-change/samples/pt-153.sql"); + +sub start_thread { + my ($dsn_opts) = @_; + my $dp = new DSNParser(opts=>$dsn_opts); + my $sb = new Sandbox(basedir => '/tmp', DSNParser => $dp); + my $dbh = $sb->get_dbh_for('master'); + PTDEBUG && diag("Thread started..."); + + local $SIG{KILL} = sub { + PTDEBUG && diag("Exit thread"); + threads->exit ; + }; + + for (my $i=0; $i < 1_000_000; $i++) { + eval { + $dbh->do("INSERT INTO test.t1 (f2,f4) VALUES (1,3)"); + }; + } +} + +my $thr = threads->create('start_thread', $dsn_opts); +threads->yield(); + +# The sandbox servers run with lock_wait_timeout=3 and it's not dynamic +# so we need to specify --set-vars innodb_lock_wait_timeout=3 else the +# tool will die. +my @args = (qw(--set-vars innodb_lock_wait_timeout=3)); +my $output; +my $exit_status; + +sleep(1); # Let is generate some rows. + +($output, $exit_status) = full_output( + sub { pt_online_schema_change::main(@args, "$master_dsn,D=test,t=t1", + '--execute', + '--alter', "DROP COLUMN f3", + '--reverse-triggers', '--no-drop-old-table', + ), + }, +); + +is( + $exit_status, + 0, + "Exit status is 0", +); + +like( + $output, + qr/Successfully altered `test`.`t1`/s, + "Successfully altered `test`.`t1`", +); + +my $triggers_sql = "SELECT TRIGGER_SCHEMA, TRIGGER_NAME, DEFINER, EVENT_OBJECT_SCHEMA, EVENT_OBJECT_TABLE, ACTION_STATEMENT, SQL_MODE, " + . " CHARACTER_SET_CLIENT, COLLATION_CONNECTION, EVENT_MANIPULATION, ACTION_TIMING " + . " FROM INFORMATION_SCHEMA.TRIGGERS " + . " WHERE TRIGGER_SCHEMA = 'test'"; + +my $rows = $master_dbh->selectall_arrayref($triggers_sql, {Slice =>{}}); + +is_deeply ( + want_triggers(), + $rows, + "Reverse triggers in place", +); + +# Kill the thread otherwise the count will be different because we are running 2 separate queries. +$thr->kill('KILL'); +$thr->join(); + +my $new_count = $master_dbh->selectrow_hashref('SELECT COUNT(*) AS cnt FROM test.t1'); +my $old_count = $master_dbh->selectrow_hashref('SELECT COUNT(*) AS cnt FROM test._t1_old'); + +is ( + $old_count->{cnt}, + $new_count->{cnt}, + "Rows count is correct", +); + +$master_dbh->do("DROP DATABASE IF EXISTS test"); + +# ############################################################################# +# Done. +# ############################################################################# +$sb->wipe_clean($master_dbh); +ok($sb->ok(), "Sandbox servers") or BAIL_OUT(__FILE__ . " broke the sandbox"); +done_testing; + + +# Heres just to make the test more clear. +sub want_triggers { + return [ + { + action_statement => 'BEGIN DECLARE CONTINUE HANDLER FOR 1146 begin end; REPLACE INTO `test`.`_t1_old` (`id`, `f2`, `f4`) VALUES (NEW.`id`, NEW.`f2`, NEW.`f4`);END', + action_timing => 'AFTER', + character_set_client => 'latin1', + collation_connection => 'latin1_swedish_ci', + definer => 'msandbox@%', + event_manipulation => 'INSERT', + event_object_schema => 'test', + event_object_table => 't1', + sql_mode => 'ONLY_FULL_GROUP_BY,NO_AUTO_VALUE_ON_ZERO,STRICT_TRANS_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_AUTO_CREATE_USER,NO_ENGINE_SUBSTITUTION', + trigger_name => 'rt_pt_osc_test__t1_new_ins', + trigger_schema => 'test' + }, + { + action_statement => 'BEGIN DECLARE CONTINUE HANDLER FOR 1146 begin end; DELETE IGNORE FROM `test`.`_t1_old` WHERE !(OLD.`id` <=> NEW.`id`) AND `test`.`_t1_old`.`id` <=> OLD.`id`; REPLACE INTO `test`.`_t1_old` (`id`, `f2`, `f4`) VALUES (NEW.`id`, NEW.`f2`, NEW.`f4`); END', + action_timing => 'AFTER', + character_set_client => 'latin1', + collation_connection => 'latin1_swedish_ci', + definer => 'msandbox@%', + event_manipulation => 'UPDATE', + event_object_schema => 'test', + event_object_table => 't1', + sql_mode => 'ONLY_FULL_GROUP_BY,NO_AUTO_VALUE_ON_ZERO,STRICT_TRANS_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_AUTO_CREATE_USER,NO_ENGINE_SUBSTITUTION', + trigger_name => 'rt_pt_osc_test__t1_new_upd', + trigger_schema => 'test' + }, + { + action_statement => 'BEGIN DECLARE CONTINUE HANDLER FOR 1146 begin end; DELETE IGNORE FROM `test`.`_t1_old` WHERE `test`.`_t1_old`.`id` <=> OLD.`id`; END', + action_timing => 'AFTER', + character_set_client => 'latin1', + collation_connection => 'latin1_swedish_ci', + definer => 'msandbox@%', + event_manipulation => 'DELETE', + event_object_schema => 'test', + event_object_table => 't1', + sql_mode => 'ONLY_FULL_GROUP_BY,NO_AUTO_VALUE_ON_ZERO,STRICT_TRANS_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_AUTO_CREATE_USER,NO_ENGINE_SUBSTITUTION', + trigger_name => 'rt_pt_osc_test__t1_new_del', + trigger_schema => 'test' + }, + ]; +} diff --git a/t/pt-online-schema-change/preserve_triggers.t b/t/pt-online-schema-change/preserve_triggers.t index d3c97974..abbd6234 100644 --- a/t/pt-online-schema-change/preserve_triggers.t +++ b/t/pt-online-schema-change/preserve_triggers.t @@ -459,16 +459,6 @@ $sb->do_as_root("master", q/set sql_log_bin=0/); $sb->do_as_root("master", q/DROP USER 'slave_user'/); $sb->do_as_root("master", q/set sql_log_bin=1/); -test_alter_table( - name => "--slave-user --slave-password", - file => "basic_no_fks_innodb.sql", - table => "pt_osc.t", - test_type => "add_col", - new_col => "bar", - cmds => [ - qw(--execute --slave-user slave_user --slave-password slave_password), '--alter', 'ADD COLUMN bar INT', - ], -); # ############################################################################# # Done. # ############################################################################# diff --git a/t/pt-online-schema-change/samples/pt-153.sql b/t/pt-online-schema-change/samples/pt-153.sql index 47b50354..cae106a5 100644 --- a/t/pt-online-schema-change/samples/pt-153.sql +++ b/t/pt-online-schema-change/samples/pt-153.sql @@ -2,11 +2,10 @@ DROP DATABASE IF EXISTS test; CREATE DATABASE test; USE test; CREATE TABLE test.t1 ( -id int, +id int AUTO_INCREMENT PRIMARY KEY, f2 int, f3 int NULL, -f4 int, -PRIMARY KEY (id) +f4 int ); INSERT INTO test.t1 VALUES