diff --git a/bin/pt-online-schema-change b/bin/pt-online-schema-change index 5b55d606..e9ed477e 100755 --- a/bin/pt-online-schema-change +++ b/bin/pt-online-schema-change @@ -4979,6 +4979,16 @@ sub main { # Explicit --chunk-size disable auto chunk sizing. $o->set('chunk-time', 0) if $o->got('chunk-size'); + foreach my $opt ( qw(max-load critical-load) ) { + my $spec = $o->get($opt); + eval { + MySQLStatusWaiter::_parse_spec($o->get($spec)); + }; + if ( $EVAL_ERROR ) { + $o->save_error("Invalid --$opt: $spec"); + } + } + if ( !$o->get('help') ) { if ( @ARGV ) { $o->save_error('Specify only one DSN on the command line'); diff --git a/lib/MySQLStatusWaiter.pm b/lib/MySQLStatusWaiter.pm index b74c2b9e..0e0ec18a 100644 --- a/lib/MySQLStatusWaiter.pm +++ b/lib/MySQLStatusWaiter.pm @@ -45,15 +45,17 @@ sub new { } PTDEBUG && _d('Parsing spec for max thresholds'); - my $max_val_for = _parse_spec( - spec => $args{max_spec}, + my $max_val_for = _parse_spec($args{max_spec}); + _set_initial_vals( + vars => $max_val_for, get_status => $args{get_status}, threshold_factor => 0.2, # +20% ); PTDEBUG && _d('Parsing spec for critical thresholds'); - my $critical_val_for = _parse_spec( - spec => $args{critical_spec} || [], + my $critical_val_for = _parse_spec($args{critical_spec} || []); + _set_initial_vals( + vars => $critical_val_for, get_status => $args{get_status}, threshold_factor => 1.0, # double (x2; +100%) ); @@ -79,27 +81,23 @@ sub new { # Returns: # Hashref with each variable's maximum permitted value. sub _parse_spec { - my ( %args ) = @_; - my @required_args = qw(spec get_status); - foreach my $arg ( @required_args ) { - die "I need a $arg argument" unless defined $args{$arg}; - } - my ($spec, $get_status) = @args{@required_args}; + my ($spec) = @_; return unless $spec && scalar @$spec; - my $threshold_factor = $args{threshold_factor} || 0.20; my %max_val_for; foreach my $var_val ( @$spec ) { my ($var, $val) = split /[:=]/, $var_val; die "Invalid spec: $var_val" unless $var; + if ( !$val ) { - my $init_val = $get_status->($var); - PTDEBUG && _d('Initial', $var, 'value:', $init_val); - $val = int(($init_val * $threshold_factor) + $init_val); + PTDEBUG && _d('Will get intial value for', $var, 'later'); + $max_val_for{$var} = undef; + } + else { + PTDEBUG && _d('Wait if', $var, '>=', $val); + $max_val_for{$var} = $val; } - PTDEBUG && _d('Wait if', $var, '>=', $val); - $max_val_for{$var} = $val; } return \%max_val_for; @@ -192,6 +190,31 @@ sub wait { return; } +sub _set_initial_vals { + my (%args) = @_; + my @required_args = qw(vars get_status threshold_factor); + foreach my $arg ( @required_args ) { + die "I need a $arg argument" unless defined $args{$arg}; + } + my ($vars, $get_status, $threshold_factor) = @args{@required_args}; + + PTDEBUG && _d('Setting initial values'); + return unless $vars && scalar %$vars; + + foreach my $var ( keys %$vars ) { + next if defined $vars->{$var}; + + my $init_val = $get_status->($var); + PTDEBUG && _d('Initial', $var, 'value:', $init_val); + die "Variable does not exist or has undefined value: $var" + unless defined $init_val; + + my $val = int(($init_val * $threshold_factor) + $init_val); + $vars->{$var} = $val; + PTDEBUG && _d('Wait if', $var, '>=', $val); + } +} + sub _d { my ($package, undef, $line) = caller 0; @_ = map { (my $temp = $_) =~ s/\n/\n# /g; $temp; } diff --git a/t/lib/MySQLStatusWaiter.t b/t/lib/MySQLStatusWaiter.t index 3b617818..d7cd22e5 100644 --- a/t/lib/MySQLStatusWaiter.t +++ b/t/lib/MySQLStatusWaiter.t @@ -39,6 +39,21 @@ sub sleep { return; } +# ############################################################################# +# _parse_spec() +# ############################################################################# + +throws_ok( + sub { new MySQLStatusWaiter( + max_spec => '100', + get_status => \&get_status, + sleep => \&sleep, + oktorun => \&oktorun, + ) }, + qr/Invalid spec/, + "Validate max_spec" +); + # ############################################################################ # Use initial vals + 20%. # ############################################################################ diff --git a/t/pt-online-schema-change/option_sanity.t b/t/pt-online-schema-change/option_sanity.t index dbd427b7..99bbe23e 100644 --- a/t/pt-online-schema-change/option_sanity.t +++ b/t/pt-online-schema-change/option_sanity.t @@ -9,7 +9,7 @@ BEGIN { use strict; use warnings FATAL => 'all'; use English qw(-no_match_vars); -use Test::More tests => 6; +use Test::More tests => 8; use PerconaTest; @@ -58,6 +58,20 @@ like( "Cannot --alter-foreign-keys-method=drop_swap with --no-drop-new-table" ); +$output = `$cmd h=127.1,P=12345,u=msandbox,p=msandbox,D=mysql,t=user --max-load 100 --alter "ENGINE=MyISAM" --dry-run`; +like( + $output, + qr/Invalid --max-load/, + "Validates --max-load" +); + +$output = `$cmd h=127.1,P=12345,u=msandbox,p=msandbox,D=mysql,t=user --critical-load 100 --alter "ENGINE=MyISAM" --dry-run`; +like( + $output, + qr/Invalid --critical-load/, + "Validates --critical-load" +); + # ############################################################################# # Done. # #############################################################################