From ab620a67078bf2ae4fe4ca2b6f7f0a52638688e9 Mon Sep 17 00:00:00 2001 From: Sveta Smirnova Date: Fri, 29 Aug 2025 01:31:31 +0300 Subject: [PATCH] PT-2305 - pt-online-schema-change should error if server is a slave in row based replication - Implemented fix and test case - Updated documentation --- bin/pt-online-schema-change | 38 +++++++ t/pt-online-schema-change/pt-2305.t | 156 ++++++++++++++++++++++++++++ 2 files changed, 194 insertions(+) create mode 100644 t/pt-online-schema-change/pt-2305.t diff --git a/bin/pt-online-schema-change b/bin/pt-online-schema-change index 4b2fbdc1..2ea2da8c 100755 --- a/bin/pt-online-schema-change +++ b/bin/pt-online-schema-change @@ -9181,6 +9181,42 @@ sub main { channel => $o->get('channel'), ); + # Check if we are not a replica of the source server with ROW or MIXED base replication + if ( !$o->get('force') ) { + my $source = $ms->get_source_dsn($cxn->dbh(), $dsn, $dp); + if ( $source ) { + my $source_cxn = $make_cxn->(dsn => $source); + + # Check source + my $is_source_of = eval { + $ms->is_source_of($source_cxn->{dbh}, $cxn->{dbh}); + }; + + # We should not die if replica connected via tunnel or port redirection + if ( $EVAL_ERROR ) { + $EVAL_ERROR =~ m/The replica is connected to (\d+) but the source's port is \d+/; + if ( !$1 || $1 != $source->{P} ) { + $is_source_of = 0; + } + } + + if ( $is_source_of) { + my $source_binlog_format = $source_cxn->dbh()->selectrow_arrayref("SHOW GLOBAL VARIABLES LIKE 'binlog_format'"); + use Data::Dumper; + if ( uc @$source_binlog_format[1] ne 'STATEMENT' ) { + _die("Server " . $dp->as_string($cxn->dsn()) + . " is a replica of " . $dp->as_string($source_cxn->dsn()) + . " running with binary log format " + . "@${source_binlog_format[1]}, therefore we cannot guarantee " + . "that all replication updates will be applied to the new table.\n" + . "Exiting.\n" + . "If you want to bypass this check, specify option --force.", + NO_MINIMUM_REQUIREMENTS); + } + } + } + } + my $slaves_to_skip = $o->get('skip-check-replica-lag'); my $get_replicas_cb = sub { @@ -13305,6 +13341,8 @@ This option bypasses confirmation in case of using alter-foreign-keys-method = n This option also allows to use option --where without options --no-drop-new-table and --no-swap-tables. +This option also allows to bypass the safety check that prevents the tool from running on replica that is replicating from a source with binary log format ROW or MIXED. + =item --help Show help and exit. diff --git a/t/pt-online-schema-change/pt-2305.t b/t/pt-online-schema-change/pt-2305.t new file mode 100644 index 00000000..82e2bd95 --- /dev/null +++ b/t/pt-online-schema-change/pt-2305.t @@ -0,0 +1,156 @@ +#!/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 Data::Dumper; +use PerconaTest; +use Sandbox; + +require "$trunk/bin/pt-online-schema-change"; + +my $dp = new DSNParser(opts=>$dsn_opts); +my $sb = new Sandbox(basedir => '/tmp', DSNParser => $dp); +my $source_dbh = $sb->get_dbh_for('source'); +my $replica_dbh = $sb->get_dbh_for('replica1'); + +if ( !$source_dbh ) { + plan skip_all => 'Cannot connect to sandbox source'; +} +elsif ( !$replica_dbh ) { + plan skip_all => 'Cannot connect to sandbox replica1'; +} + +my $source_dsn = 'h=127.1,P=12345,u=msandbox,p=msandbox'; +my $replica_dsn = 'h=127.0.0.1,P=12346,u=msandbox,p=msandbox'; +my $output; +my $exit_code; +my $sample = "t/pt-online-schema-change/samples/"; + +my ($orig_binlog_format) = $source_dbh->selectrow_array(q{SELECT @@global.binlog_format}); + +$source_dbh->do("SET GLOBAL binlog_format = 'STATEMENT'"); + +($output, $exit_code) = full_output( + sub { pt_online_schema_change::main("$replica_dsn,D=sakila,t=actor", + '--alter', 'FORCE', '--execute', + '--alter-foreign-keys-method', 'auto') } +); + +is( + $exit_code, + 0, + "pt-online-schema-change run successfully with binlog_format=STATEMENT" +) or diag($output); + +like( + $output, + qr/Altering `sakila`.`actor`/, + "Expected output of pt-online-schema-change with binlog_format=STATEMENT" +) or diag($output); + +$source_dbh->do("SET GLOBAL binlog_format = 'ROW'"); + +($output, $exit_code) = full_output( + sub { pt_online_schema_change::main("$replica_dsn,D=sakila,t=actor", + '--alter', 'FORCE', '--execute', + '--alter-foreign-keys-method', 'auto') } +); + +is( + $exit_code, + 3, + "pt-online-schema-change failed with binlog_format=ROW" +) or diag($output); + +like( + $output, + qr/running with binary log format ROW.*Exiting./s, + "Expected output of pt-online-schema-change with binlog_format=ROW" +) or diag($output); + +($output, $exit_code) = full_output( + sub { pt_online_schema_change::main("$replica_dsn,D=sakila,t=actor", + '--alter', 'FORCE', '--execute', + '--alter-foreign-keys-method', 'auto', + '--force') } +); + +is( + $exit_code, + 0, + "pt-online-schema-change run successfully with binlog_format=ROW and --force" +) or diag($output); + +like( + $output, + qr/Altering `sakila`.`actor`/, + "Expected output of pt-online-schema-change with binlog_format=ROW and --force" +) or diag($output); + +unlike( + $output, + qr/running with binary log format ROW.*Exiting./s, + "No error for pt-online-schema-change with binlog_format=ROW and --force" +) or diag($output); + +$source_dbh->do("SET GLOBAL binlog_format = 'MIXED'"); + +($output, $exit_code) = full_output( + sub { pt_online_schema_change::main("$replica_dsn,D=sakila,t=actor", + '--alter', 'FORCE', '--execute', + '--alter-foreign-keys-method', 'auto') } +); + +is( + $exit_code, + 3, + "pt-online-schema-change failed with binlog_format=MIXED" +) or diag($output); + +like( + $output, + qr/running with binary log format MIXED.*Exiting./s, + "Expected output of pt-online-schema-change with binlog_format=MIXED" +) or diag($output); + +($output, $exit_code) = full_output( + sub { pt_online_schema_change::main("$replica_dsn,D=sakila,t=actor", + '--alter', 'FORCE', '--execute', + '--alter-foreign-keys-method', 'auto', + '--force') } +); + +is( + $exit_code, + 0, + "pt-online-schema-change run successfully with binlog_format=MIXED and --force" +) or diag($output); + +like( + $output, + qr/Altering `sakila`.`actor`/, + "Expected output of pt-online-schema-change with binlog_format=MIXED and --force" +) or diag($output); + +unlike( + $output, + qr/running with binary log format ROW.*Exiting./s, + "No error for pt-online-schema-change with binlog_format=MIXED and --force" +) or diag($output); + +# ############################################################################# +# Done. +# ############################################################################# +$source_dbh->do("SET GLOBAL binlog_format = '${orig_binlog_format}'"); +$sb->wipe_clean($source_dbh); +ok($sb->ok(), "Sandbox servers") or BAIL_OUT(__FILE__ . " broke the sandbox"); +done_testing;