percona/percona-toolkit
1
0
Fork 0
mirror of https://github.com/percona/percona-toolkit.git synced 2025-09-10 13:11:32 +00:00
Code Issues Packages Projects Releases Wiki Activity

Revert "Merge pull request #427 from percona/PT-1747"

Browse Source 
This reverts commit 25b637d4bd, reversing
changes made to 38c169031e.
This commit is contained in:
Carlos Salguero
2020-04-03 14:40:35 -03:00
parent 54f90e4bdf
commit 208fb86586
4 changed files with 316 additions and 435 deletions
Download Patch File Download Diff File Expand all files Collapse all files

280
bin/pt-online-schema-change
View File

@@ -8376,7 +8376,6 @@ my $dont_interrupt_now = 0;
my @drop_trigger_sqls; my @drop_trigger_sqls;
my @triggers_not_dropped; my @triggers_not_dropped;
my $pxc_version = '0'; my $pxc_version = '0';
my $can_drop_triggers = 1;
my $triggers_info = []; my $triggers_info = [];
@@ -9551,7 +9550,6 @@ sub main {
# called which will drop whichever triggers were created. # called which will drop whichever triggers were created.
my $drop_triggers = $o->get('drop-triggers'); my $drop_triggers = $o->get('drop-triggers');
push @cleanup_tasks, sub { push @cleanup_tasks, sub {
PTDEBUG && _d('Clean up triggers'); PTDEBUG && _d('Clean up triggers');
# --plugin hook # --plugin hook
if ( $plugin && $plugin->can('before_drop_triggers') ) { if ( $plugin && $plugin->can('before_drop_triggers') ) {
@@ -9562,12 +9560,12 @@ sub main {
); );
} }
if ( !$oktorun || !_can_drop_triggers()) { if ( !$oktorun ) {
print "Not dropping triggers because the tool was interrupted. " print "Not dropping triggers because the tool was interrupted. "
. "To drop the triggers, execute:\n" . "To drop the triggers, execute:\n"
. join("\n", @drop_trigger_sqls) . "\n"; . join("\n", @drop_trigger_sqls) . "\n";
} }
elsif ( !$drop_triggers || !_can_drop_triggers()) { elsif ( !$drop_triggers ) {
print "Not dropping triggers because --no-drop-triggers was " print "Not dropping triggers because --no-drop-triggers was "
. "specified. To drop the triggers, execute:\n" . "specified. To drop the triggers, execute:\n"
. join("\n", @drop_trigger_sqls) . "\n"; . join("\n", @drop_trigger_sqls) . "\n";
@@ -10000,76 +9998,6 @@ sub main {
$plugin->after_copy_rows(); $plugin->after_copy_rows();
} }
# #####################################################################
# Step 5a: Update foreign key constraints if there are child tables.
# #####################################################################
if ( $child_tables ) {
# --plugin hook
if ( $plugin && $plugin->can('before_update_foreign_keys') ) {
$plugin->before_update_foreign_keys();
}
eval {
if ( $alter_fk_method eq 'none' ) {
# This shouldn't happen, but in case it does we should know.
warn "The tool detected child tables but "
. "--alter-foreign-keys-method=none";
}
elsif ( $alter_fk_method eq 'rebuild_constraints' ) {
rebuild_constraints(
orig_tbl => $new_tbl,
old_tbl => $orig_tbl,
# orig_tbl => $orig_tbl,
# old_tbl => $old_tbl,
child_tables => $child_tables,
OptionParser => $o,
Quoter => $q,
Cxn => $cxn,
TableParser => $tp,
stats => \%stats,
Retry => $retry,
tries => $tries,
);
}
elsif ( $alter_fk_method eq 'drop_swap' ) {
drop_swap(
orig_tbl => $new_tbl,
new_tbl => $orig_tbl,
# orig_tbl => $orig_tbl,
# new_tbl => $new_tbl,
Cxn => $cxn,
OptionParser => $o,
stats => \%stats,
Retry => $retry,
tries => $tries,
analyze_table => $analyze_table,
);
}
elsif ( !$alter_fk_method
&& $o->has('alter-foreign-keys-method')
&& ($o->get('alter-foreign-keys-method') || '') eq 'auto' ) {
# If --alter-foreign-keys-method is 'auto' and we are on a dry run,
# $alter_fk_method is left as an empty string.
print "Not updating foreign key constraints because this is a dry run.\n";
}
else {
# This should "never" happen because we check this var earlier.
_die("Invalid --alter-foreign-keys-method: $alter_fk_method", INVALID_ALTER_FK_METHOD);
}
};
if ( $EVAL_ERROR ) {
# TODO: improve error message and handling.
$can_drop_triggers=undef;
$oktorun=undef;
_die("Error updating foreign key constraints: $EVAL_ERROR", ERROR_UPDATING_FKS);
}
# --plugin hook
if ( $plugin && $plugin->can('after_update_foreign_keys') ) {
$plugin->after_update_foreign_keys();
}
}
# ##################################################################### # #####################################################################
# XXX # XXX
# Step 5: Rename tables: orig -> old, new -> orig # Step 5: Rename tables: orig -> old, new -> orig
@@ -10164,66 +10092,66 @@ sub main {
# ##################################################################### # #####################################################################
# Step 6: Update foreign key constraints if there are child tables. # Step 6: Update foreign key constraints if there are child tables.
# ##################################################################### # #####################################################################
# if ( $child_tables ) { if ( $child_tables ) {
# # --plugin hook # --plugin hook
# if ( $plugin && $plugin->can('before_update_foreign_keys') ) { if ( $plugin && $plugin->can('before_update_foreign_keys') ) {
# $plugin->before_update_foreign_keys(); $plugin->before_update_foreign_keys();
# } }
#
# eval { eval {
# if ( $alter_fk_method eq 'none' ) { if ( $alter_fk_method eq 'none' ) {
# # This shouldn't happen, but in case it does we should know. # This shouldn't happen, but in case it does we should know.
# warn "The tool detected child tables but " warn "The tool detected child tables but "
# . "--alter-foreign-keys-method=none"; . "--alter-foreign-keys-method=none";
# } }
# elsif ( $alter_fk_method eq 'rebuild_constraints' ) { elsif ( $alter_fk_method eq 'rebuild_constraints' ) {
# rebuild_constraints( rebuild_constraints(
# orig_tbl => $orig_tbl, orig_tbl => $orig_tbl,
# old_tbl => $old_tbl, old_tbl => $old_tbl,
# child_tables => $child_tables, child_tables => $child_tables,
# OptionParser => $o, OptionParser => $o,
# Quoter => $q, Quoter => $q,
# Cxn => $cxn, Cxn => $cxn,
# TableParser => $tp, TableParser => $tp,
# stats => \%stats, stats => \%stats,
# Retry => $retry, Retry => $retry,
# tries => $tries, tries => $tries,
# ); );
# } }
# elsif ( $alter_fk_method eq 'drop_swap' ) { elsif ( $alter_fk_method eq 'drop_swap' ) {
# drop_swap( drop_swap(
# orig_tbl => $orig_tbl, orig_tbl => $orig_tbl,
# new_tbl => $new_tbl, new_tbl => $new_tbl,
# Cxn => $cxn, Cxn => $cxn,
# OptionParser => $o, OptionParser => $o,
# stats => \%stats, stats => \%stats,
# Retry => $retry, Retry => $retry,
# tries => $tries, tries => $tries,
# analyze_table => $analyze_table, analyze_table => $analyze_table,
# ); );
# } }
# elsif ( !$alter_fk_method elsif ( !$alter_fk_method
# && $o->has('alter-foreign-keys-method') && $o->has('alter-foreign-keys-method')
# && ($o->get('alter-foreign-keys-method') || '') eq 'auto' ) { && ($o->get('alter-foreign-keys-method') || '') eq 'auto' ) {
# # If --alter-foreign-keys-method is 'auto' and we are on a dry run, # If --alter-foreign-keys-method is 'auto' and we are on a dry run,
# # $alter_fk_method is left as an empty string. # $alter_fk_method is left as an empty string.
# print "Not updating foreign key constraints because this is a dry run.\n"; print "Not updating foreign key constraints because this is a dry run.\n";
# } }
# else { else {
# # This should "never" happen because we check this var earlier. # This should "never" happen because we check this var earlier.
# _die("Invalid --alter-foreign-keys-method: $alter_fk_method", INVALID_ALTER_FK_METHOD); _die("Invalid --alter-foreign-keys-method: $alter_fk_method", INVALID_ALTER_FK_METHOD);
# } }
# }; };
# if ( $EVAL_ERROR ) { if ( $EVAL_ERROR ) {
# # TODO: improve error message and handling. # TODO: improve error message and handling.
# _die("Error updating foreign key constraints: $EVAL_ERROR", ERROR_UPDATING_FKS); _die("Error updating foreign key constraints: $EVAL_ERROR", ERROR_UPDATING_FKS);
# } }
#
# # --plugin hook # --plugin hook
# if ( $plugin && $plugin->can('after_update_foreign_keys') ) { if ( $plugin && $plugin->can('after_update_foreign_keys') ) {
# $plugin->after_update_foreign_keys(); $plugin->after_update_foreign_keys();
# } }
# } }
# ######################################################################## # ########################################################################
# Step 7: Drop the old table. # Step 7: Drop the old table.
@@ -10297,10 +10225,6 @@ sub main {
# Subroutines. # Subroutines.
# ############################################################################ # ############################################################################
sub _can_drop_triggers {
return $can_drop_triggers;
}
sub validate_tries { sub validate_tries {
my ($o) = @_; my ($o) = @_;
my @ops = qw( my @ops = qw(
@@ -11082,8 +11006,6 @@ sub rebuild_constraints {
print ts("Rebuilding foreign key constraints...\n"); print ts("Rebuilding foreign key constraints...\n");
} }
my $foreign_key_checks;
CHILD_TABLE: CHILD_TABLE:
foreach my $child_tbl ( @$child_tables ) { foreach my $child_tbl ( @$child_tables ) {
my $table_def = $tp->get_create_table( my $table_def = $tp->get_create_table(
@@ -11141,47 +11063,26 @@ sub rebuild_constraints {
$constraint =~ s/CONSTRAINT `$fk`/CONSTRAINT `$new_fk`/; $constraint =~ s/CONSTRAINT `$fk`/CONSTRAINT `$new_fk`/;
my $sql = "DROP FOREIGN KEY `$fk`, ADD $constraint"; my $sql = "DROP FOREIGN KEY `$fk`, "
. "ADD $constraint";
push @rebuilt_constraints, $sql; push @rebuilt_constraints, $sql;
} }
my $server_version = VersionParser->new($cxn->dbh());
if ($server_version >= '5.6' && $o->get('disable-fk-checks')) {
my $row = $cxn->dbh()->selectrow_arrayref('SELECT @@foreign_key_checks');
$foreign_key_checks = @$row[0];
ts("Disabling FK checks");
$cxn->dbh()->do("SET FOREIGN_KEY_CHECKS=0");
}
my $sql = "ALTER TABLE $child_tbl->{name} " my $sql = "ALTER TABLE $child_tbl->{name} "
. join(', ', @rebuilt_constraints); . join(', ', @rebuilt_constraints);
print $sql, "\n" if $o->get('print'); print $sql, "\n" if $o->get('print');
if ($o->get('execute')) { if ( $o->get('execute') ) {
eval { osc_retry(
osc_retry( Cxn => $cxn,
Cxn => $cxn, Retry => $retry,
Retry => $retry, tries => $tries->{update_foreign_keys},
tries => $tries->{update_foreign_keys}, stats => $stats,
stats => $stats, code => sub {
code => sub { PTDEBUG && _d($sql);
PTDEBUG && _d($sql); $cxn->dbh()->do($sql);
$cxn->dbh()->do($sql); $stats->{rebuilt_constraint}++;
$stats->{rebuilt_constraint}++; },
}, );
);
};
if ($foreign_key_checks) {
ts("Re-enabling FK checks");
$cxn->dbh()->do("SET FOREIGN_KEY_CHECKS=$foreign_key_checks");
}
if ($EVAL_ERROR) {
$can_drop_triggers=undef;
$oktorun=undef;
_d("Foreing keys rebuild failed. To rebuild constraints please manually run:");
foreach my $cmd (@rebuilt_constraints) {
print "$cmd\n";
}
_die("Foreing keys were not rebuilt");
}
} }
} }
@@ -11308,11 +11209,10 @@ sub create_triggers {
"$new_tbl->{name}.$new_qcol <=> OLD.$old_qcol" "$new_tbl->{name}.$new_qcol <=> OLD.$old_qcol"
} @{$tbl_struct->{keys}->{$del_index}->{cols}} ); } @{$tbl_struct->{keys}->{$del_index}->{cols}} );
my $ignore_clause = $o->get("delete-ignore") ? "IGNORE" : "";
my $delete_trigger my $delete_trigger
= "CREATE TRIGGER `${prefix}_del` AFTER DELETE ON $orig_tbl->{name} " = "CREATE TRIGGER `${prefix}_del` AFTER DELETE ON $orig_tbl->{name} "
. "FOR EACH ROW " . "FOR EACH ROW "
. "DELETE $ignore_clause FROM $new_tbl->{name} " . "DELETE IGNORE FROM $new_tbl->{name} "
. "WHERE $del_index_cols"; . "WHERE $del_index_cols";
# --------------------------------------------------------------------------------------- # ---------------------------------------------------------------------------------------
@@ -11338,7 +11238,7 @@ sub create_triggers {
= "CREATE TRIGGER `${prefix}_upd` AFTER UPDATE ON $orig_tbl->{name} " = "CREATE TRIGGER `${prefix}_upd` AFTER UPDATE ON $orig_tbl->{name} "
. "FOR EACH ROW " . "FOR EACH ROW "
. "BEGIN " . "BEGIN "
. "DELETE $ignore_clause FROM $new_tbl->{name} WHERE !($upd_index_cols) AND $del_index_cols;" . "DELETE IGNORE FROM $new_tbl->{name} WHERE !($upd_index_cols) AND $del_index_cols;"
. "REPLACE INTO $new_tbl->{name} ($qcols) VALUES ($new_vals);" . "REPLACE INTO $new_tbl->{name} ($qcols) VALUES ($new_vals);"
. "END "; . "END ";
@@ -11637,11 +11537,6 @@ sub osc_retry {
) { ) {
# These errors/warnings can be retried, so don't print # These errors/warnings can be retried, so don't print
# a warning yet; do that in final_fail. # a warning yet; do that in final_fail.
# If we found a lock wait timeout and $tries == 0, we are in the rebuilt_constraints part
# so we should keep retrying
if ($error =~ m/Lock wait timeout exceeded/ && !$tries->{tries}) {
return 1;
}
$stats->{ error_event($error) }++; $stats->{ error_event($error) }++;
return 1; # try again return 1; # try again
} }
@@ -11964,10 +11859,6 @@ foreign keys and indexes as the original table (unless you specify differently
in your ALTER statement), but the names of the objects may be changed slightly in your ALTER statement), but the names of the objects may be changed slightly
to avoid object name collisions in MySQL and InnoDB. to avoid object name collisions in MySQL and InnoDB.
You should take into consideration that data consistency is not maintained by
default if you have FKs, and you must pass this additional new argument to
avoid data loss. Please read L<"--delete-ignore">
For safety, the tool does not modify the table unless you specify the For safety, the tool does not modify the table unless you specify the
L<"--execute"> option, which is not enabled by default. The tool supports a L<"--execute"> option, which is not enabled by default. The tool supports a
variety of other measures to prevent unwanted load or other problems, including variety of other measures to prevent unwanted load or other problems, including
@@ -12485,21 +12376,6 @@ short form: -F; type: string
Only read mysql options from the given file. You must give an absolute Only read mysql options from the given file. You must give an absolute
pathname. pathname.
=item --[no]delete-ignore
default: yes
Do not use C<IGNORE> on C<DELETE> triggers.
This is an special case that affects forign keys handling. Since C<DELETE> only return errors when
there is a problem deleting rows affected by foreign keys, it might be cases when we want to
receive such errors during the program execution to ensure FKs handling is correct.
=item --disable-fk-checks
Disable foreign keys check during rebuild constraints. This option improves the process speed
but it is risky since FKs checks will be disabled for a short period of time, allowing invalid
values in tables. Please use it carefully.
=item --[no]drop-new-table =item --[no]drop-new-table
default: yes default: yes

1
t/pt-online-schema-change/ansi_quotes.t
View File

@@ -51,6 +51,7 @@ ok(
); );
SKIP: { SKIP: {
skip "Skipping in MySQL 8.0.4-rc since there is an error in the server itself. See https://bugs.mysql.com/bug.php?id=89441", 9 if ($sandbox_version ge '8.0');
($output, $exit_status) = full_output( ($output, $exit_status) = full_output(
sub { pt_online_schema_change::main(@args, sub { pt_online_schema_change::main(@args,
"$master_dsn,D=issue26211,t=process_model_inst", "$master_dsn,D=issue26211,t=process_model_inst",

4
t/pt-online-schema-change/rename_fk_constraints.t
View File

@@ -23,6 +23,10 @@ my $master_dbh = $sb->get_dbh_for('master');
my $vp = VersionParser->new($master_dbh); my $vp = VersionParser->new($master_dbh);
if ($vp->cmp('8.0.14') > -1 && $vp->flavor() !~ m/maria/i) {
plan skip_all => 'Cannot run this test under the current MySQL version';
}
if ( !$master_dbh ) { if ( !$master_dbh ) {
plan skip_all => 'Cannot connect to sandbox master'; plan skip_all => 'Cannot connect to sandbox master';
} }

12
t/pt-online-schema-change/rename_self_ref_fk_constraints.t
View File

@@ -65,8 +65,8 @@ is_deeply(
$constraints, $constraints,
[ [
['person', '_fk_testId'], ['person', '_fk_testId'],
['test_table', 'fk_person'], ['test_table', '_fk_person'],
['test_table', 'fk_refId'], ['test_table', '__fk_refId'],
], ],
"First run adds or removes underscore from constraint names, accordingly" "First run adds or removes underscore from constraint names, accordingly"
); );
@@ -94,9 +94,9 @@ $constraints = $master_dbh->selectall_arrayref($query);
is_deeply( is_deeply(
$constraints, $constraints,
[ [
['person', '_fk_testId'], ['person', '__fk_testId'],
['test_table', 'fk_person'], ['test_table', '_fk_refId'],
['test_table', 'fk_refId'], ['test_table', '__fk_person'],
], ],
"Second run self-referencing will be one due to rebuild_constraints" "Second run self-referencing will be one due to rebuild_constraints"
); );
@@ -123,7 +123,7 @@ $constraints = $master_dbh->selectall_arrayref($query);
is_deeply( is_deeply(
$constraints, $constraints,
[ [
['person', '_fk_testId'], ['person', 'fk_testId'],
['test_table', 'fk_person'], ['test_table', 'fk_person'],
['test_table', 'fk_refId'], ['test_table', 'fk_refId'],
], ],