Merge lp:~percona-toolkit-dev/percona-toolkit/validate-load-options-bug-996915-2.0 into lp:percona-toolkit/2.0

[Daniel Nichter]

[Daniel Nichter (daniel-nichter)]

review: Approve

=== modified file 'bin/pt-table-checksum'
--- bin/pt-table-checksum	2012-05-24 17:52:01 +0000
+++ bin/pt-table-checksum	2012-05-25 21:51:20 +0000
@@ -5603,47 +5603,66 @@
 
 sub new {
    my ( $class, %args ) = @_;
-   my @required_args = qw(spec get_status sleep oktorun);
+   my @required_args = qw(max_spec get_status sleep oktorun);
    foreach my $arg ( @required_args ) {
       die "I need a $arg argument" unless defined $args{$arg};
    }
 
-   my $max_val_for = _parse_spec(%args);
+   PTDEBUG && _d('Parsing spec for max thresholds');
+   my $max_val_for = _parse_spec($args{max_spec});
+   if ( $max_val_for ) {
+      _check_and_set_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($args{critical_spec} || []);
+   if ( $critical_val_for ) {
+      _check_and_set_vals(
+         vars             => $critical_val_for,
+         get_status       => $args{get_status},
+         threshold_factor => 1.0, # double (x2; +100%)
+      );
+   }
 
    my $self = {
-      get_status  => $args{get_status},
-      sleep       => $args{sleep},
-      oktorun     => $args{oktorun},
-      max_val_for => $max_val_for,
+      get_status       => $args{get_status},
+      sleep            => $args{sleep},
+      oktorun          => $args{oktorun},
+      max_val_for      => $max_val_for,
+      critical_val_for => $critical_val_for,
    };
 
    return bless $self, $class;
 }
 
 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) = @_;
 
-   if ( !@$spec ) {
-      PTDEBUG && _d('No spec, disabling status var waits');
-      return;
-   }
+   return unless $spec && scalar @$spec;
 
    my %max_val_for;
    foreach my $var_val ( @$spec ) {
+      die "Empty or undefined spec\n" unless $var_val;
+      $var_val =~ s/^\s+//;
+      $var_val =~ s/\s+$//g;
+
       my ($var, $val) = split /[:=]/, $var_val;
-      die "Invalid spec: $var_val" unless $var;
+      die "$var_val does not contain a variable\n" unless $var;
+      die "$var is not a variable name\n" unless $var =~ m/^[a-zA-Z_]+$/;
+
       if ( !$val ) {
-         my $init_val = $get_status->($var);
-         PTDEBUG && _d('Initial', $var, 'value:', $init_val);
-         $val = int(($init_val * .20) + $init_val);
-      }
-      PTDEBUG && _d('Wait if', $var, '>=', $val);
-      $max_val_for{$var} = $val;
+         PTDEBUG && _d('Will get intial value for', $var, 'later');
+         $max_val_for{$var} = undef;
+      }
+      else {
+         die "The value for $var must be a number\n"
+            unless $val =~ m/^[\d\.]+$/;
+         $max_val_for{$var} = $val;
+      }
    }
 
    return \%max_val_for; 
@@ -5654,6 +5673,11 @@
    return $self->{max_val_for};
 }
 
+sub critical_values {
+   my ($self) = @_;
+   return $self->{critical_val_for};
+}
+
 sub wait {
    my ( $self, %args ) = @_;
 
@@ -5664,7 +5688,7 @@
    my $oktorun    = $self->{oktorun};
    my $get_status = $self->{get_status};
    my $sleep      = $self->{sleep};
-   
+
    my %vals_too_high = %{$self->{max_val_for}};
    my $pr_callback;
    if ( $pr ) {
@@ -5688,6 +5712,12 @@
       foreach my $var ( sort keys %vals_too_high ) {
          my $val = $get_status->($var);
          PTDEBUG && _d($var, '=', $val);
+         if ( $val
+              && exists $self->{critical_val_for}->{$var}
+              && $val >= $self->{critical_val_for}->{$var} ) {
+            die "$var=$val exceeds its critical threshold "
+               . "$self->{critical_val_for}->{$var}\n";
+         }
          if ( !$val || $val >= $self->{max_val_for}->{$var} ) {
             $vals_too_high{$var} = $val;
          }
@@ -5712,6 +5742,34 @@
    return;
 }
 
+sub _check_and_set_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('Checking and setting values');
+   return unless $vars && scalar %$vars;
+
+   foreach my $var ( keys %$vars ) {
+      my $init_val = $get_status->($var);
+      die "Variable $var does not exist or its value is undefined\n"
+         unless defined $init_val;
+      my $val;
+      if ( defined $vars->{$var} ) {
+         $val = $vars->{$var};
+      }
+      else {
+         PTDEBUG && _d('Initial', $var, 'value:', $init_val);
+         $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; }
@@ -5874,6 +5932,18 @@
 
    $o->set('chunk-time', 0) if $o->got('chunk-size');
 
+   foreach my $opt ( qw(max-load critical-load) ) {
+      next unless $o->has($opt);
+      my $spec = $o->get($opt);
+      eval {
+         MySQLStatusWaiter::_parse_spec($o->get($opt));
+      };
+      if ( $EVAL_ERROR ) {
+         chomp $EVAL_ERROR;
+         $o->save_error("Invalid --$opt: $EVAL_ERROR");
+      }
+   }
+
    if ( !$o->get('help') ) {
       if ( @ARGV > 1 ) {
          $o->save_error("More than one host specified; only one allowed");
@@ -6233,13 +6303,23 @@
          };
       }
 
-      $sys_load = new MySQLStatusWaiter(
-         spec       => $o->get('max-load'),
-         get_status => $get_status,
-         oktorun    => sub { return $oktorun },
-         sleep      => $sleep,
-      );
-      
+      eval {
+         $sys_load = new MySQLStatusWaiter(
+            max_spec   => $o->get('max-load'),
+            get_status => $get_status,
+            oktorun    => sub { return $oktorun },
+            sleep      => $sleep,
+         );
+      };
+      if ( $EVAL_ERROR ) {
+         chomp $EVAL_ERROR;
+         die "Error checking --max-load: $EVAL_ERROR.  "
+            . "Check that the variables specified for --max-load "
+            . "are spelled correctly and exist in "
+            . "SHOW GLOBAL STATUS.  Current value for this option is:\n"
+            . "  --max-load " . (join(',', @{$o->get('max-load')})) . "\n";
+      }
+
       if ( $o->get('progress') ) {
          $replica_lag_pr = new Progress(
             jobsize => scalar @$slaves,

=== modified file 'lib/MySQLStatusWaiter.pm'
--- lib/MySQLStatusWaiter.pm	2012-01-19 19:46:56 +0000
+++ lib/MySQLStatusWaiter.pm	2012-05-25 21:51:20 +0000
@@ -39,18 +39,37 @@
 #   MySQLStatusWaiter object 
 sub new {
    my ( $class, %args ) = @_;
-   my @required_args = qw(spec get_status sleep oktorun);
+   my @required_args = qw(max_spec get_status sleep oktorun);
    foreach my $arg ( @required_args ) {
       die "I need a $arg argument" unless defined $args{$arg};
    }
 
-   my $max_val_for = _parse_spec(%args);
+   PTDEBUG && _d('Parsing spec for max thresholds');
+   my $max_val_for = _parse_spec($args{max_spec});
+   if ( $max_val_for ) {
+      _check_and_set_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($args{critical_spec} || []);
+   if ( $critical_val_for ) {
+      _check_and_set_vals(
+         vars             => $critical_val_for,
+         get_status       => $args{get_status},
+         threshold_factor => 1.0, # double (x2; +100%)
+      );
+   }
 
    my $self = {
-      get_status  => $args{get_status},
-      sleep       => $args{sleep},
-      oktorun     => $args{oktorun},
-      max_val_for => $max_val_for,
+      get_status       => $args{get_status},
+      sleep            => $args{sleep},
+      oktorun          => $args{oktorun},
+      max_val_for      => $max_val_for,
+      critical_val_for => $critical_val_for,
    };
 
    return bless $self, $class;
@@ -66,29 +85,29 @@
 # 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) = @_;
 
-   if ( !@$spec ) {
-      PTDEBUG && _d('No spec, disabling status var waits');
-      return;
-   }
+   return unless $spec && scalar @$spec;
 
    my %max_val_for;
    foreach my $var_val ( @$spec ) {
+      die "Empty or undefined spec\n" unless $var_val;
+      $var_val =~ s/^\s+//;
+      $var_val =~ s/\s+$//g;
+
       my ($var, $val) = split /[:=]/, $var_val;
-      die "Invalid spec: $var_val" unless $var;
+      die "$var_val does not contain a variable\n" unless $var;
+      die "$var is not a variable name\n" unless $var =~ m/^[a-zA-Z_]+$/;
+
       if ( !$val ) {
-         my $init_val = $get_status->($var);
-         PTDEBUG && _d('Initial', $var, 'value:', $init_val);
-         $val = int(($init_val * .20) + $init_val);
-      }
-      PTDEBUG && _d('Wait if', $var, '>=', $val);
-      $max_val_for{$var} = $val;
+         PTDEBUG && _d('Will get intial value for', $var, 'later');
+         $max_val_for{$var} = undef;
+      }
+      else {
+         die "The value for $var must be a number\n"
+            unless $val =~ m/^[\d\.]+$/;
+         $max_val_for{$var} = $val;
+      }
    }
 
    return \%max_val_for; 
@@ -101,6 +120,11 @@
    return $self->{max_val_for};
 }
 
+sub critical_values {
+   my ($self) = @_;
+   return $self->{critical_val_for};
+}
+
 # Sub: wait
 #   Wait until all variables' values are less than their permitted maximums.
 #
@@ -117,7 +141,7 @@
    my $oktorun    = $self->{oktorun};
    my $get_status = $self->{get_status};
    my $sleep      = $self->{sleep};
-   
+
    my %vals_too_high = %{$self->{max_val_for}};
    my $pr_callback;
    if ( $pr ) {
@@ -144,6 +168,12 @@
       foreach my $var ( sort keys %vals_too_high ) {
          my $val = $get_status->($var);
          PTDEBUG && _d($var, '=', $val);
+         if ( $val
+              && exists $self->{critical_val_for}->{$var}
+              && $val >= $self->{critical_val_for}->{$var} ) {
+            die "$var=$val exceeds its critical threshold "
+               . "$self->{critical_val_for}->{$var}\n";
+         }
          if ( !$val || $val >= $self->{max_val_for}->{$var} ) {
             $vals_too_high{$var} = $val;
          }
@@ -170,6 +200,34 @@
    return;
 }
 
+sub _check_and_set_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('Checking and setting values');
+   return unless $vars && scalar %$vars;
+
+   foreach my $var ( keys %$vars ) {
+      my $init_val = $get_status->($var);
+      die "Variable $var does not exist or its value is undefined\n"
+         unless defined $init_val;
+      my $val;
+      if ( defined $vars->{$var} ) {
+         $val = $vars->{$var};
+      }
+      else {
+         PTDEBUG && _d('Initial', $var, 'value:', $init_val);
+         $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; }

=== modified file 't/lib/MySQLStatusWaiter.t'
--- t/lib/MySQLStatusWaiter.t	2011-10-20 02:07:02 +0000
+++ t/lib/MySQLStatusWaiter.t	2012-05-25 21:51:20 +0000
@@ -9,7 +9,7 @@
 use strict;
 use warnings FATAL => 'all';
 use English qw(-no_match_vars);
-use Test::More tests => 12;
+use Test::More tests => 17;
 
 use MySQLStatusWaiter;
 use PerconaTest;
@@ -39,11 +39,48 @@
    return;
 }
 
+# #############################################################################
+# _parse_spec()
+# #############################################################################
+
+throws_ok(
+   sub { new MySQLStatusWaiter(
+      max_spec => [qw(100)],
+      get_status => \&get_status,
+      sleep      => \&sleep,
+      oktorun    => \&oktorun,
+   ) },
+   qr/100 is not a variable name/,
+   "Catch non-variable name"
+);
+
+throws_ok(
+   sub { new MySQLStatusWaiter(
+      max_spec => [qw(foo=bar)],
+      get_status => \&get_status,
+      sleep      => \&sleep,
+      oktorun    => \&oktorun,
+   ) },
+   qr/value for foo must be a number/,
+   "Catch non-number value"
+);
+
+throws_ok(
+   sub { new MySQLStatusWaiter(
+      max_spec => [qw(foo)],
+      get_status => \&get_status,
+      sleep      => \&sleep,
+      oktorun    => \&oktorun,
+   ) },
+   qr/foo does not exist/,
+   "Catch non-existent variable"
+);
+
 # ############################################################################
 # Use initial vals + 20%.
 # ############################################################################
 @vals = (
-   # initial values
+   # initial check for existence
    { Threads_connected => 10, },
    { Threads_running   => 5,  },
 
@@ -68,11 +105,13 @@
    { Threads_running   => 5,  },
 );
 
+$oktorun = 1;
+
 my $sw = new MySQLStatusWaiter(
    oktorun    => \&oktorun,
    get_status => \&get_status,
    sleep      => \&sleep,
-   spec       => [qw(Threads_connected Threads_running)],
+   max_spec   => [qw(Threads_connected Threads_running)],
 );
 
 is_deeply(
@@ -127,6 +166,10 @@
 # Use static vals.
 # ############################################################################
 @vals = (
+   # initial check for existence
+   { Threads_connected => 1, },
+   { Threads_running   => 1, },
+
    # first check, no wait
    { Threads_connected => 1, },
    { Threads_running   => 1, },
@@ -136,7 +179,7 @@
    oktorun    => \&oktorun,
    get_status => \&get_status,
    sleep      => \&sleep,
-   spec       => [qw(Threads_connected=5 Threads_running=5)],
+   max_spec   => [qw(Threads_connected=5 Threads_running=5)],
 );
 
 is_deeply(
@@ -178,7 +221,7 @@
    oktorun    => \&oktorun,
    get_status => \&get_status,
    sleep      => \&sleep,
-   spec       => [],
+   max_spec   => [],
 );
 
 is(
@@ -204,6 +247,44 @@
    "No spec, no sleep"
 );
 
+# ############################################################################
+# Critical thresholds (with static vals).
+# ############################################################################
+@vals = (
+   # initial check for existence
+   { Threads_running => 1, },
+   { Threads_running => 9, },
+
+   # first check, no wait
+   { Threads_running => 1, },
+   { Threads_running => 9, },
+);
+
+$sw = new MySQLStatusWaiter(
+   oktorun       => \&oktorun,
+   get_status    => \&get_status,
+   sleep         => \&sleep,
+   max_spec      => [qw(Threads_running=4)],
+   critical_spec => [qw(Threads_running=8)],
+
+);
+
+@checked = ();
+$slept   = 0;
+$sw->wait();
+
+is(
+   $slept,
+   0,
+   "Vals not critical, did not sleep"
+);
+
+throws_ok(
+   sub { $sw->wait(); },
+   qr/Threads_running=9 exceeds its critical threshold 8/,
+   "Die on critical threshold"
+);
+
 # #############################################################################
 # Done.
 # #############################################################################

=== modified file 't/pt-table-checksum/option_sanity.t'
--- t/pt-table-checksum/option_sanity.t	2011-12-27 17:24:35 +0000
+++ t/pt-table-checksum/option_sanity.t	2012-05-25 21:51:20 +0000
@@ -9,7 +9,7 @@
 use strict;
 use warnings FATAL => 'all';
 use English qw(-no_match_vars);
-use Test::More tests => 18;
+use Test::More tests => 19;
 
 use PerconaTest;
 shift @INC;  # our unshift (above)
@@ -157,6 +157,17 @@
 );
 
 # #############################################################################
+# --max-load
+# #############################################################################
+
+$output = `$trunk/bin/pt-table-checksum h=127.1,P=12345 --max-load 100`;
+like(
+   $output,
+   qr/Invalid --max-load/,
+   "Validates --max-load"
+);
+
+# #############################################################################
 # Done.
 # #############################################################################
 exit;