Percona Toolkit moved to https://jira.percona.com/projects/PT

Merge lp:~percona-toolkit-dev/percona-toolkit/pt-online-schema-change-doesnt-work-with-HASH-indexes-1340728 into lp:~percona-toolkit-dev/percona-toolkit/release-2.2.10

  1. pt-online-schema-change-doesnt-work-with-HASH-indexes-1340728
  2. Merge into release-2.2.10
Proposed by Frank Cizmich
Status: Merged
Approved by: Daniel Nichter
Approved revision: 621
Merged at revision: 613
Proposed branch: lp:~percona-toolkit-dev/percona-toolkit/pt-online-schema-change-doesnt-work-with-HASH-indexes-1340728
Merge into: lp:~percona-toolkit-dev/percona-toolkit/release-2.2.10
Diff against target: 288 lines (+116/-111)
5 files modified
bin/pt-online-schema-change (+40/-33)
t/pt-online-schema-change/bug-1315130.t (+0/-78)
t/pt-online-schema-change/bugs.t (+64/-0)
t/pt-online-schema-change/samples/bug-1340728.sql (+10/-0)
t/pt-online-schema-change/samples/bug-1340728_cleanup.sql (+2/-0)
To merge this branch: bzr merge lp:~percona-toolkit-dev/percona-toolkit/pt-online-schema-change-doesnt-work-with-HASH-indexes-1340728
Reviewer Review Type Date Requested Status
Daniel Nichter Approve
Review via email: mp+228726@code.launchpad.net

Description of the change

Intended to make pt-osc work with engines that allow a HASH index (MEMORY , NDB) . When issued the EXPLAIN query, this type of index sometimes doesn't select a primary key even though it's available.
This fix makes the --nocheck-plan option skip *all* checks on the key chosen by EXPLAIN.

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

We talked on IRC about some changes. I'll set this as "Needs Fixing" until then.

review: Needs Fixing
lp:~percona-toolkit-dev/percona-toolkit/pt-online-schema-change-doesnt-work-with-HASH-indexes-1340728 updated
620. By Frank Cizmich

Skipped another plan check. Skipped running explain when unnecessary. Added test. Fixed test that set check-plan off by default

Revision history for this message
Daniel Nichter (daniel-nichter) wrote :

Code is good, let's just tweak the test a little.

review: Needs Fixing
lp:~percona-toolkit-dev/percona-toolkit/pt-online-schema-change-doesnt-work-with-HASH-indexes-1340728 updated
621. By Frank Cizmich

used output instead of full_output. merged bug tests into bugs.t

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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'bin/pt-online-schema-change'
--- bin/pt-online-schema-change 2014-07-04 17:16:08 +0000
+++ bin/pt-online-schema-change 2014-08-05 15:51:35 +0000
@@ -8908,28 +8908,31 @@
8908 # boundary sql. This check applies to the next nibble. So if8908 # boundary sql. This check applies to the next nibble. So if
8909 # the current nibble number is 5, then nibble 5 is already done8909 # the current nibble number is 5, then nibble 5 is already done
8910 # and we're checking nibble number 6.8910 # and we're checking nibble number 6.
8911 my $expl = explain_statement(8911 # Skip if --nocheck-plan See: https://bugs.launchpad.net/percona-toolkit/+bug/1340728
8912 tbl => $tbl,8912 if ( $o->get('check-plan') ) {
8913 sth => $sth->{explain_upper_boundary},8913 my $expl = explain_statement(
8914 vals => [ @{$boundary->{lower}}, $nibble_iter->limit() ],8914 tbl => $tbl,
8915 );8915 sth => $sth->{explain_upper_boundary},
8916 if (lc($expl->{key} || '') ne lc($nibble_iter->nibble_index() || '')) {8916 vals => [ @{$boundary->{lower}}, $nibble_iter->limit() ],
8917 my $msg8917 );
8918 = "Aborting copying table $tbl->{name} at chunk "8918 if ( lc($expl->{key} || '') ne lc($nibble_iter->nibble_index() || '') ) {
8919 . ($nibble_iter->nibble_number() + 1)8919 my $msg
8920 . " because it is not safe to ascend. Chunking should "8920 = "Aborting copying table $tbl->{name} at chunk "
8921 . "use the "8921 . ($nibble_iter->nibble_number() + 1)
8922 . ($nibble_iter->nibble_index() || '?')8922 . " because it is not safe to ascend. Chunking should "
8923 . " index, but MySQL EXPLAIN reports that "8923 . "use the "
8924 . ($expl->{key} ? "the $expl->{key}" : "no")8924 . ($nibble_iter->nibble_index() || '?')
8925 . " index will be used for "8925 . " index, but MySQL EXPLAIN reports that "
8926 . $sth->{upper_boundary}->{Statement}8926 . ($expl->{key} ? "the $expl->{key}" : "no")
8927 . " with values "8927 . " index will be used for "
8928 . join(", ", map { defined $_ ? $_ : "NULL" }8928 . $sth->{upper_boundary}->{Statement}
8929 (@{$boundary->{lower}}, $nibble_iter->limit()))8929 . " with values "
8930 . "\n";8930 . join(", ", map { defined $_ ? $_ : "NULL" }
8931 die ts($msg);8931 (@{$boundary->{lower}}, $nibble_iter->limit()))
8932 } 8932 . "\n";
8933 die ts($msg);
8934 }
8935 }
89338936
8934 # Once nibbling begins for a table, control does not return to this8937 # Once nibbling begins for a table, control does not return to this
8935 # tool until nibbling is done because, as noted above, all work is8938 # tool until nibbling is done because, as noted above, all work is
@@ -9531,15 +9534,17 @@
9531 vals => [ @{$boundary->{lower}}, @{$boundary->{upper}} ],9534 vals => [ @{$boundary->{lower}}, @{$boundary->{upper}} ],
9532 );9535 );
95339536
9534 # Ensure that MySQL is using the chunk index if the table is being chunked.9537 # Ensure that MySQL is using the chunk index if the table is being chunked.
9535 if ( !$nibble_iter->one_nibble()9538 # Skip if --nocheck-plan See: https://bugs.launchpad.net/percona-toolkit/+bug/1340728
9536 && lc($expl->{key} || '') ne lc($nibble_iter->nibble_index() || '') )9539 if ( !$nibble_iter->one_nibble()
9537 {9540 && lc($expl->{key} || '') ne lc($nibble_iter->nibble_index() || '')
9538 die ts("Error copying rows at chunk " . $nibble_iter->nibble_number()9541 && $o->get('check-plan') )
9539 . " of $tbl->{db}.$tbl->{tbl} because MySQL chose "9542 {
9540 . ($expl->{key} ? "the $expl->{key}" : "no") . " index "9543 die ts("Error copying rows at chunk " . $nibble_iter->nibble_number()
9541 . " instead of the " . $nibble_iter->nibble_index() . "index.\n");9544 . " of $tbl->{db}.$tbl->{tbl} because MySQL chose "
9542 }9545 . ($expl->{key} ? "the $expl->{key}" : "no") . " index "
9546 . " instead of the " . $nibble_iter->nibble_index() . "index.\n");
9547 }
95439548
9544 # Ensure that the chunk isn't too large if there's a --chunk-size-limit.9549 # Ensure that the chunk isn't too large if there's a --chunk-size-limit.
9545 # If single-chunking the table, this has already been checked, so it9550 # If single-chunking the table, this has already been checked, so it
@@ -9564,11 +9569,13 @@
9564 }9569 }
9565 }9570 }
95669571
9567 # Ensure that MySQL is still using the entire index.9572 # Ensure that MySQL is still using the entire index.
9568 # https://bugs.launchpad.net/percona-toolkit/+bug/10102329573 # https://bugs.launchpad.net/percona-toolkit/+bug/1010232
9574 # Skip if --nocheck-plan See: https://bugs.launchpad.net/percona-toolkit/+bug/1340728
9569 if ( !$nibble_iter->one_nibble()9575 if ( !$nibble_iter->one_nibble()
9570 && $tbl->{key_len}9576 && $tbl->{key_len}
9571 && ($expl->{key_len} || 0) < $tbl->{key_len} )9577 && ($expl->{key_len} || 0) < $tbl->{key_len}
9578 && $o->get('check-plan') )
9572 {9579 {
9573 die ts("Error copying rows at chunk " . $nibble_iter->nibble_number()9580 die ts("Error copying rows at chunk " . $nibble_iter->nibble_number()
9574 . " of $tbl->{db}.$tbl->{tbl} because MySQL used "9581 . " of $tbl->{db}.$tbl->{tbl} because MySQL used "
95759582
=== removed file 't/pt-online-schema-change/bug-1315130.t'
--- t/pt-online-schema-change/bug-1315130.t 2014-06-20 21:31:30 +0000
+++ t/pt-online-schema-change/bug-1315130.t 1970-01-01 00:00:00 +0000
@@ -1,78 +0,0 @@
1#!/usr/bin/env perl
2
3BEGIN {
4 die "The PERCONA_TOOLKIT_BRANCH environment variable is not set.\n"
5 unless $ENV{PERCONA_TOOLKIT_BRANCH} && -d $ENV{PERCONA_TOOLKIT_BRANCH};
6 unshift @INC, "$ENV{PERCONA_TOOLKIT_BRANCH}/lib";
7};
8
9use strict;
10use warnings FATAL => 'all';
11use English qw(-no_match_vars);
12use Test::More;
13use Time::HiRes qw(sleep);
14
15$ENV{PTTEST_FAKE_TS} = 1;
16$ENV{PERCONA_TOOLKIT_TEST_USE_DSN_NAMES} = 1;
17
18use PerconaTest;
19use Sandbox;
20require "$trunk/bin/pt-online-schema-change";
21require VersionParser;
22
23use Data::Dumper;
24$Data::Dumper::Indent = 1;
25$Data::Dumper::Sortkeys = 1;
26$Data::Dumper::Quotekeys = 0;
27
28my $dp = new DSNParser(opts=>$dsn_opts);
29my $sb = new Sandbox(basedir => '/tmp', DSNParser => $dp);
30my $master_dbh = $sb->get_dbh_for('master');
31my $slave_dbh = $sb->get_dbh_for('slave1');
32
33if ( !$master_dbh ) {
34 plan skip_all => 'Cannot connect to sandbox master';
35}
36elsif ( !$slave_dbh ) {
37 plan skip_all => 'Cannot connect to sandbox slave';
38}
39
40my $q = new Quoter();
41my $tp = new TableParser(Quoter => $q);
42my @args = qw(--set-vars innodb_lock_wait_timeout=3);
43my $output = "";
44my $dsn = "h=127.1,P=12345,u=msandbox,p=msandbox";
45my $exit = 0;
46my $sample = "t/pt-online-schema-change/samples";
47my $rows;
48
49
50# #############################################################################
51# Issue 1315130
52# Failed to detect child tables in other schema, and falsely identified
53# child tables in own schema
54# #############################################################################
55
56$sb->load_file('master', "$sample/bug-1315130_cleanup.sql");
57$sb->load_file('master', "$sample/bug-1315130.sql");
58($output, $exit) = full_output(
59 sub { pt_online_schema_change::main(@args, "$dsn,D=bug_1315130_a,t=parent_table",
60 '--dry-run',
61 '--alter', "add column c varchar(16)",
62 '--alter-foreign-keys-method', 'auto'),
63 },
64);
65print STDERR "[$output]\n";
66 like(
67 $output,
68 qr/Child tables:\s*`bug_1315130_a`\.`child_table_in_same_schema` \(approx\. 1 rows\)\s*`bug_1315130_b`\.`child_table_in_second_schema` \(approx\. 1 rows\)[^`]*?Will/s,
69 "Correctly identify child tables from other schemas and ignores tables from same schema referencig same named parent in other schema.",
70 );
71# clear databases with their foreign keys
72$sb->load_file('master', "$sample/bug-1315130_cleanup.sql");
73
74# Done.
75# #############################################################################
76$sb->wipe_clean($master_dbh);
77ok($sb->ok(), "Sandbox servers") or BAIL_OUT(__FILE__ . " broke the sandbox");
78done_testing;
790
=== modified file 't/pt-online-schema-change/bugs.t'
--- t/pt-online-schema-change/bugs.t 2013-06-26 20:28:27 +0000
+++ t/pt-online-schema-change/bugs.t 2014-08-05 15:51:35 +0000
@@ -124,7 +124,11 @@
124{124{
125 my $o = new OptionParser(file => "$trunk/bin/pt-table-checksum");125 my $o = new OptionParser(file => "$trunk/bin/pt-table-checksum");
126 $o->get_specs();126 $o->get_specs();
127
128 $o->set('check-plan', 1); # check-plan is true by default
129
127 no warnings;130 no warnings;
131
128 local *pt_online_schema_change::explain_statement = sub {132 local *pt_online_schema_change::explain_statement = sub {
129 return { key => 'some_key' }133 return { key => 'some_key' }
130 };134 };
@@ -369,6 +373,66 @@
369 "Bug 1188264: warning about expected MySQL error 1265"373 "Bug 1188264: warning about expected MySQL error 1265"
370);374);
371375
376
377# #############################################################################
378# Issue 1315130
379# Failed to detect child tables in other schema, and falsely identified
380# child tables in own schema
381# #############################################################################
382
383$sb->load_file('master', "$sample/bug-1315130_cleanup.sql");
384$sb->load_file('master', "$sample/bug-1315130.sql");
385
386$output = output(
387 sub { pt_online_schema_change::main(@args, "$master_dsn,D=bug_1315130_a,t=parent_table",
388 '--dry-run',
389 '--alter', "add column c varchar(16)",
390 '--alter-foreign-keys-method', 'auto'),
391 },
392);
393
394 like(
395 $output,
396 qr/Child tables:\s*`bug_1315130_a`\.`child_table_in_same_schema` \(approx\. 1 rows\)\s*`bug_1315130_b`\.`child_table_in_second_schema` \(approx\. 1 rows\)[^`]*?Will/s,
397 "Correctly identify child tables from other schemas and ignores tables from same schema referencig same named parent in other schema.",
398 );
399# clear databases with their foreign keys
400$sb->load_file('master', "$sample/bug-1315130_cleanup.sql");
401
402
403# #############################################################################
404# Issue 1340728
405# fails when no index is returned in EXPLAIN, even though --nocheck-plan is set
406# (happens on HASH indexes)
407# #############################################################################
408
409$sb->load_file('master', "$sample/bug-1340728_cleanup.sql");
410$sb->load_file('master', "$sample/bug-1340728.sql");
411
412# insert a few thousand rows (else test isn't valid)
413my $rows = 5000;
414for (my $i = 0; $i < $rows; $i++) {
415 $master_dbh->do("INSERT INTO bug_1340728.test VALUES (NULL, 'xx')");
416}
417
418
419$output = output(
420 sub { pt_online_schema_change::main(@args, "$master_dsn,D=bug_1340728,t=test",
421 '--execute',
422 '--alter', "ADD COLUMN c INT",
423 '--nocheck-plan',
424 ),
425 },
426);
427
428 like(
429 $output,
430 qr/Successfully altered/s,
431 "--nocheck-plan ignores plans without index",
432 );
433# clear databases
434$sb->load_file('master', "$sample/bug-1340728_cleanup.sql");
435
372# #############################################################################436# #############################################################################
373# Done.437# Done.
374# #############################################################################438# #############################################################################
375439
=== added file 't/pt-online-schema-change/samples/bug-1340728.sql'
--- t/pt-online-schema-change/samples/bug-1340728.sql 1970-01-01 00:00:00 +0000
+++ t/pt-online-schema-change/samples/bug-1340728.sql 2014-08-05 15:51:35 +0000
@@ -0,0 +1,10 @@
1drop database if exists bug_1340728;
2create database bug_1340728;
3use bug_1340728;
4
5CREATE TABLE `test` (
6 `id` int(10) unsigned NOT NULL AUTO_INCREMENT,
7 `name` char(2) NOT NULL,
8 PRIMARY KEY (`id`) USING HASH
9) ENGINE=MEMORY DEFAULT CHARSET=latin1;
10
011
=== added file 't/pt-online-schema-change/samples/bug-1340728_cleanup.sql'
--- t/pt-online-schema-change/samples/bug-1340728_cleanup.sql 1970-01-01 00:00:00 +0000
+++ t/pt-online-schema-change/samples/bug-1340728_cleanup.sql 2014-08-05 15:51:35 +0000
@@ -0,0 +1,2 @@
1drop database if exists bug_1340728;
2

Subscribers

People subscribed via source and target branches

to all changes: