From 6186f942b22e11215b708f5ac269420189449855 Mon Sep 17 00:00:00 2001 From: frank-cizmich Date: Mon, 14 Dec 2015 00:40:16 -0300 Subject: [PATCH] pt-osc fixed recursion method dsn - lp1523685 --- bin/pt-online-schema-change | 36 ++++++++---------- bin/pt-query-digest | 36 ++++++++---------- lib/MasterSlave.pm | 38 ++++++++----------- t/pt-online-schema-change/basics.t | 21 ++++++++++ .../samples/create_dsns.sql | 13 +++++++ 5 files changed, 79 insertions(+), 65 deletions(-) create mode 100644 t/pt-online-schema-change/samples/create_dsns.sql diff --git a/bin/pt-online-schema-change b/bin/pt-online-schema-change index a796074b..ea51ad82 100755 --- a/bin/pt-online-schema-change +++ b/bin/pt-online-schema-change @@ -4057,28 +4057,22 @@ use warnings FATAL => 'all'; use English qw(-no_match_vars); use constant PTDEBUG => $ENV{PTDEBUG} || 0; -sub check_recursion_method { +sub check_recursion_method { my ($methods) = @_; - - foreach my $method ( @$methods ) { - die "Invalid recursion method: " . ($method || 'undef') . "\n" - unless $method && $method =~ m/^(?:processlist$|hosts$|none$|cluster|dsn=)/i; - } - - if ( @$methods > 1 ) { - if ( grep( { m/none/ } @$methods) && grep( {! m/none/ } @$methods) ) { - die "--recursion-method=none cannot be combined with other methods\n"; - } - elsif ( grep({ !m/processlist|hosts/i } @$methods) - && $methods->[0] !~ /^dsn=/i ) - { - die "Invalid combination of recursion methods: " - . join(", ", map { defined($_) ? $_ : 'undef' } @$methods) . ". " - . "Only hosts and processlist may be combined.\n" - } - } - - return; + if ( @$methods != 1 ) { + if ( grep({ !m/processlist|hosts/i } @$methods) + && $methods->[0] !~ /^dsn=/i ) + { + die "Invalid combination of recursion methods: " + . join(", ", map { defined($_) ? $_ : 'undef' } @$methods) . ". " + . "Only hosts and processlist may be combined.\n" + } + } + else { + my ($method) = @$methods; + die "Invalid recursion method: " . ( $method || 'undef' ) + unless $method && $method =~ m/^(?:processlist$|hosts$|none$|cluster$|dsn=)/i; + } } sub new { diff --git a/bin/pt-query-digest b/bin/pt-query-digest index 0e4ad2d1..32420a8d 100755 --- a/bin/pt-query-digest +++ b/bin/pt-query-digest @@ -10356,28 +10356,22 @@ use warnings FATAL => 'all'; use English qw(-no_match_vars); use constant PTDEBUG => $ENV{PTDEBUG} || 0; -sub check_recursion_method { +sub check_recursion_method { my ($methods) = @_; - - foreach my $method ( @$methods ) { - die "Invalid recursion method: " . ($method || 'undef') . "\n" - unless $method && $method =~ m/^(?:processlist$|hosts$|none$|cluster|dsn=)/i; - } - - if ( @$methods > 1 ) { - if ( grep( { m/none/ } @$methods) && grep( {! m/none/ } @$methods) ) { - die "--recursion-method=none cannot be combined with other methods\n"; - } - elsif ( grep({ !m/processlist|hosts/i } @$methods) - && $methods->[0] !~ /^dsn=/i ) - { - die "Invalid combination of recursion methods: " - . join(", ", map { defined($_) ? $_ : 'undef' } @$methods) . ". " - . "Only hosts and processlist may be combined.\n" - } - } - - return; + if ( @$methods != 1 ) { + if ( grep({ !m/processlist|hosts/i } @$methods) + && $methods->[0] !~ /^dsn=/i ) + { + die "Invalid combination of recursion methods: " + . join(", ", map { defined($_) ? $_ : 'undef' } @$methods) . ". " + . "Only hosts and processlist may be combined.\n" + } + } + else { + my ($method) = @$methods; + die "Invalid recursion method: " . ( $method || 'undef' ) + unless $method && $method =~ m/^(?:processlist$|hosts$|none$|cluster$|dsn=)/i; + } } sub new { diff --git a/lib/MasterSlave.pm b/lib/MasterSlave.pm index 9c1083be..6b9eec75 100644 --- a/lib/MasterSlave.pm +++ b/lib/MasterSlave.pm @@ -29,30 +29,22 @@ use constant PTDEBUG => $ENV{PTDEBUG} || 0; # Sub: check_recursion_method # Check that the arrayref of recursion methods passed in is valid -sub check_recursion_method { +sub check_recursion_method { my ($methods) = @_; - - # Check that each method is valid. - foreach my $method ( @$methods ) { - die "Invalid recursion method: " . ($method || 'undef') . "\n" - unless $method && $method =~ m/^(?:processlist$|hosts$|none$|cluster|dsn=)/i; - } - - # Check for invalid combination of methods. - if ( @$methods > 1 ) { - if ( grep( { m/none/ } @$methods) && grep( {! m/none/ } @$methods) ) { - die "--recursion-method=none cannot be combined with other methods\n"; - } - elsif ( grep({ !m/processlist|hosts/i } @$methods) - && $methods->[0] !~ /^dsn=/i ) - { - die "Invalid combination of recursion methods: " - . join(", ", map { defined($_) ? $_ : 'undef' } @$methods) . ". " - . "Only hosts and processlist may be combined.\n" - } - } - - return; + if ( @$methods != 1 ) { + if ( grep({ !m/processlist|hosts/i } @$methods) + && $methods->[0] !~ /^dsn=/i ) + { + die "Invalid combination of recursion methods: " + . join(", ", map { defined($_) ? $_ : 'undef' } @$methods) . ". " + . "Only hosts and processlist may be combined.\n" + } + } + else { + my ($method) = @$methods; + die "Invalid recursion method: " . ( $method || 'undef' ) + unless $method && $method =~ m/^(?:processlist$|hosts$|none$|cluster$|dsn=)/i; + } } sub new { diff --git a/t/pt-online-schema-change/basics.t b/t/pt-online-schema-change/basics.t index 68cfb660..0f315b54 100644 --- a/t/pt-online-schema-change/basics.t +++ b/t/pt-online-schema-change/basics.t @@ -46,6 +46,7 @@ my $exit = 0; my $sample = "t/pt-online-schema-change/samples"; my $rows; + # ############################################################################# # Tool shouldn't run without --execute (bug 933232). # ############################################################################# @@ -787,6 +788,26 @@ test_alter_table( ], ); +# ############################################################################# +# --recursion-method=dns (lp: 1523685) +# ############################################################################# + +$sb->load_file('master', "$sample/create_dsns.sql"); + +($output, $exit) = full_output( + sub { pt_online_schema_change::main(@args, + "$dsn,D=sakila,t=actor", ('--recursion-method=dsn=D=test_recursion_method,t=dsns,h=127.0.0.1,P=12345,u=msandbox,p=msandbox', '--alter-foreign-keys-method', 'drop_swap', '--execute', '--alter', 'ENGINE=InnoDB')) }, + stderr => 1, +); + +like( + $output, + qr/Found 2 slaves.*Successfully altered/si, + "--recursion-method=dns works" +); + +$master_dbh->do("DROP DATABASE test_recursion_method"); + # ############################################################################# # Done. # ############################################################################# diff --git a/t/pt-online-schema-change/samples/create_dsns.sql b/t/pt-online-schema-change/samples/create_dsns.sql new file mode 100644 index 00000000..da857432 --- /dev/null +++ b/t/pt-online-schema-change/samples/create_dsns.sql @@ -0,0 +1,13 @@ +CREATE DATABASE IF NOT EXISTS test_recursion_method; +USE test_recursion_method; +DROP TABLE IF EXISTS `dsns`; +CREATE TABLE `dsns` ( + `id` int(11) NOT NULL AUTO_INCREMENT, + `parent_id` int(11) DEFAULT NULL, + `dsn` varchar(255) NOT NULL, + PRIMARY KEY (`id`) +) ENGINE=InnoDB AUTO_INCREMENT=3 DEFAULT CHARSET=latin1; + +INSERT INTO `dsns` VALUES (1, 12345, "D=test_recursion_method,t=dsns,P=12346,h=127.0.0.1,u=root,p=msandbox"); +INSERT INTO `dsns` VALUES (2, 12345, "D=test_recursion_method,t=dsns,P=12347,h=127.0.0.1,u=root,p=msandbox"); +