Merge lp:~percona-toolkit-dev/percona-toolkit/pt-upgrade-fails-to-aggregate-queries-based-on-fingerprint-1421405 into lp:~percona-toolkit-dev/percona-toolkit/release-2.2.14

[Frank Cizmich]

problem:
pt-upgrade reported differences of same class of queries (same fingerprint) repeatedly.

solution:
fixed by correcting the way "reported" attribute was set and also checking for it before reporting.

problem:
erroneous message "Reporting class because there are 1000 row diffs." which always confused the max number of rows in a class (1000) with max number of diffs (default 3)
(Same fix for 'errors' and 'failures')

solution:
fix is fairly obvious from the diffs

notes:
both of these were fixed in UpgradeResults module
test added

[Daniel Nichter (daniel-nichter)]

review: Approve

=== modified file 'bin/pt-upgrade'
--- bin/pt-upgrade	2015-01-23 10:19:56 +0000
+++ bin/pt-upgrade	2015-04-01 20:08:35 +0000
@@ -5858,7 +5858,7 @@
             class   => $class,
             reasons => ["$reason, but hasn't been reported yet"],
          );
-         $class = { reported => 1 };
+         $class->{reported} = 1; 
       };
       if ( $EVAL_ERROR ) {
          $success = 1;
@@ -5872,11 +5872,11 @@
 sub report_if_ready {
    my ($self, %args) = @_;
    my $class = $args{class};
-
-   my $max_examples = $self->max_class_size;
+   my $max_examples   = $self->max_examples;
+   my $max_class_size = $self->max_class_size;
    my @report_reasons;
 
-   if ( scalar keys %{$class->{unique_queries}} >= $self->max_class_size ) {
+   if ( scalar keys %{$class->{unique_queries}} >= $max_class_size ) {
       push @report_reasons, "it's full (--max-class-size)";
    }
 
@@ -5888,15 +5888,15 @@
       push @report_reasons, "there are $max_examples warning diffs";
    }
 
-   if ( scalar @{$class->{row_diffs}} >= $self->max_examples ) {
+   if ( scalar @{$class->{row_diffs}} >= $max_examples ) {
       push @report_reasons, "there are $max_examples row diffs";
    }
 
-   if ( scalar @{$class->{errors}} >= $self->max_examples ) {
+   if ( scalar @{$class->{errors}} >= $max_examples ) {
       push @report_reasons, "there are $max_examples query errors";
    }
 
-   if ( scalar @{$class->{failures}} >= $self->max_examples ) {
+   if ( scalar @{$class->{failures}} >= $max_examples ) {
       push @report_reasons, "there are $max_examples failed queries";
    }
 
@@ -5906,7 +5906,7 @@
          class   => $class,
          reasons => \@report_reasons,
       );
-      $class = { reported => 1 };
+      $class->{reported} = 1; 
    }
 
    return;
@@ -5917,6 +5917,11 @@
    my $class   = $args{class};
    my $reasons = $args{reasons};
 
+   if ( $class->{reported} ) {
+      PTDEBUG && _d('Class already reported');
+      return;
+   }
+
    PTDEBUG && _d('Reporting class', $class->{id}, $class->{fingerprint});
 
    $self->_print_class_header(

=== modified file 'lib/UpgradeResults.pm'
--- lib/UpgradeResults.pm	2013-03-14 15:20:54 +0000
+++ lib/UpgradeResults.pm	2015-04-01 20:08:35 +0000
@@ -220,7 +220,7 @@
             class   => $class,
             reasons => ["$reason, but hasn't been reported yet"],
          );
-         $class = { reported => 1 };
+         $class->{reported} = 1; 
       };
       if ( $EVAL_ERROR ) {
          $success = 1;
@@ -234,11 +234,11 @@
 sub report_if_ready {
    my ($self, %args) = @_;
    my $class = $args{class};
-
-   my $max_examples = $self->max_class_size;
+   my $max_examples   = $self->max_examples;
+   my $max_class_size = $self->max_class_size;
    my @report_reasons;
 
-   if ( scalar keys %{$class->{unique_queries}} >= $self->max_class_size ) {
+   if ( scalar keys %{$class->{unique_queries}} >= $max_class_size ) {
       push @report_reasons, "it's full (--max-class-size)";
    }
 
@@ -250,15 +250,15 @@
       push @report_reasons, "there are $max_examples warning diffs";
    }
 
-   if ( scalar @{$class->{row_diffs}} >= $self->max_examples ) {
+   if ( scalar @{$class->{row_diffs}} >= $max_examples ) {
       push @report_reasons, "there are $max_examples row diffs";
    }
 
-   if ( scalar @{$class->{errors}} >= $self->max_examples ) {
+   if ( scalar @{$class->{errors}} >= $max_examples ) {
       push @report_reasons, "there are $max_examples query errors";
    }
 
-   if ( scalar @{$class->{failures}} >= $self->max_examples ) {
+   if ( scalar @{$class->{failures}} >= $max_examples ) {
       push @report_reasons, "there are $max_examples failed queries";
    }
 
@@ -268,7 +268,7 @@
          class   => $class,
          reasons => \@report_reasons,
       );
-      $class = { reported => 1 };
+      $class->{reported} = 1; 
    }
 
    return;
@@ -279,6 +279,11 @@
    my $class   = $args{class};
    my $reasons = $args{reasons};
 
+   if ( $class->{reported} ) {
+      PTDEBUG && _d('Class already reported');
+      return;
+   }
+
    PTDEBUG && _d('Reporting class', $class->{id}, $class->{fingerprint});
 
    $self->_print_class_header(

=== added directory 't/pt-upgrade/samples/008'
=== added file 't/pt-upgrade/samples/008/conf'
--- t/pt-upgrade/samples/008/conf	1970-01-01 00:00:00 +0000
+++ t/pt-upgrade/samples/008/conf	2015-04-01 20:08:35 +0000
@@ -0,0 +1,2 @@
+type=rawlog
+max-examples=3

=== added file 't/pt-upgrade/samples/008/fingerprint-grouping-and-max-examples-issue-1421405.log'
--- t/pt-upgrade/samples/008/fingerprint-grouping-and-max-examples-issue-1421405.log	1970-01-01 00:00:00 +0000
+++ t/pt-upgrade/samples/008/fingerprint-grouping-and-max-examples-issue-1421405.log	2015-04-01 20:08:35 +0000
@@ -0,0 +1,9 @@
+select @@port
+select @@port
+select @@port
+select @@port
+select @@port
+select @@port
+select @@port
+select @@port
+select @@port

=== added file 't/pt-upgrade/samples/008/fingerprint-grouping-and-max-examples-issue-1421405.txt'
--- t/pt-upgrade/samples/008/fingerprint-grouping-and-max-examples-issue-1421405.txt	1970-01-01 00:00:00 +0000
+++ t/pt-upgrade/samples/008/fingerprint-grouping-and-max-examples-issue-1421405.txt	2015-04-01 20:08:35 +0000
@@ -0,0 +1,75 @@
+
+#-----------------------------------------------------------------------
+# Logs
+#-----------------------------------------------------------------------
+
+File: ...
+Size: 126
+
+#-----------------------------------------------------------------------
+# Hosts
+#-----------------------------------------------------------------------
+
+host1:
+
+  DSN:       h=127.1,P=12345
+  hostname:  ...
+  MySQL:     ...
+
+host2:
+
+  DSN:       h=127.1,P=12348
+  hostname:  ...
+  MySQL:     ...
+
+########################################################################
+# Query class 3530F787A2430DC8
+########################################################################
+
+Reporting class because there are 3 row diffs.
+
+Total queries      3
+Unique queries     3
+Discarded queries  0
+
+select @@port
+
+##
+## Row diffs: 3
+##
+
+-- 1.
+
+@ row 1
+< 12345
+> 12348
+
+select @@port
+
+-- 2.
+
+@ row 1
+< 12345
+> 12348
+
+select @@port
+
+-- 3.
+
+@ row 1
+< 12345
+> 12348
+
+select @@port
+
+#-----------------------------------------------------------------------
+# Stats
+#-----------------------------------------------------------------------
+
+failed_queries        0
+not_select            0
+queries_filtered      0
+queries_no_diffs      0
+queries_read          9
+queries_with_diffs    9
+queries_with_errors   0