From 7b330c481784d3a145b023e76e6099bd2c2d7a0e Mon Sep 17 00:00:00 2001 From: Daniel Nichter Date: Wed, 28 Nov 2012 01:32:31 +0000 Subject: [PATCH] Add and use basic_no_fks_innodb.sql with PXC to avoid MyISAM bugs. Make query_table.pl handle deadlock on START TRANSACTION. Give alter_active_table.t tests unique names. --- .../alter_active_table.t | 92 +++++++------- t/pt-online-schema-change/basics.t | 112 +++++++++++------- t/pt-online-schema-change/privs.t | 7 +- .../samples/basic_no_fks_innodb.sql | 30 +++++ .../samples/basic_with_fks.sql | 20 ++-- .../samples/query_table.pl | 7 +- t/pt-online-schema-change/sanity_checks.t | 9 +- 7 files changed, 164 insertions(+), 113 deletions(-) create mode 100644 t/pt-online-schema-change/samples/basic_no_fks_innodb.sql diff --git a/t/pt-online-schema-change/alter_active_table.t b/t/pt-online-schema-change/alter_active_table.t index 78555b8f..0e2e6342 100644 --- a/t/pt-online-schema-change/alter_active_table.t +++ b/t/pt-online-schema-change/alter_active_table.t @@ -14,6 +14,7 @@ use Test::More; use PerconaTest; use Sandbox; require "$trunk/bin/pt-online-schema-change"; +require VersionParser; use Time::HiRes qw(sleep); use Data::Dumper; @@ -72,23 +73,27 @@ sub get_ids { my @lines = <$fh>; close $fh; - my %ids; + my %ids = ( + updated => '', + deleted => '', + inserted => '', + ); foreach my $line ( @lines ) { my ($stmt, $ids) = split(':', $line); chomp $ids; - $ids{$stmt} = $ids; + $ids{$stmt} = $ids || ''; } return \%ids; } sub check_ids { - my ( $db, $tbl, $pkcol, $ids ) = @_; + my ( $db, $tbl, $pkcol, $ids, $test ) = @_; my $rows; my $n_updated = $ids->{updated} ? ($ids->{updated} =~ tr/,//) : 0; my $n_deleted = $ids->{deleted} ? ($ids->{deleted} =~ tr/,//) : 0; - my $n_inserted = ($ids->{inserted} =~ tr/,//); + my $n_inserted = $ids->{inserted} ?($ids->{inserted} =~ tr/,//) : 0; # "1,1"=~tr/,// returns 1 but is 2 values $n_updated++ if $n_updated; @@ -100,16 +105,16 @@ sub check_ids { is( $rows->[0], 500 + $n_inserted - $n_deleted, - "New table row count: 500 original + $n_inserted inserted - $n_deleted deleted" - ) or print Dumper($rows); + "$test: new table rows: 500 original + $n_inserted inserted - $n_deleted deleted" + ) or diag(Dumper($rows)); $rows = $master_dbh->selectall_arrayref( "SELECT $pkcol FROM $db.$tbl WHERE $pkcol > 500 AND $pkcol NOT IN ($ids->{inserted})"); is_deeply( $rows, [], - "No extra rows inserted in new table" - ) or print Dumper($rows); + "$test: no extra rows inserted in new table" + ) or diag(Dumper($rows)); if ( $n_deleted ) { $rows = $master_dbh->selectall_arrayref( @@ -117,13 +122,13 @@ sub check_ids { is_deeply( $rows, [], - "No deleted rows present in new table" - ) or print Dumper($rows); + "$test: no deleted rows present in new table" + ) or diag(Dumper($rows)); } else { ok( 1, - "No rows deleted" + "$test: no rows deleted" ); }; @@ -134,13 +139,13 @@ sub check_ids { is_deeply( $rows, [], - "Updated rows correct in new table" - ) or print Dumper($rows); + "$test: updated rows correct in new table" + ) or diag(Dumper($rows)); } else { ok( 1, - "No rows updated" + "$test: no rows updated" ); } @@ -150,19 +155,20 @@ sub check_ids { # ############################################################################# # Attempt to alter a table while another process is changing it. # ############################################################################# - -# Load 500 rows. -diag('Loading sample dataset...'); -$sb->load_file('master', "$sample/basic_no_fks.sql"); +sleep 2; +my $db_flavor = VersionParser->new($master_dbh)->flavor(); +if ( $db_flavor =~ m/XtraDB Cluster/ ) { + $sb->load_file('master', "$sample/basic_no_fks_innodb.sql"); +} +else { + $sb->load_file('master', "$sample/basic_no_fks.sql"); +} $master_dbh->do("USE pt_osc"); $master_dbh->do("TRUNCATE TABLE t"); $master_dbh->do("LOAD DATA INFILE '$trunk/t/pt-online-schema-change/samples/basic_no_fks.data' INTO TABLE t"); $master_dbh->do("ANALYZE TABLE t"); $sb->wait_for_slaves(); -$rows = $master_dbh->selectrow_hashref('show master status'); -diag('Binlog position before altering table: ', $rows->{file}, '/', $rows->{position}); - # Start inserting, updating, and deleting rows at random. start_query_table(qw(pt_osc t id)); @@ -178,29 +184,31 @@ start_query_table(qw(pt_osc t id)); # Stop changing the table's data. stop_query_table(); -like($output, qr/Successfully altered `pt_osc`.`t`/, 'Altered OK'); +like( + $output, + qr/Successfully altered `pt_osc`.`t`/, + 'Change engine: altered OK' +); $rows = $master_dbh->selectall_hashref('SHOW TABLE STATUS FROM pt_osc', 'name'); is( $rows->{t}->{engine}, 'InnoDB', - "New table ENGINE=InnoDB" + "Change engine: new table ENGINE=InnoDB" ) or warn Dumper($rows); is( scalar keys %$rows, 1, - "Dropped old table" + "Change engine: dropped old table" ); is( $exit, 0, - "Exit status 0" + "Change engine: exit status 0" ); -check_ids(qw(pt_osc t id), get_ids()); - # ############################################################################# # Check that triggers work when renaming a column # ############################################################################# @@ -211,8 +219,6 @@ $master_dbh->do("LOAD DATA INFILE '$trunk/t/pt-online-schema-change/samples/basi $master_dbh->do("ANALYZE TABLE t"); $sb->wait_for_slaves(); -my $orig_rows = $master_dbh->selectall_arrayref(qq{SELECT id,d FROM pt_osc.t}); - # Start inserting, updating, and deleting rows at random. start_query_table(qw(pt_osc t id)); @@ -230,33 +236,19 @@ start_query_table(qw(pt_osc t id)); # Stop changing the table's data. stop_query_table(); -like($output, qr/Successfully altered `pt_osc`.`t`/, 'Altered OK'); +like( + $output, + qr/Successfully altered `pt_osc`.`t`/, + 'Rename column: altered OK' +); is( $exit, 0, - "Exit status 0" + "Rename columnn: exit status 0" ); -my $ids = get_ids(); -my %deleted_ids = map { $_ => 1 } split /,/, $ids->{deleted}; -my %updated_ids = map { $_ => 1 } split /,/, $ids->{updated}; - -$rows = $master_dbh->selectall_arrayref( - qq{SELECT id,q FROM pt_osc.t WHERE id} - . ($ids->{updated} ? qq{ AND id NOT IN ($ids->{updated})} : '') - . ($ids->{inserted} ? qq{ AND id NOT IN ($ids->{inserted})} : '') -); - -my @filtered_orig_rows = grep { - !$deleted_ids{$_->[0]} && !$updated_ids{$_->[0]} - } @$orig_rows; - -is_deeply( - $rows, - \@filtered_orig_rows, - "Triggers work if renaming a column" -); +check_ids(qw(pt_osc t id), get_ids(), "Rename column"); # ############################################################################# # Done. diff --git a/t/pt-online-schema-change/basics.t b/t/pt-online-schema-change/basics.t index b2826ee7..f5cb1a12 100644 --- a/t/pt-online-schema-change/basics.t +++ b/t/pt-online-schema-change/basics.t @@ -15,6 +15,7 @@ use Time::HiRes qw(sleep); use PerconaTest; use Sandbox; require "$trunk/bin/pt-online-schema-change"; +require VersionParser; use Data::Dumper; $Data::Dumper::Indent = 1; @@ -46,7 +47,7 @@ my $rows; # Tool shouldn't run without --execute (bug 933232). # ############################################################################# -$sb->load_file('master', "$sample/basic_no_fks.sql"); +$sb->load_file('master', "$sample/basic_no_fks_innodb.sql"); ($output, $exit) = full_output( sub { pt_online_schema_change::main(@args, "$dsn,D=pt_osc,t=t", @@ -57,7 +58,7 @@ like( $output, qr/neither --dry-run nor --execute was specified/, "Doesn't run without --execute (bug 933232)" -) or warn $output; +) or diag($output); my $ddl = $master_dbh->selectrow_arrayref("show create table pt_osc.t"); like( @@ -305,36 +306,71 @@ sub test_alter_table { # The most basic: alter a small table with no fks that's not active. # ############################################################################# -test_alter_table( - name => "Basic no fks --dry-run", - table => "pt_osc.t", - file => "basic_no_fks.sql", - max_id => 20, - test_type => "new_engine", - new_engine => "MyISAM", - cmds => [qw(--dry-run --alter ENGINE=InnoDB)], -); +my $db_flavor = VersionParser->new($master_dbh)->flavor(); +if ( $db_flavor =~ m/XtraDB Cluster/ ) { + test_alter_table( + name => "Basic no fks --dry-run", + table => "pt_osc.t", + file => "basic_no_fks_innodb.sql", + max_id => 20, + test_type => "drop_col", + drop_col => "d", + cmds => [qw(--dry-run --alter), 'DROP COLUMN d'], + ); -test_alter_table( - name => "Basic no fks --execute", - table => "pt_osc.t", - # The previous test should not have modified the table. - # file => "basic_no_fks.sql", - # max_id => 20, - test_type => "new_engine", - new_engine => "InnoDB", - cmds => [qw(--execute --alter ENGINE=InnoDB)], -); + test_alter_table( + name => "Basic no fks --execute", + table => "pt_osc.t", + # The previous test should not have modified the table. + # file => "basic_no_fks_innodb.sql", + # max_id => 20, + test_type => "drop_col", + drop_col => "d", + cmds => [qw(--execute --alter), 'DROP COLUMN d'], + ); -test_alter_table( - name => "--execute but no --alter", - table => "pt_osc.t", - file => "basic_no_fks.sql", - max_id => 20, - test_type => "new_engine", - new_engine => "MyISAM", - cmds => [qw(--execute)], -); + test_alter_table( + name => "--execute but no --alter", + table => "pt_osc.t", + file => "basic_no_fks_innodb.sql", + max_id => 20, + test_type => "new_engine", # When there's no change, we just check + new_engine => "InnoDB", # the engine as a NOP. Any other + cmds => [qw(--execute)], # unintended changes are still detected. + ); +} +else { + test_alter_table( + name => "Basic no fks --dry-run", + table => "pt_osc.t", + file => "basic_no_fks.sql", + max_id => 20, + test_type => "new_engine", + new_engine => "MyISAM", + cmds => [qw(--dry-run --alter ENGINE=InnoDB)], + ); + + test_alter_table( + name => "Basic no fks --execute", + table => "pt_osc.t", + # The previous test should not have modified the table. + # file => "basic_no_fks.sql", + # max_id => 20, + test_type => "new_engine", + new_engine => "InnoDB", + cmds => [qw(--execute --alter ENGINE=InnoDB)], + ); + + test_alter_table( + name => "--execute but no --alter", + table => "pt_osc.t", + file => "basic_no_fks.sql", + max_id => 20, + test_type => "new_engine", + new_engine => "MyISAM", + cmds => [qw(--execute)], + ); +} # ############################################################################ # Alter a table with foreign keys. @@ -520,7 +556,7 @@ SKIP: { ); # Restore the original fks. - diag('Restoring original Sakila foreign keys...'); + diag('Restoring sakila...'); diag(`$trunk/sandbox/load-sakila-db 12345`); } @@ -528,23 +564,18 @@ SKIP: { # --alther-foreign-keys-method=none. This intentionally breaks fks because # they're not updated so they'll point to the old table that is dropped. # ############################################################################# -diag('Loading file and waiting for replication...'); -$sb->load_file('master', "$sample/basic_with_fks.sql"); # Specify --alter-foreign-keys-method for a table with no child tables. test_alter_table( name => "Update fk method none", + file => "basic_with_fks.sql", table => "pt_osc.country", pk_col => "country_id", - file => "basic_with_fks.sql", + max_id => 20, test_type => "new_engine", new_engine => "innodb", cmds => [ - qw( - --execute - --alter-foreign-keys-method none - ), - '--alter', 'ENGINE=INNODB', + qw(--execute --alter-foreign-keys-method none --alter ENGINE=INNODB) ], ); @@ -654,7 +685,7 @@ ok( '--execute', '--statistics', '--alter', "modify column val ENUM('M','E','H') NOT NULL") }, - ($sandbox_version ge '5.5' + ($sandbox_version ge '5.5' && $db_flavor !~ m/XtraDB Cluster/ ? "$sample/stats-execute-5.5.txt" : "$sample/stats-execute.txt"), ), @@ -689,7 +720,6 @@ SKIP: { # ############################################################################# # Done. # ############################################################################# -$master_dbh->do("UPDATE mysql.proc SET created='2012-06-05 00:00:00', modified='2012-06-05 00:00:00'"); $sb->wipe_clean($master_dbh); ok($sb->ok(), "Sandbox servers") or BAIL_OUT(__FILE__ . " broke the sandbox"); done_testing; diff --git a/t/pt-online-schema-change/privs.t b/t/pt-online-schema-change/privs.t index 41a7e463..821953a8 100644 --- a/t/pt-online-schema-change/privs.t +++ b/t/pt-online-schema-change/privs.t @@ -38,9 +38,6 @@ elsif ( !$slave2_dbh ) { elsif ( !@{$master_dbh->selectall_arrayref("show databases like 'sakila'")} ) { plan skip_all => 'sakila database is not loaded'; } -else { - plan tests => 3; -} # The sandbox servers run with lock_wait_timeout=3 and it's not dynamic # so we need to specify --lock-wait-timeout=3 else the tool will die. @@ -58,7 +55,7 @@ my $sample = "t/pt-online-schema-change/samples/"; diag(`/tmp/12345/use -u root < $trunk/$sample/osc-user.sql`); PerconaTest::wait_for_table($slave1_dbh, "mysql.tables_priv", "user='osc_user'"); -$sb->load_file('master', "$sample/basic_no_fks.sql"); +$sb->load_file('master', "$sample/basic_no_fks_innodb.sql"); ($output, $exit_status) = full_output( sub { $exit_status = pt_online_schema_change::main(@args, @@ -97,4 +94,4 @@ wait_until( # ############################################################################# $sb->wipe_clean($master_dbh); ok($sb->ok(), "Sandbox servers") or BAIL_OUT(__FILE__ . " broke the sandbox"); -exit; +done_testing; diff --git a/t/pt-online-schema-change/samples/basic_no_fks_innodb.sql b/t/pt-online-schema-change/samples/basic_no_fks_innodb.sql new file mode 100644 index 00000000..b2eaaa6c --- /dev/null +++ b/t/pt-online-schema-change/samples/basic_no_fks_innodb.sql @@ -0,0 +1,30 @@ +DROP DATABASE IF EXISTS pt_osc; +CREATE DATABASE pt_osc; +USE pt_osc; +CREATE TABLE t ( + id int auto_increment primary key, + c char(32), + d date, + unique index (c(32)) +) ENGINE=InnoDB; +INSERT INTO pt_osc.t VALUES + (1, 'a', now()), + (2, 'b', now()), + (3, 'c', now()), + (4, 'd', now()), + (5, 'e', now()), + (6, 'f', now()), + (7, 'g', now()), + (8, 'h', now()), + (9, 'i', now()), + (10, 'j', now()), -- 10 + (11, 'k', now()), + (12, 'l', now()), + (13, 'm', now()), + (14, 'n', now()), + (15, 'o', now()), + (16, 'p', now()), + (17, 'q', now()), + (18, 'r', now()), + (19, 's', now()), + (20, 't', now()); -- 20 diff --git a/t/pt-online-schema-change/samples/basic_with_fks.sql b/t/pt-online-schema-change/samples/basic_with_fks.sql index c98bd6c2..937fa418 100644 --- a/t/pt-online-schema-change/samples/basic_with_fks.sql +++ b/t/pt-online-schema-change/samples/basic_with_fks.sql @@ -40,17 +40,17 @@ INSERT INTO pt_osc.country VALUES (5, 'Spain', null); INSERT INTO pt_osc.city VALUES - (null, 'Montréal', 1, null), - (null, 'New York', 2, null), - (null, 'Durango', 3, null), - (null, 'Paris', 4, null), - (null, 'Madrid', 5, null); + (1, 'Montréal', 1, null), + (2, 'New York', 2, null), + (3, 'Durango', 3, null), + (4, 'Paris', 4, null), + (5, 'Madrid', 5, null); INSERT INTO pt_osc.address VALUES - (null, 'addy 1', 1, '10000', null), - (null, 'addy 2', 2, '20000', null), - (null, 'addy 3', 3, '30000', null), - (null, 'addy 4', 4, '40000', null), - (null, 'addy 5', 5, '50000', null); + (1, 'addy 1', 1, '10000', null), + (2, 'addy 2', 2, '20000', null), + (3, 'addy 3', 3, '30000', null), + (4, 'addy 4', 4, '40000', null), + (5, 'addy 5', 5, '50000', null); SET foreign_key_checks=1; diff --git a/t/pt-online-schema-change/samples/query_table.pl b/t/pt-online-schema-change/samples/query_table.pl index 14d43084..a0d0cbf3 100755 --- a/t/pt-online-schema-change/samples/query_table.pl +++ b/t/pt-online-schema-change/samples/query_table.pl @@ -108,7 +108,12 @@ for my $i ( 1..5_000 ) { commit(); reset_counters(); sleep $sleep; - $dbh->do("START TRANSACTION"); + # TODO: somehow this can fail if called very near when + # the old table is dropped. + eval { $dbh->do("START TRANSACTION"); }; + if ( $EVAL_ERROR ) { + #Test::More::diag($EVAL_ERROR); + } } else { sleep 0.001; diff --git a/t/pt-online-schema-change/sanity_checks.t b/t/pt-online-schema-change/sanity_checks.t index 4d060176..0faa2697 100644 --- a/t/pt-online-schema-change/sanity_checks.t +++ b/t/pt-online-schema-change/sanity_checks.t @@ -28,9 +28,6 @@ my $slave_dbh = $sb->get_dbh_for('slave1'); if ( !$master_dbh ) { plan skip_all => 'Cannot connect to sandbox master'; } -else { - plan tests => 6; -} my $q = new Quoter(); my $tp = new TableParser(Quoter => $q); @@ -66,7 +63,7 @@ like( $output, "Original table must exist" ); -$sb->load_file('master', "$sample/basic_no_fks.sql"); +$sb->load_file('master', "$sample/basic_no_fks_innodb.sql"); $master_dbh->do("USE pt_osc"); $slave_dbh->do("USE pt_osc"); @@ -100,7 +97,7 @@ like( $output, # Checks for the new table. # ############################################################################# -$sb->load_file('master', "$sample/basic_no_fks.sql"); +$sb->load_file('master', "$sample/basic_no_fks_innodb.sql"); $master_dbh->do("USE pt_osc"); $slave_dbh->do("USE pt_osc"); @@ -126,4 +123,4 @@ like( # ############################################################################# $sb->wipe_clean($master_dbh); ok($sb->ok(), "Sandbox servers") or BAIL_OUT(__FILE__ . " broke the sandbox"); -exit; +done_testing;