Merge lp:~percona-toolkit-dev/percona-toolkit/pt-archiver-skipping-rows-on-partitioned-tables-1376561 into lp:~percona-toolkit-dev/percona-toolkit/release-2.2.12

Proposed by Frank Cizmich
Status: Merged
Approved by: Daniel Nichter
Approved revision: 629
Merged at revision: 630
Proposed branch: lp:~percona-toolkit-dev/percona-toolkit/pt-archiver-skipping-rows-on-partitioned-tables-1376561
Merge into: lp:~percona-toolkit-dev/percona-toolkit/release-2.2.12
Diff against target: 58 lines (+15/-4)
4 files modified
bin/pt-archiver (+11/-0)
t/pt-archiver/basics.t (+1/-1)
t/pt-archiver/indexes.t (+1/-1)
t/pt-archiver/samples/issue-248.txt (+2/-2)
To merge this branch: bzr merge lp:~percona-toolkit-dev/percona-toolkit/pt-archiver-skipping-rows-on-partitioned-tables-1376561
Reviewer Review Type Date Requested Status
Daniel Nichter Approve
Review via email: mp+238033@code.launchpad.net

Description of the change

When ascending a table using an index and the limit clause, returned sets of rows are not technically guaranteed to be disjoint. This problem manifested itself on partitioned tables, causing row skipping.
Fixed by adding an ORDER BY on the selected index columns.

Modified tests to adjust minor output change.

To post a comment you must log in.
Revision history for this message
Daniel Nichter (daniel-nichter) wrote :

One minute detail, otherwise good.

review: Approve
630. By Frank Cizmich

removed trailing space in code and tests

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'bin/pt-archiver'
2--- bin/pt-archiver 2014-09-25 13:48:22 +0000
3+++ bin/pt-archiver 2014-11-05 20:31:52 +0000
4@@ -5811,7 +5811,18 @@
5 $next_sql .= " AND $sel_stmt->{where}";
6 }
7
8+ # Obtain index cols so we can order them when ascending
9+ # this ensures returned sets are disjoint when ran on partitioned tables
10+ # issue 1376561
11+ my $index_cols;
12+ if ( $sel_stmt->{index}
13+ && $src->{info}->{keys}->{$sel_stmt->{index}}->{cols}
14+ ) {
15+ $index_cols = $src->{info}->{keys}->{$sel_stmt->{index}}->{colnames};
16+ }
17+
18 foreach my $thing ( $first_sql, $next_sql ) {
19+ $thing .= " ORDER BY $index_cols" if $index_cols;
20 $thing .= " LIMIT $limit";
21 if ( $o->get('for-update') ) {
22 $thing .= ' FOR UPDATE';
23
24=== modified file 't/pt-archiver/basics.t'
25--- t/pt-archiver/basics.t 2012-11-21 16:21:30 +0000
26+++ t/pt-archiver/basics.t 2014-11-05 20:31:52 +0000
27@@ -161,7 +161,7 @@
28 my $t = time - $t0;
29
30 ok(
31- $t >= 2 && $t <= 3.5,
32+ $t >= 2 && $t <= ($ENV{PERCONA_SLOW_BOX} ? 5 : 3),
33 "--sleep between SELECT (bug 979092)"
34 ) or diag($output, "t=", $t);
35
36
37=== modified file 't/pt-archiver/indexes.t'
38--- t/pt-archiver/indexes.t 2012-06-03 17:54:32 +0000
39+++ t/pt-archiver/indexes.t 2014-11-05 20:31:52 +0000
40@@ -97,7 +97,7 @@
41
42 # Check ascending only first column
43 $output = `$cmd --where 1=1 --dry-run --ascend-first --source D=test,t=table_5,F=$cnf --purge --limit 50 2>&1`;
44-like ( $output, qr/WHERE \(1=1\) AND \(\(`a` >= \?\)\) LIMIT/, 'Can ascend just first column');
45+like ( $output, qr/WHERE \(1=1\) AND \(\(`a` >= \?\)\) ORDER BY `a`,`b`,`c`,`d` LIMIT/, 'Can ascend just first column');
46
47 # #############################################################################
48 # Done.
49
50=== modified file 't/pt-archiver/samples/issue-248.txt'
51--- t/pt-archiver/samples/issue-248.txt 2011-06-24 22:02:05 +0000
52+++ t/pt-archiver/samples/issue-248.txt 2014-11-05 20:31:52 +0000
53@@ -1,3 +1,3 @@
54-SELECT /*!40001 SQL_NO_CACHE */ `film_id`,`title`,`description`,`release_year`,`language_id`,`original_language_id`,`rental_duration`,`rental_rate`,`length`,`replacement_cost`,`rating`,`special_features`,`last_update` FROM `sakila`.`film` FORCE INDEX(`PRIMARY`) WHERE (film_id < 100) AND (`film_id` < '1000') LIMIT 1
55-SELECT /*!40001 SQL_NO_CACHE */ `film_id`,`title`,`description`,`release_year`,`language_id`,`original_language_id`,`rental_duration`,`rental_rate`,`length`,`replacement_cost`,`rating`,`special_features`,`last_update` FROM `sakila`.`film` FORCE INDEX(`PRIMARY`) WHERE (film_id < 100) AND (`film_id` < '1000') AND ((`film_id` >= ?)) LIMIT 1
56+SELECT /*!40001 SQL_NO_CACHE */ `film_id`,`title`,`description`,`release_year`,`language_id`,`original_language_id`,`rental_duration`,`rental_rate`,`length`,`replacement_cost`,`rating`,`special_features`,`last_update` FROM `sakila`.`film` FORCE INDEX(`PRIMARY`) WHERE (film_id < 100) AND (`film_id` < '1000') ORDER BY `film_id` LIMIT 1
57+SELECT /*!40001 SQL_NO_CACHE */ `film_id`,`title`,`description`,`release_year`,`language_id`,`original_language_id`,`rental_duration`,`rental_rate`,`length`,`replacement_cost`,`rating`,`special_features`,`last_update` FROM `sakila`.`film` FORCE INDEX(`PRIMARY`) WHERE (film_id < 100) AND (`film_id` < '1000') AND ((`film_id` >= ?)) ORDER BY `film_id` LIMIT 1
58 DELETE FROM `sakila`.`film` WHERE (`film_id` = ?)

Subscribers

People subscribed via source and target branches

to all changes: