Merge lp:~percona-toolkit-dev/percona-toolkit/fix-923896 into lp:percona-toolkit/2.1

Proposed by Brian Fraser
Status: Merged
Merged at revision: 270
Proposed branch: lp:~percona-toolkit-dev/percona-toolkit/fix-923896
Merge into: lp:percona-toolkit/2.1
Diff against target: 92 lines (+38/-1)
3 files modified
bin/pt-kill (+2/-0)
lib/Processlist.pm (+2/-0)
t/lib/Processlist.t (+34/-1)
To merge this branch: bzr merge lp:~percona-toolkit-dev/percona-toolkit/fix-923896
Reviewer Review Type Date Requested Status
Brian Fraser (community) Approve
Daniel Nichter Approve
Review via email: mp+107643@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Daniel Nichter (daniel-nichter) wrote :

The new tests need to be more descriptive than "ok !$@;".

review: Needs Fixing
270. By Brian Fraser <email address hidden>

Name the tests for 923896's fix

Revision history for this message
Daniel Nichter (daniel-nichter) :
review: Approve
Revision history for this message
Daniel Nichter (daniel-nichter) :
review: Approve
Revision history for this message
Brian Fraser (fraserbn) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'bin/pt-kill'
2--- bin/pt-kill 2012-05-24 17:25:20 +0000
3+++ bin/pt-kill 2012-05-30 14:42:19 +0000
4@@ -2177,6 +2177,7 @@
5 }
6
7 if ( $find_spec{busy_time} && ($query->{Command} || '') eq 'Query' ) {
8+ next QUERY unless defined $query->{Time};
9 if ( $query->{Time} < $find_spec{busy_time} ) {
10 PTDEBUG && _d("Query isn't running long enough");
11 next QUERY;
12@@ -2186,6 +2187,7 @@
13 }
14
15 if ( $find_spec{idle_time} && ($query->{Command} || '') eq 'Sleep' ) {
16+ next QUERY unless defined $query->{Time};
17 if ( $query->{Time} < $find_spec{idle_time} ) {
18 PTDEBUG && _d("Query isn't idle long enough");
19 next QUERY;
20
21=== modified file 'lib/Processlist.pm'
22--- lib/Processlist.pm 2012-01-19 19:46:56 +0000
23+++ lib/Processlist.pm 2012-05-30 14:42:19 +0000
24@@ -470,6 +470,7 @@
25
26 # Match special busy_time.
27 if ( $find_spec{busy_time} && ($query->{Command} || '') eq 'Query' ) {
28+ next QUERY unless defined($query->{Time});
29 if ( $query->{Time} < $find_spec{busy_time} ) {
30 PTDEBUG && _d("Query isn't running long enough");
31 next QUERY;
32@@ -480,6 +481,7 @@
33
34 # Match special idle_time.
35 if ( $find_spec{idle_time} && ($query->{Command} || '') eq 'Sleep' ) {
36+ next QUERY unless defined($query->{Time});
37 if ( $query->{Time} < $find_spec{idle_time} ) {
38 PTDEBUG && _d("Query isn't idle long enough");
39 next QUERY;
40
41=== modified file 't/lib/Processlist.t'
42--- t/lib/Processlist.t 2012-03-06 13:56:08 +0000
43+++ t/lib/Processlist.t 2012-05-30 14:42:19 +0000
44@@ -9,7 +9,7 @@
45 use strict;
46 use warnings FATAL => 'all';
47 use English qw(-no_match_vars);
48-use Test::More tests => 32;
49+use Test::More tests => 34;
50
51 use Processlist;
52 use PerconaTest;
53@@ -828,6 +828,39 @@
54 );
55
56 # #############################################################################
57+# https://bugs.launchpad.net/percona-toolkit/+bug/923896
58+# #############################################################################
59+
60+%find_spec = (
61+ busy_time => 1,
62+ ignore => {},
63+ match => {},
64+);
65+my $proc = { 'Time' => undef,
66+ 'Command' => 'Query',
67+ 'db' => undef,
68+ 'Id' => '7',
69+ 'Info' => undef,
70+ 'User' => 'msandbox',
71+ 'State' => '',
72+ 'Host' => 'localhost'
73+ };
74+
75+local $@;
76+eval { $pl->find([$proc], %find_spec) };
77+ok !$@,
78+ "Bug 923896: NULL Time in processlist doesn't fail for busy_time+Command=Query";
79+
80+delete $find_spec{busy_time};
81+$find_spec{idle_time} = 1;
82+$proc->{Command} = 'Sleep';
83+
84+local $@;
85+eval { $pl->find([$proc], %find_spec) };
86+ok !$@,
87+ "Bug 923896: NULL Time in processlist doesn't fail for idle_time+Command=Sleep";
88+
89+# #############################################################################
90 # Done.
91 # #############################################################################
92 exit;

Subscribers

People subscribed via source and target branches