From c5cafbb4869dbbeffd8a291413309be3d0409250 Mon Sep 17 00:00:00 2001 From: Sveta Smirnova Date: Thu, 11 Apr 2024 21:47:11 +0300 Subject: [PATCH] PT-2231 - pt-osc + PTDEBUG=1 fails with Use of uninitialized value in concatenation (.) or string at ./pt-online-schema-change line 4309. - Re-implemented fix for PT-1799 properly - Added test case - Updated modules --- bin/pt-archiver | 3 +- bin/pt-heartbeat | 3 +- bin/pt-kill | 3 +- bin/pt-online-schema-change | 3 +- bin/pt-query-digest | 3 +- bin/pt-slave-find | 3 +- bin/pt-slave-restart | 3 +- bin/pt-table-checksum | 3 +- bin/pt-table-sync | 3 +- lib/MasterSlave.pm | 3 +- t/pt-online-schema-change/pt-2231.t | 63 +++++++++++++++++++++++++++++ 11 files changed, 83 insertions(+), 10 deletions(-) create mode 100644 t/pt-online-schema-change/pt-2231.t diff --git a/bin/pt-archiver b/bin/pt-archiver index 563d6b6d..2bf7a99f 100755 --- a/bin/pt-archiver +++ b/bin/pt-archiver @@ -3781,7 +3781,8 @@ sub recurse_to_slaves { my $slave_dsn = $dsn; if ($slave_user) { $slave_dsn->{u} = $slave_user; - PTDEBUG && _d("Using slave user $slave_user on ".$slave_dsn->{h}.":".$slave_dsn->{P}); + PTDEBUG && _d("Using slave user $slave_user on " + . $slave_dsn->{h} . ":" . ( $slave_dsn->{P} ? $slave_dsn->{P} : "")); } if ($slave_password) { $slave_dsn->{p} = $slave_password; diff --git a/bin/pt-heartbeat b/bin/pt-heartbeat index e03c9001..a0cf5c02 100755 --- a/bin/pt-heartbeat +++ b/bin/pt-heartbeat @@ -242,7 +242,8 @@ sub recurse_to_slaves { my $slave_dsn = $dsn; if ($slave_user) { $slave_dsn->{u} = $slave_user; - PTDEBUG && _d("Using slave user $slave_user on ".$slave_dsn->{h}.":".$slave_dsn->{P}); + PTDEBUG && _d("Using slave user $slave_user on " + . $slave_dsn->{h} . ":" . ( $slave_dsn->{P} ? $slave_dsn->{P} : "")); } if ($slave_password) { $slave_dsn->{p} = $slave_password; diff --git a/bin/pt-kill b/bin/pt-kill index 38cee058..3c2da447 100755 --- a/bin/pt-kill +++ b/bin/pt-kill @@ -4058,7 +4058,8 @@ sub recurse_to_slaves { my $slave_dsn = $dsn; if ($slave_user) { $slave_dsn->{u} = $slave_user; - PTDEBUG && _d("Using slave user $slave_user on ".$slave_dsn->{h}.":".$slave_dsn->{P}); + PTDEBUG && _d("Using slave user $slave_user on " + . $slave_dsn->{h} . ":" . ( $slave_dsn->{P} ? $slave_dsn->{P} : "")); } if ($slave_password) { $slave_dsn->{p} = $slave_password; diff --git a/bin/pt-online-schema-change b/bin/pt-online-schema-change index fea52be9..42967021 100755 --- a/bin/pt-online-schema-change +++ b/bin/pt-online-schema-change @@ -4346,7 +4346,8 @@ sub recurse_to_slaves { my $slave_dsn = $dsn; if ($slave_user) { $slave_dsn->{u} = $slave_user; - PTDEBUG && _d("Using slave user $slave_user on ".$slave_dsn->{h}.":".$slave_dsn->{P}); + PTDEBUG && _d("Using slave user $slave_user on " + . $slave_dsn->{h} . ":" . ( $slave_dsn->{P} ? $slave_dsn->{P} : "")); } if ($slave_password) { $slave_dsn->{p} = $slave_password; diff --git a/bin/pt-query-digest b/bin/pt-query-digest index 1ccb8e5a..f12b0063 100755 --- a/bin/pt-query-digest +++ b/bin/pt-query-digest @@ -10652,7 +10652,8 @@ sub recurse_to_slaves { my $slave_dsn = $dsn; if ($slave_user) { $slave_dsn->{u} = $slave_user; - PTDEBUG && _d("Using slave user $slave_user on ".$slave_dsn->{h}.":".$slave_dsn->{P}); + PTDEBUG && _d("Using slave user $slave_user on " + . $slave_dsn->{h} . ":" . ( $slave_dsn->{P} ? $slave_dsn->{P} : "")); } if ($slave_password) { $slave_dsn->{p} = $slave_password; diff --git a/bin/pt-slave-find b/bin/pt-slave-find index 5c6557d7..3a2b9295 100755 --- a/bin/pt-slave-find +++ b/bin/pt-slave-find @@ -2383,7 +2383,8 @@ sub recurse_to_slaves { my $slave_dsn = $dsn; if ($slave_user) { $slave_dsn->{u} = $slave_user; - PTDEBUG && _d("Using slave user $slave_user on ".$slave_dsn->{h}.":".$slave_dsn->{P}); + PTDEBUG && _d("Using slave user $slave_user on " + . $slave_dsn->{h} . ":" . ( $slave_dsn->{P} ? $slave_dsn->{P} : "")); } if ($slave_password) { $slave_dsn->{p} = $slave_password; diff --git a/bin/pt-slave-restart b/bin/pt-slave-restart index 046a2635..284f0367 100755 --- a/bin/pt-slave-restart +++ b/bin/pt-slave-restart @@ -2794,7 +2794,8 @@ sub recurse_to_slaves { my $slave_dsn = $dsn; if ($slave_user) { $slave_dsn->{u} = $slave_user; - PTDEBUG && _d("Using slave user $slave_user on ".$slave_dsn->{h}.":".$slave_dsn->{P}); + PTDEBUG && _d("Using slave user $slave_user on " + . $slave_dsn->{h} . ":" . ( $slave_dsn->{P} ? $slave_dsn->{P} : "")); } if ($slave_password) { $slave_dsn->{p} = $slave_password; diff --git a/bin/pt-table-checksum b/bin/pt-table-checksum index e0185f9a..78570d60 100755 --- a/bin/pt-table-checksum +++ b/bin/pt-table-checksum @@ -5298,7 +5298,8 @@ sub recurse_to_slaves { my $slave_dsn = $dsn; if ($slave_user) { $slave_dsn->{u} = $slave_user; - PTDEBUG && _d("Using slave user $slave_user on ".$slave_dsn->{h}.":".$slave_dsn->{P}); + PTDEBUG && _d("Using slave user $slave_user on " + . $slave_dsn->{h} . ":" . ( $slave_dsn->{P} ? $slave_dsn->{P} : "")); } if ($slave_password) { $slave_dsn->{p} = $slave_password; diff --git a/bin/pt-table-sync b/bin/pt-table-sync index dce6fa8d..55ca97cd 100755 --- a/bin/pt-table-sync +++ b/bin/pt-table-sync @@ -6812,7 +6812,8 @@ sub recurse_to_slaves { my $slave_dsn = $dsn; if ($slave_user) { $slave_dsn->{u} = $slave_user; - PTDEBUG && _d("Using slave user $slave_user on ".$slave_dsn->{h}.":".$slave_dsn->{P}); + PTDEBUG && _d("Using slave user $slave_user on " + . $slave_dsn->{h} . ":" . ( $slave_dsn->{P} ? $slave_dsn->{P} : "")); } if ($slave_password) { $slave_dsn->{p} = $slave_password; diff --git a/lib/MasterSlave.pm b/lib/MasterSlave.pm index 007c4d6d..135411dc 100644 --- a/lib/MasterSlave.pm +++ b/lib/MasterSlave.pm @@ -181,7 +181,8 @@ sub recurse_to_slaves { my $slave_dsn = $dsn; if ($slave_user) { $slave_dsn->{u} = $slave_user; - PTDEBUG && _d("Using slave user $slave_user on ".$slave_dsn->{h}.":".$slave_dsn->{P}); + PTDEBUG && _d("Using slave user $slave_user on " + . $slave_dsn->{h} . ":" . ( $slave_dsn->{P} ? $slave_dsn->{P} : "")); } if ($slave_password) { $slave_dsn->{p} = $slave_password; diff --git a/t/pt-online-schema-change/pt-2231.t b/t/pt-online-schema-change/pt-2231.t new file mode 100644 index 00000000..4ff53406 --- /dev/null +++ b/t/pt-online-schema-change/pt-2231.t @@ -0,0 +1,63 @@ +#!/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"; + +my $dp = new DSNParser(opts=>$dsn_opts); +my $sb = new Sandbox(basedir => '/tmp', DSNParser => $dp); +my $master_dbh = $sb->get_dbh_for('master'); +my $slave1_dbh = $sb->get_dbh_for('slave1'); +my $slave2_dbh = $sb->get_dbh_for('slave2'); + +if ( !$master_dbh ) { + plan skip_all => 'Cannot connect to sandbox master'; +} +elsif ( !$slave1_dbh ) { + plan skip_all => 'Cannot connect to sandbox slave1'; +} +elsif ( !$slave1_dbh ) { + plan skip_all => 'Cannot connect to sandbox slave2'; +} +else { + plan tests => 2; +} + +$sb->load_file('master', "t/pt-online-schema-change/samples/basic_no_fks.sql"); + +$sb->wait_for_slaves(); + +# Save original PTDEBUG env because we modify it below. +my $dbg = $ENV{PTDEBUG}; + +$ENV{PTDEBUG} = 1; + +my $output = `$trunk/bin/pt-online-schema-change h=localhost,S=/tmp/12345/mysql_sandbox12345.sock,D=pt_osc,t=t --user=msandbox --password=msandbox --slave-user=msandbox --slave-password=msandbox --alter "FORCE" --recursion-method=processlist --no-check-replication-filters --no-check-alter --no-check-plan --chunk-index=PRIMARY --no-version-check --execute 2>&1`; + +unlike( + $output, + qr/Use of uninitialized value in concatenation (.) or string/, + 'No error with PTDEBUG output' +) or diag($output); + +# Restore PTDEBUG env. +delete $ENV{PTDEBUG}; +$ENV{PTDEBUG} = $dbg || 0; + +# ############################################################################# +# Done. +# ############################################################################# +$sb->wipe_clean($master_dbh); +ok($sb->ok(), "Sandbox servers") or BAIL_OUT(__FILE__ . " broke the sandbox"); +exit;