From b031df3a7e87c89aa5d06fa0c09211fdd061df21 Mon Sep 17 00:00:00 2001 From: Brian Fraser Date: Fri, 16 Nov 2012 11:21:44 -0300 Subject: [PATCH 1/4] Doc patch emphasizing how --empty-replicate-table works --- bin/pt-table-checksum | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/bin/pt-table-checksum b/bin/pt-table-checksum index c9eee68f..690c9f66 100755 --- a/bin/pt-table-checksum +++ b/bin/pt-table-checksum @@ -11060,7 +11060,7 @@ pathname. default: yes -Delete previous checksums for each table before checksumming the table. This +Delete previous checksums for each table B. This option does not truncate the entire table, it only deletes rows (checksums) for each table just before checksumming the table. Therefore, if checksumming stops prematurely and there was preexisting data, there will still be rows for tables @@ -11069,6 +11069,9 @@ that were not checksummed before the tool was stopped. If you're resuming from a previous checksum run, then the checksum records for the table from which the tool resumes won't be emptied. +If you truly need to empty the replicate table, you should do a +C prior to running the tool. + =item --engines short form: -e; type: hash; group: Filter From 739babd9bf8e8731af8f972db95537a06a8a9f55 Mon Sep 17 00:00:00 2001 From: Brian Fraser Date: Thu, 29 Nov 2012 22:48:13 -0300 Subject: [PATCH 2/4] Remove the B<> and reword the new text --- bin/pt-table-checksum | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/bin/pt-table-checksum b/bin/pt-table-checksum index 690c9f66..6ba8b6da 100755 --- a/bin/pt-table-checksum +++ b/bin/pt-table-checksum @@ -11060,7 +11060,7 @@ pathname. default: yes -Delete previous checksums for each table B. This +Delete previous checksums for each table before checksumming the table. This option does not truncate the entire table, it only deletes rows (checksums) for each table just before checksumming the table. Therefore, if checksumming stops prematurely and there was preexisting data, there will still be rows for tables @@ -11069,8 +11069,8 @@ that were not checksummed before the tool was stopped. If you're resuming from a previous checksum run, then the checksum records for the table from which the tool resumes won't be emptied. -If you truly need to empty the replicate table, you should do a -C prior to running the tool. +To empty the entire replicate table, you must manually execute C +before running the tool. =item --engines From 76a010abeed489237b2659f578ca7a7be046372e Mon Sep 17 00:00:00 2001 From: Brian Fraser Date: Fri, 30 Nov 2012 16:17:45 -0300 Subject: [PATCH 3/4] Fix for 917770: Use of uninitialized value in substitution (s///) at pt-config-diff line 1996 This turned out to be two bugs mangled into one. First, _parse_varvals can deal with (var, undef), but not with (undef). This is a problem because two of the trhee spots that call _parse_varvals can return undef because of this: map { $_ =~ m/^([^=]+)(?:=(.*))?$/ } grep { $_ !~ m/^\s*#/ } # no # comment lines split("\n", $mysqld_section) The problem is twofold. First, we are not skipping empty or whitespace-only lines. That means that the map will fail, and pass an undef to _parse_varvals. So this ended up in a triple fix: Make _parse_varvals deal with a sole undef, skip empty/whitespace lines, and change that map to map { $_ =~ m/^([^=]+)(?:=(.*))?$/ ? ($1, $2) : () } so even if the regex fails in the future, no sole undef will be passed down the chain. --- bin/pt-config-diff | 10 ++++++-- lib/MySQLConfig.pm | 12 ++++++++-- t/lib/MySQLConfig.t | 27 ++++++++++++++++++++- t/pt-config-diff/samples/bug_917770.cnf | 31 +++++++++++++++++++++++++ 4 files changed, 75 insertions(+), 5 deletions(-) create mode 100644 t/pt-config-diff/samples/bug_917770.cnf diff --git a/bin/pt-config-diff b/bin/pt-config-diff index bb96108f..9b200985 100755 --- a/bin/pt-config-diff +++ b/bin/pt-config-diff @@ -2168,7 +2168,9 @@ sub parse_my_print_defaults { my ($output) = @args{@required_args}; my ($config, $dupes) = _parse_varvals( - map { $_ =~ m/^--([^=]+)(?:=(.*))?$/ } split("\n", $output) + map { $_ =~ m/^--([^=]+)(?:=(.*))?$/ ? ($1, $2) : () } + grep { $_ !~ m/^\s*$/ } # no empty lines + split("\n", $output) ); return $config, $dupes; @@ -2186,7 +2188,8 @@ sub parse_option_file { die "Failed to parse the [mysqld] section" unless $mysqld_section; my ($config, $dupes) = _parse_varvals( - map { $_ =~ m/^([^=]+)(?:=(.*))?$/ } + map { $_ =~ m/^([^=]+)(?:=(.*))?$/ ? ($1, $2) : () } + grep { $_ !~ m/^\s*$/ } # no empty lines grep { $_ !~ m/^\s*#/ } # no # comment lines split("\n", $mysqld_section) ); @@ -2206,6 +2209,9 @@ sub _parse_varvals { my $val; # value for current variable ITEM: foreach my $item ( @varvals ) { + if ( !defined($item) ) { + $item = ''; + } if ( $item ) { $item =~ s/^\s+//; $item =~ s/\s+$//; diff --git a/lib/MySQLConfig.pm b/lib/MySQLConfig.pm index 703cada5..46f9af32 100644 --- a/lib/MySQLConfig.pm +++ b/lib/MySQLConfig.pm @@ -288,7 +288,9 @@ sub parse_my_print_defaults { # Parse the "--var=val" lines. my ($config, $dupes) = _parse_varvals( - map { $_ =~ m/^--([^=]+)(?:=(.*))?$/ } split("\n", $output) + map { $_ =~ m/^--([^=]+)(?:=(.*))?$/ ? ($1, $2) : () } + grep { $_ !~ m/^\s*$/ } # no empty lines + split("\n", $output) ); return $config, $dupes; @@ -309,7 +311,8 @@ sub parse_option_file { # Parse the "var=val" lines. my ($config, $dupes) = _parse_varvals( - map { $_ =~ m/^([^=]+)(?:=(.*))?$/ } + map { $_ =~ m/^([^=]+)(?:=(.*))?$/ ? ($1, $2) : () } + grep { $_ !~ m/^\s*$/ } # no empty lines grep { $_ !~ m/^\s*#/ } # no # comment lines split("\n", $mysqld_section) ); @@ -336,6 +339,11 @@ sub _parse_varvals { my $val; # value for current variable ITEM: foreach my $item ( @varvals ) { + # We were passed an undef for the value, or we're dealing with an empty + # line + if ( !defined($item) ) { + $item = ''; + } if ( $item ) { # Strip leading and trailing whitespace. $item =~ s/^\s+//; diff --git a/t/lib/MySQLConfig.t b/t/lib/MySQLConfig.t index 86cde81c..3e9b7b62 100644 --- a/t/lib/MySQLConfig.t +++ b/t/lib/MySQLConfig.t @@ -9,7 +9,7 @@ BEGIN { use strict; use warnings FATAL => 'all'; use English qw(-no_match_vars); -use Test::More tests => 30; +use Test::More; use MySQLConfig; use DSNParser; @@ -827,6 +827,30 @@ SKIP: { ); } +# ############################################################################# +# Use of uninitialized value in substitution (s///) at pt-config-diff line 1996 +# https://bugs.launchpad.net/percona-toolkit/+bug/917770 +# ############################################################################# + +$config = eval { + new MySQLConfig( + file => "$trunk/t/pt-config-diff/samples/bug_917770.cnf", + TextResultSetParser => $trp, + ); +}; + +is( + $EVAL_ERROR, + '', + "Bug 917770: Lives ok on lines with just spaces" +); + +is( + $config->format(), + 'option_file', + "Detect option_file type" +); + # ############################################################################# # Done. # ############################################################################# @@ -841,4 +865,5 @@ like( '_d() works' ); ok($sb->ok(), "Sandbox servers") or BAIL_OUT(__FILE__ . " broke the sandbox"); +done_testing; exit; diff --git a/t/pt-config-diff/samples/bug_917770.cnf b/t/pt-config-diff/samples/bug_917770.cnf new file mode 100644 index 00000000..65a78028 --- /dev/null +++ b/t/pt-config-diff/samples/bug_917770.cnf @@ -0,0 +1,31 @@ +[client] +user = msandbox +password = msandbox +port = PORT +socket = /tmp/PORT/mysql_sandboxPORT.sock + +[mysqld] +port = PORTa +socket = /tmp/PORT/mysql_sandboxPORT.sock +pid-file = /tmp/PORT/data/mysql_sandboxPORT.pid +basedir = PERCONA_TOOLKIT_SANDBOX +datadir = /tmp/PORT/data +key_buffer_size = 16M +innodb_buffer_pool_size = 16M +innodb_data_home_dir = /tmp/PORT/data +innodb_log_group_home_dir = /tmp/PORT/data +innodb_data_file_path = ibdata1:10M:autoextend +innodb_log_file_size = 5M +log-bin = mysql-bin +relay_log = mysql-relay-bin +log_slave_updates +server-id = PORT +report-host = 127.0.0.1 +report-port = PORT +log-error = /tmp/PORT/data/mysqld.log +innodb_lock_wait_timeout = 3 +general_log +general_log_file = genlog +# These next two lines have a space in them, and trigger the bug + + \ No newline at end of file From a7a0da3bc9b0223bb8d97e1b697b2a61c6917890 Mon Sep 17 00:00:00 2001 From: Brian Fraser Date: Tue, 4 Dec 2012 05:02:56 -0300 Subject: [PATCH 4/4] Refactor _parse_varvals. Now it takes two arguments: A regexp and a string to match against. _parse_varvals itself was split in three: _preprocess_varvals, _parse_varvals, and _process_val. This also modifies the three places that call _parse_varvals; For two, no real changes were needed, but parse_mysqld() required a fix to deal with the two final lines of mysqld --help --verbose: To see what values a running MySQL server is using, type 'mysqladmin variables' instead of 'mysqld --verbose --help'. --- bin/pt-config-diff | 136 ++++++++++++++++---------------- lib/MySQLConfig.pm | 190 ++++++++++++++++++++++++--------------------- 2 files changed, 172 insertions(+), 154 deletions(-) diff --git a/bin/pt-config-diff b/bin/pt-config-diff index 9b200985..f9fe1566 100755 --- a/bin/pt-config-diff +++ b/bin/pt-config-diff @@ -2145,15 +2145,16 @@ sub parse_mysqld { PTDEBUG && _d("mysqld help output doesn't list option files"); } - if ( $output !~ m/^-+ -+$/mg ) { + if ( $output !~ m/^-+ -+$(.+?)(?:\n\n.+)?\z/sm ) { PTDEBUG && _d("mysqld help output doesn't list vars and vals"); return; } - my $varvals = substr($output, (pos $output) + 1, length $output); + my $varvals = $1; my ($config, undef) = _parse_varvals( - $varvals =~ m/\G^(\S+)(.*)\n/mg + qr/^(\S+)(.*)$/, + $varvals, ); return $config, \@opt_files; @@ -2168,9 +2169,8 @@ sub parse_my_print_defaults { my ($output) = @args{@required_args}; my ($config, $dupes) = _parse_varvals( - map { $_ =~ m/^--([^=]+)(?:=(.*))?$/ ? ($1, $2) : () } - grep { $_ !~ m/^\s*$/ } # no empty lines - split("\n", $output) + qr/^--([^=]+)(?:=(.*))?$/, + $output, ); return $config, $dupes; @@ -2188,88 +2188,94 @@ sub parse_option_file { die "Failed to parse the [mysqld] section" unless $mysqld_section; my ($config, $dupes) = _parse_varvals( - map { $_ =~ m/^([^=]+)(?:=(.*))?$/ ? ($1, $2) : () } - grep { $_ !~ m/^\s*$/ } # no empty lines - grep { $_ !~ m/^\s*#/ } # no # comment lines - split("\n", $mysqld_section) + qr/^([^=]+)(?:=(.*))?$/, + $mysqld_section, ); return $config, $dupes; } -sub _parse_varvals { - my ( @varvals ) = @_; +sub _preprocess_varvals { + my ($re, $to_parse) = @_; - my %config; + my %vars; + LINE: + foreach my $line ( split /\n/, $to_parse ) { + next LINE if $line =~ m/^\s*$/; # no empty lines + next LINE if $line =~ m/^\s*#/; # no # comment lines - my $duplicate_var = 0; - my %duplicates; - - my $var; # current variable (e.g. datadir) - my $val; # value for current variable - ITEM: - foreach my $item ( @varvals ) { - if ( !defined($item) ) { - $item = ''; + if ( $line !~ $re ) { + PTDEBUG && _d("Line <", $line, "> didn't match $re"); + next LINE; } - if ( $item ) { + + my ($var, $val) = ($1, $2); + + $var =~ tr/-/_/; + + if ( !defined $val ) { + $val = ''; + } + + for my $item ($var, $val) { $item =~ s/^\s+//; $item =~ s/\s+$//; } - if ( !$var ) { - $var = $item; + push @{$vars{$var} ||= []}, $val + } - $var =~ s/-/_/g; + return \%vars; +} - if ( exists $config{$var} && !$can_be_duplicate{$var} ) { - PTDEBUG && _d("Duplicate var:", $var); - $duplicate_var = 1; # flag on, save all the var's values +sub _parse_varvals { + my ( $vars ) = _preprocess_varvals(@_); + + my %config; + + my %duplicates; + + while ( my ($var, $vals) = each %$vars ) { + my $val = _process_val( pop @$vals ); + if ( @$vals && !$can_be_duplicate{$var} ) { + PTDEBUG && _d("Duplicate var:", $var); + foreach my $current_val ( map { _process_val($_) } @$vals ) { + push @{$duplicates{$var} ||= []}, $current_val; } } - else { - my $val = $item; - PTDEBUG && _d("Var:", $var, "val:", $val); - if ( !defined $val ) { - $val = ''; - } - else { - $val =~ s/ - \A # Start of value - (['"]) # Opening quote - (.*) # Value - \1 # Closing quote - [\n\r]*\z # End of value - /$2/x; - if ( my ($num, $factor) = $val =~ m/(\d+)([KMGT])b?$/i ) { - my %factor_for = ( - k => 1_024, - m => 1_048_576, - g => 1_073_741_824, - t => 1_099_511_627_776, - ); - $val = $num * $factor_for{lc $factor}; - } - elsif ( $val =~ m/No default/ ) { - $val = ''; - } - } + PTDEBUG && _d("Var:", $var, "val:", $val); - if ( $duplicate_var ) { - push @{$duplicates{$var}}, $config{$var}; - $duplicate_var = 0; # flag off for next var - } - - $config{$var} = $val; - - $var = undef; # next item should be a var - } + $config{$var} = $val; } return \%config, \%duplicates; } +sub _process_val { + my ($val) = @_; + $val =~ s/ + \A # Start of value + (['"]) # Opening quote + (.*) # Value + \1 # Closing quote + [\n\r]*\z # End of value + /$2/x; + if ( my ($num, $factor) = $val =~ m/(\d+)([KMGT])b?$/i ) { + my %factor_for = ( + k => 1_024, + m => 1_048_576, + g => 1_073_741_824, + t => 1_099_511_627_776, + ); + $val = $num * $factor_for{lc $factor}; + } + elsif ( $val =~ m/No default/ ) { + $val = ''; + } + return $val; +} + sub _mimic_show_variables { my ( %args ) = @_; my @required_args = qw(vars format); diff --git a/lib/MySQLConfig.pm b/lib/MySQLConfig.pm index 46f9af32..ac9cd668 100644 --- a/lib/MySQLConfig.pm +++ b/lib/MySQLConfig.pm @@ -259,18 +259,28 @@ sub parse_mysqld { # help TRUE # abort-slave-event-count 0 # So we search for that line of hypens. - if ( $output !~ m/^-+ -+$/mg ) { + # + # It also ends with something like + # + # wait_timeout 28800 + # + # To see what values a running MySQL server is using, type + # 'mysqladmin variables' instead of 'mysqld --verbose --help'. + # + # So try to find it by locating a double newline, and strip it away + if ( $output !~ m/^-+ -+$(.+?)(?:\n\n.+)?\z/sm ) { PTDEBUG && _d("mysqld help output doesn't list vars and vals"); return; } - # Cut off everything before the list of vars and vals. - my $varvals = substr($output, (pos $output) + 1, length $output); + # Grab the varval list. + my $varvals = $1; # Parse the "var val" lines. 2nd retval is duplicates but there # shouldn't be any with mysqld. my ($config, undef) = _parse_varvals( - $varvals =~ m/\G^(\S+)(.*)\n/mg + qr/^(\S+)(.*)$/, + $varvals, ); return $config, \@opt_files; @@ -288,9 +298,8 @@ sub parse_my_print_defaults { # Parse the "--var=val" lines. my ($config, $dupes) = _parse_varvals( - map { $_ =~ m/^--([^=]+)(?:=(.*))?$/ ? ($1, $2) : () } - grep { $_ !~ m/^\s*$/ } # no empty lines - split("\n", $output) + qr/^--([^=]+)(?:=(.*))?$/, + $output, ); return $config, $dupes; @@ -311,113 +320,116 @@ sub parse_option_file { # Parse the "var=val" lines. my ($config, $dupes) = _parse_varvals( - map { $_ =~ m/^([^=]+)(?:=(.*))?$/ ? ($1, $2) : () } - grep { $_ !~ m/^\s*$/ } # no empty lines - grep { $_ !~ m/^\s*#/ } # no # comment lines - split("\n", $mysqld_section) + qr/^([^=]+)(?:=(.*))?$/, + $mysqld_section, ); return $config, $dupes; } -# Parses a list of variables and their values ("varvals"), returns two +# Called by _parse_varvals(), takes two arguments: a regex, and +# a string to match against. The string will be split in lines, +# and each line will be matched against the regex. +# The regex must return to captures, although the second doesn't +# have to match anything. +# Returns a hashref of arrayrefs ala +# { port => [ 12345, 12346 ], key_buffer_size => [ "16M" ] } +sub _preprocess_varvals { + my ($re, $to_parse) = @_; + + my %vars; + LINE: + foreach my $line ( split /\n/, $to_parse ) { + next LINE if $line =~ m/^\s*$/; # no empty lines + next LINE if $line =~ m/^\s*#/; # no # comment lines + + if ( $line !~ $re ) { + PTDEBUG && _d("Line <", $line, "> didn't match $re"); + next LINE; + } + + my ($var, $val) = ($1, $2); + + # Variable names are usually specified like "log-bin" + # but in SHOW VARIABLES they're all like "log_bin". + $var =~ tr/-/_/; + + if ( !defined $val ) { + $val = ''; + } + + # Strip leading and trailing whitespace. + for my $item ($var, $val) { + $item =~ s/^\s+//; + $item =~ s/\s+$//; + } + + push @{$vars{$var} ||= []}, $val + } + + return \%vars; +} + +# Parses a string of variables and their values ("varvals"), returns two # hashrefs: one with normalized variable=>value, the other with duplicate -# vars. The varvals list should start with a var at index 0 and its value -# at index 1 then repeat for the next var-val pair. +# vars. sub _parse_varvals { - my ( @varvals ) = @_; + my ( $vars ) = _preprocess_varvals(@_); # Config built from parsing the given varvals. my %config; # Discover duplicate vars. - my $duplicate_var = 0; my %duplicates; - # Keep track if item is var or val because each needs special modifications. - my $var; # current variable (e.g. datadir) - my $val; # value for current variable - ITEM: - foreach my $item ( @varvals ) { - # We were passed an undef for the value, or we're dealing with an empty - # line - if ( !defined($item) ) { - $item = ''; - } - if ( $item ) { - # Strip leading and trailing whitespace. - $item =~ s/^\s+//; - $item =~ s/\s+$//; - } - - if ( !$var ) { - # No var means this item is (should be) the next var in the list. - $var = $item; - - # Variable names are usually specified like "log-bin" - # but in SHOW VARIABLES they're all like "log_bin". - $var =~ s/-/_/g; - + while ( my ($var, $vals) = each %$vars ) { + my $val = _process_val( pop @$vals ); + # If the variable has duplicates, then @$vals will contain + # the rest of the values + if ( @$vals && !$can_be_duplicate{$var} ) { # The var is a duplicate (in the bad sense, i.e. where user is # probably unaware that there's two different values for this var - # but only the last is used) if we've seen it already and it cannot - # be duplicated. We don't have its value yet (next loop iter), - # so we set a flag to indicate that we should save the duplicate value. - if ( exists $config{$var} && !$can_be_duplicate{$var} ) { - PTDEBUG && _d("Duplicate var:", $var); - $duplicate_var = 1; # flag on, save all the var's values + # but only the last is used). + PTDEBUG && _d("Duplicate var:", $var); + foreach my $current_val ( map { _process_val($_) } @$vals ) { + push @{$duplicates{$var} ||= []}, $current_val; } } - else { - # $var is set so this item should be its value. - my $val = $item; - PTDEBUG && _d("Var:", $var, "val:", $val); - # Avoid crashing on undef comparison. Also, SHOW VARIABLES uses - # blank strings, not NULL/undef. - if ( !defined $val ) { - $val = ''; - } - else { - $val =~ s/ - \A # Start of value - (['"]) # Opening quote - (.*) # Value - \1 # Closing quote - [\n\r]*\z # End of value - /$2/x; - if ( my ($num, $factor) = $val =~ m/(\d+)([KMGT])b?$/i ) { - # value is a size like 1k, 16M, etc. - my %factor_for = ( - k => 1_024, - m => 1_048_576, - g => 1_073_741_824, - t => 1_099_511_627_776, - ); - $val = $num * $factor_for{lc $factor}; - } - elsif ( $val =~ m/No default/ ) { - $val = ''; - } - } + PTDEBUG && _d("Var:", $var, "val:", $val); - if ( $duplicate_var ) { - # Save the var's last value before we overwrite it with this - # current value. - push @{$duplicates{$var}}, $config{$var}; - $duplicate_var = 0; # flag off for next var - } - - # Save this var-val. - $config{$var} = $val; - - $var = undef; # next item should be a var - } + # Save this var-val. + $config{$var} = $val; } return \%config, \%duplicates; } +sub _process_val { + my ($val) = @_; + $val =~ s/ + \A # Start of value + (['"]) # Opening quote + (.*) # Value + \1 # Closing quote + [\n\r]*\z # End of value + /$2/x; + if ( my ($num, $factor) = $val =~ m/(\d+)([KMGT])b?$/i ) { + # value is a size like 1k, 16M, etc. + my %factor_for = ( + k => 1_024, + m => 1_048_576, + g => 1_073_741_824, + t => 1_099_511_627_776, + ); + $val = $num * $factor_for{lc $factor}; + } + elsif ( $val =~ m/No default/ ) { + $val = ''; + } + return $val; +} + # Sub: _mimic_show_variables # Make the variables' values mimic SHOW VARIABLES. Different output formats # list values differently. To make comparisons easier, outputs are made to