From 738f9809f8653c2079c9f88a3d373eb01c3c3fd4 Mon Sep 17 00:00:00 2001 From: Carlos Salguero Date: Mon, 4 Sep 2017 12:03:36 -0300 Subject: [PATCH 01/13] PT-167 Still need to add tests Description here --- bin/pt-kill | 30 +++++++++++- bin/pt-query-digest | 14 +++++- lib/Processlist.pm | 17 ++++++- t/pt-kill/pt_167.t | 93 ++++++++++++++++++++++++++++++++++++ t/pt-kill/samples/pt_167.sql | 0 5 files changed, 150 insertions(+), 4 deletions(-) create mode 100644 t/pt-kill/pt_167.t create mode 100644 t/pt-kill/samples/pt_167.sql diff --git a/bin/pt-kill b/bin/pt-kill index 6d1fb47e..67e13b3b 100755 --- a/bin/pt-kill +++ b/bin/pt-kill @@ -3297,6 +3297,17 @@ sub new { foreach my $arg ( qw(MasterSlave) ) { die "I need a $arg argument" unless $args{$arg}; } + my $kill_busy_commands = {}; + if ($args{kill_busy_commands}) { + for my $command (split /,/,$args{kill_busy_commands}) { + $command =~ s/^\s+|\s+$//g; + $kill_busy_commands->{$command} = 1; + } + } else { + $kill_busy_commands->{Query} = 1; + } + $args{kill_busy_commands} = $kill_busy_commands; + my $self = { %args, polls => 0, @@ -3523,7 +3534,7 @@ sub find { next QUERY; } - if ( $find_spec{busy_time} && ($query->{Command} || '') eq 'Query' ) { + if ( $find_spec{busy_time} && exists($self->{kill_busy_commands}->{$query->{Command} || ''}) ) { next QUERY unless defined($query->{Time}); if ( $query->{Time} < $find_spec{busy_time} ) { PTDEBUG && _d("Query isn't running long enough"); @@ -6788,7 +6799,7 @@ sub main { DSNParser => $dp, Quoter => "Quoter", ); - my $pl = new Processlist(MasterSlave => $ms); + my $pl = new Processlist(MasterSlave => $ms, kill_busy_commands => $o->get('kill-busy-commands')); my $qr = new QueryRewriter(); my $cxn; @@ -8198,6 +8209,21 @@ that pt-kill matched and killed a query. See also L<"--wait-before-kill"> and L<"--wait-after-kill">. +=item --kill-busy-commands + +default: Query + +group: Actions + +Comma sepatated list of commands that will be watched/killed if they ran for +more than L<"--busy-time"> seconds. Default: C + +By default, L<"--busy-time"> kills only C commands but in some cases, it +is needed to make L<"--busy-time"> to watch and kill other commands. For example, +a prepared statement execution command is C instead of C. In this +case, specifying C<--kill-busy-commands=Query,Execute> will also kill the prepared +stamente execution. + =item --kill-query group: Actions diff --git a/bin/pt-query-digest b/bin/pt-query-digest index b90f3426..f01767a5 100755 --- a/bin/pt-query-digest +++ b/bin/pt-query-digest @@ -3251,6 +3251,17 @@ sub new { foreach my $arg ( qw(MasterSlave) ) { die "I need a $arg argument" unless $args{$arg}; } + my $kill_busy_commands = {}; + if ($args{kill_busy_commands}) { + for my $command (split /,/,$args{kill_busy_commands}) { + $command =~ s/^\s+|\s+$//g; + $kill_busy_commands->{$command} = 1; + } + } else { + $kill_busy_commands->{Query} = 1; + } + $args{kill_busy_commands} = $kill_busy_commands; + my $self = { %args, polls => 0, @@ -3465,6 +3476,7 @@ sub find { my $ms = $self->{MasterSlave}; my @matches; + $self->{_reasons_for_matching} = undef; QUERY: foreach my $query ( @$proclist ) { PTDEBUG && _d('Checking query', Dumper($query)); @@ -3476,7 +3488,7 @@ sub find { next QUERY; } - if ( $find_spec{busy_time} && ($query->{Command} || '') eq 'Query' ) { + if ( $find_spec{busy_time} && exists($self->{kill_busy_commands}->{$query->{Command} || ''}) ) { next QUERY unless defined($query->{Time}); if ( $query->{Time} < $find_spec{busy_time} ) { PTDEBUG && _d("Query isn't running long enough"); diff --git a/lib/Processlist.pm b/lib/Processlist.pm index 0546b1ce..a953a1c6 100644 --- a/lib/Processlist.pm +++ b/lib/Processlist.pm @@ -69,6 +69,19 @@ sub new { foreach my $arg ( qw(MasterSlave) ) { die "I need a $arg argument" unless $args{$arg}; } + # Convert the list of kill commands (Query, Execute, etc) to a hashref for + # faster check later + my $kill_busy_commands = {}; + if ($args{kill_busy_commands}) { + for my $command (split /,/,$args{kill_busy_commands}) { + $command =~ s/^\s+|\s+$//g; + $kill_busy_commands->{$command} = 1; + } + } else { + $kill_busy_commands->{Query} = 1; + } + $args{kill_busy_commands} = $kill_busy_commands; + my $self = { %args, polls => 0, @@ -471,6 +484,7 @@ sub find { my $ms = $self->{MasterSlave}; my @matches; + $self->{_reasons_for_matching} = undef; QUERY: foreach my $query ( @$proclist ) { PTDEBUG && _d('Checking query', Dumper($query)); @@ -484,7 +498,8 @@ sub find { } # Match special busy_time. - if ( $find_spec{busy_time} && ($query->{Command} || '') eq 'Query' ) { + #if ( $find_spec{busy_time} && ($query->{Command} || '') eq 'Query' ) { + if ( $find_spec{busy_time} && exists($self->{kill_busy_commands}->{$query->{Command} || ''}) ) { next QUERY unless defined($query->{Time}); if ( $query->{Time} < $find_spec{busy_time} ) { PTDEBUG && _d("Query isn't running long enough"); diff --git a/t/pt-kill/pt_167.t b/t/pt-kill/pt_167.t new file mode 100644 index 00000000..494c7283 --- /dev/null +++ b/t/pt-kill/pt_167.t @@ -0,0 +1,93 @@ +#!/usr/bin/env perl + +BEGIN { + die "The PERCONA_TOOLKIT_BRANCH environment variable is not set.\n" + unless $ENV{PERCONA_TOOLKIT_BRANCH} && -d $ENV{PERCONA_TOOLKIT_BRANCH}; + unshift @INC, "$ENV{PERCONA_TOOLKIT_BRANCH}/lib"; +}; + +use strict; +use warnings FATAL => 'all'; +use English qw(-no_match_vars); +use Time::HiRes qw(sleep); +use Test::More; + +use PerconaTest; +use Sandbox; +require "$trunk/bin/pt-kill"; + +use Data::Dumper; +$Data::Dumper::Indent = 1; +$Data::Dumper::Sortkeys = 1; +$Data::Dumper::Quotekeys = 0; + +my $dp = new DSNParser(opts=>$dsn_opts); +my $sb = new Sandbox(basedir => '/tmp', DSNParser => $dp); +my $dbh = $sb->get_dbh_for('master'); + +if ( !$dbh ) { + plan skip_all => 'Cannot connect to sandbox master'; +} + +my $output; +my $dsn = $sb->dsn_for('master'); +my $cnf = '/tmp/12345/my.sandbox.cnf'; + +# ############################################################################# +# Test that --kill-query only kills the query, not the connection. +# ############################################################################# + +# Here's how this works. This cmd is going to try 2 queries on the same +# connection: sleep5 and sleep3. --kill-query will kill sleep5 causing +# sleep3 to start using the same connection id (pid). +system("/tmp/12345/use -e 'select sleep(5); select sleep(2)' >/dev/null&"); +sleep 0.5; + +SKIP: { + + skip "TODO"; + my $iterations=99; + my $query = 'select benchmark(?, md5("when will it end?"))'; + my $ps = $dbh->prepare($query); + $ps->execute($iterations); + my $rows = $dbh->selectall_hashref('show processlist', 'id'); + my $pid = 0; # reuse, reset + map { $pid = $_->{id} } + grep { $_->{info} && $_->{info} =~ m/select sleep\(15\)/ } + values %$rows; + + ok( + $pid, + 'Got proc id of sleeping query' + ); + + $output = output( + sub { pt_kill::main('-F', $cnf, qw(--busy-time 4s --print --match-info "^(select|SELECT)")), }, + ); + like( + $output, + qr/KILL QUERY $pid /, + '--kill-query' + ); + + sleep 1; + $rows = $dbh->selectall_hashref('show processlist', 'id'); + my $con_alive = grep { $_->{id} eq $pid } values %$rows; + ok( + $con_alive, + 'Killed query, not connection' + ); + + is( + ($rows->{$pid}->{info} || ''), + 'select sleep(3)', + 'Connection is still alive' + ); +} + +# ############################################################################# +# Done. +# ############################################################################# +$sb->wipe_clean($dbh); +ok($sb->ok(), "Sandbox servers") or BAIL_OUT(__FILE__ . " broke the sandbox"); +done_testing; diff --git a/t/pt-kill/samples/pt_167.sql b/t/pt-kill/samples/pt_167.sql new file mode 100644 index 00000000..e69de29b From fec1a1f75883a20a96301fd4a2c9d7bb469447d7 Mon Sep 17 00:00:00 2001 From: Carlos Salguero Date: Wed, 6 Sep 2017 16:46:47 -0300 Subject: [PATCH 02/13] PT-167 pt-kill kills prepared statements w/o checking busy-time --- bin/pt-kill | 2 +- t/pt-kill/pt_167.t | 113 ++++++++++++++++++----------------- t/pt-kill/samples/pt_167.sql | 4 ++ 3 files changed, 64 insertions(+), 55 deletions(-) diff --git a/bin/pt-kill b/bin/pt-kill index 67e13b3b..11663760 100755 --- a/bin/pt-kill +++ b/bin/pt-kill @@ -47,7 +47,7 @@ BEGIN { { package Percona::Toolkit; -our $VERSION = '3.0.4'; +our $VERSION = '3.0.5'; use strict; use warnings FATAL => 'all'; diff --git a/t/pt-kill/pt_167.t b/t/pt-kill/pt_167.t index 494c7283..a133cd25 100644 --- a/t/pt-kill/pt_167.t +++ b/t/pt-kill/pt_167.t @@ -11,6 +11,7 @@ use warnings FATAL => 'all'; use English qw(-no_match_vars); use Time::HiRes qw(sleep); use Test::More; +use threads; use PerconaTest; use Sandbox; @@ -21,6 +22,13 @@ $Data::Dumper::Indent = 1; $Data::Dumper::Sortkeys = 1; $Data::Dumper::Quotekeys = 0; + +my $o = new OptionParser(description => 'Diskstats', + file => "$trunk/bin/pt-kill", +); +$o->get_specs("$trunk/bin/pt-table-checksum"); +$o->get_opts(); + my $dp = new DSNParser(opts=>$dsn_opts); my $sb = new Sandbox(basedir => '/tmp', DSNParser => $dp); my $dbh = $sb->get_dbh_for('master'); @@ -29,62 +37,59 @@ if ( !$dbh ) { plan skip_all => 'Cannot connect to sandbox master'; } -my $output; -my $dsn = $sb->dsn_for('master'); -my $cnf = '/tmp/12345/my.sandbox.cnf'; - -# ############################################################################# -# Test that --kill-query only kills the query, not the connection. -# ############################################################################# - -# Here's how this works. This cmd is going to try 2 queries on the same -# connection: sleep5 and sleep3. --kill-query will kill sleep5 causing -# sleep3 to start using the same connection id (pid). -system("/tmp/12345/use -e 'select sleep(5); select sleep(2)' >/dev/null&"); -sleep 0.5; - -SKIP: { - - skip "TODO"; - my $iterations=99; - my $query = 'select benchmark(?, md5("when will it end?"))'; - my $ps = $dbh->prepare($query); - $ps->execute($iterations); - my $rows = $dbh->selectall_hashref('show processlist', 'id'); - my $pid = 0; # reuse, reset - map { $pid = $_->{id} } - grep { $_->{info} && $_->{info} =~ m/select sleep\(15\)/ } - values %$rows; - - ok( - $pid, - 'Got proc id of sleeping query' - ); - - $output = output( - sub { pt_kill::main('-F', $cnf, qw(--busy-time 4s --print --match-info "^(select|SELECT)")), }, - ); - like( - $output, - qr/KILL QUERY $pid /, - '--kill-query' - ); - - sleep 1; - $rows = $dbh->selectall_hashref('show processlist', 'id'); - my $con_alive = grep { $_->{id} eq $pid } values %$rows; - ok( - $con_alive, - 'Killed query, not connection' - ); - - is( - ($rows->{$pid}->{info} || ''), - 'select sleep(3)', - 'Connection is still alive' - ); +$sb->load_file('master', "t/pt-kill/samples/pt_167.sql"); +my $ps = $dbh->prepare('INSERT INTO test.foo VALUES (?)'); +for (my $i=0; $i < 20000; $i++) { + $ps->execute($i); } +sub start_thread { + my ($dp, $o, $dsn, $sleep_time) = @_; + + diag("Thread started"); + + my $cxn = new Cxn(DSNParser => $dp, OptionParser => $o, dsn => $dsn); + $cxn->connect(); + my $dbh = $cxn->dbh(); + my $sth = $dbh->prepare('SELECT COUNT(*) FROM (SELECT a.id AS a_id, b.id AS b_id FROM foo AS a, foo AS b) AS c'); + # Since this query is going to be killed, wrap the execution in an eval to prevent + # displaying the error message. + eval { + $sth->execute(); + }; +} + +my $dsn = "h=127.1,P=12345,u=msandbox,p=msandbox,D=test;mysql_server_prepare=1"; +my $thr = threads->create('start_thread', $dp, $o, $dp->parse($dsn), 1); +$thr->detach(); +threads->yield(); + +sleep(1); + +my $rows = $dbh->selectall_hashref('show processlist', 'id'); +my $pid = 0; # reuse, reset +map { $pid = $_->{id} } +grep { $_->{info} && $_->{info} =~ m/SELECT COUNT\(\*\) FROM \(SELECT a.id/ } +values %$rows; + +ok( + $pid, + "Got proc id of sleeping query: $pid" +); + +$dsn = $sb->dsn_for('master'); +my $output = output( + sub { pt_kill::main($dsn, "--kill-busy-commands","Query,Execute", qw(--run-time 3s --kill --busy-time 2s --print --match-info), "^(select|SELECT)"), }, + stderr => 1, +); + +like( + $output, + qr/KILL $pid \(Execute/, + '--kill-query' +) or diag($output); + + # ############################################################################# # Done. # ############################################################################# diff --git a/t/pt-kill/samples/pt_167.sql b/t/pt-kill/samples/pt_167.sql index e69de29b..0b15466e 100644 --- a/t/pt-kill/samples/pt_167.sql +++ b/t/pt-kill/samples/pt_167.sql @@ -0,0 +1,4 @@ +DROP DATABASE IF EXISTS test; +CREATE DATABASE test; + +create table test.foo (id integer); From 3928b1e47f9d2cea502f4884379b3ba664e92d7e Mon Sep 17 00:00:00 2001 From: MATSUU Takuto Date: Wed, 13 Sep 2017 18:01:47 +0900 Subject: [PATCH 03/13] fix a formula --- src/go/mongolib/stats/stats.go | 2 +- src/go/tests/profiler_docs_stats.want.json | 6 +++--- src/go/tests/profiler_docs_total_stats.want.json | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/go/mongolib/stats/stats.go b/src/go/mongolib/stats/stats.go index 9123d79e..2f84147e 100644 --- a/src/go/mongolib/stats/stats.go +++ b/src/go/mongolib/stats/stats.go @@ -281,7 +281,7 @@ func countersToStats(query QueryInfoAndCounters, uptime int64, tc totalCounters) queryStats.QueryTime.Pct = queryStats.QueryTime.Total * 100 / tc.QueryTime } if tc.Bytes > 0 { - queryStats.ResponseLength.Pct = queryStats.ResponseLength.Total / tc.Bytes + queryStats.ResponseLength.Pct = queryStats.ResponseLength.Total * 100 / tc.Bytes } if queryStats.Returned.Total > 0 { queryStats.Ratio = queryStats.Scanned.Total / queryStats.Returned.Total diff --git a/src/go/tests/profiler_docs_stats.want.json b/src/go/tests/profiler_docs_stats.want.json index fc203be0..3c0e01df 100644 --- a/src/go/tests/profiler_docs_stats.want.json +++ b/src/go/tests/profiler_docs_stats.want.json @@ -22,7 +22,7 @@ "Median": 0 }, "ResponseLength": { - "Pct": 0.9995949739087844, + "Pct": 99.95949739087844, "Total": 1061230, "Min": 1061230, "Max": 1061230, @@ -75,7 +75,7 @@ "Median": 7 }, "ResponseLength": { - "Pct": 0.00020251304560782172, + "Pct": 0.020251304560782172, "Total": 215, "Min": 215, "Max": 215, @@ -128,7 +128,7 @@ "Median": 0 }, "ResponseLength": { - "Pct": 0.00020251304560782172, + "Pct": 0.020251304560782172, "Total": 215, "Min": 215, "Max": 215, diff --git a/src/go/tests/profiler_docs_total_stats.want.json b/src/go/tests/profiler_docs_total_stats.want.json index 39bc4c5c..7bcdfeb0 100755 --- a/src/go/tests/profiler_docs_total_stats.want.json +++ b/src/go/tests/profiler_docs_total_stats.want.json @@ -21,7 +21,7 @@ "Median": 0 }, "ResponseLength": { - "Pct": 1, + "Pct": 100, "Total": 1061660, "Min": 215, "Max": 1061230, From 08cfe9bd94387f6d8cd4c0e373f092ad0995adfa Mon Sep 17 00:00:00 2001 From: MATSUU Takuto Date: Wed, 13 Sep 2017 18:05:28 +0900 Subject: [PATCH 04/13] set Rank parameter --- src/go/pt-mongodb-query-digest/main.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/go/pt-mongodb-query-digest/main.go b/src/go/pt-mongodb-query-digest/main.go index ae0d174e..dc87f8cd 100644 --- a/src/go/pt-mongodb-query-digest/main.go +++ b/src/go/pt-mongodb-query-digest/main.go @@ -165,7 +165,8 @@ func main() { if opts.Limit > 0 && len(sortedQueryStats) > opts.Limit { sortedQueryStats = sortedQueryStats[:opts.Limit] } - for _, qs := range sortedQueryStats { + for i, qs := range sortedQueryStats { + qs.Rank = i + 1 t.Execute(os.Stdout, qs) } From 31d130cbdec2a41b9d9dea41f7ff178a16470957 Mon Sep 17 00:00:00 2001 From: MATSUU Takuto Date: Wed, 13 Sep 2017 18:10:12 +0900 Subject: [PATCH 05/13] set Count parameter for totalStats --- src/go/mongolib/stats/stats.go | 1 + src/go/tests/profiler_docs_total_stats.want.json | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/go/mongolib/stats/stats.go b/src/go/mongolib/stats/stats.go index 2f84147e..0534b029 100644 --- a/src/go/mongolib/stats/stats.go +++ b/src/go/mongolib/stats/stats.go @@ -293,6 +293,7 @@ func countersToStats(query QueryInfoAndCounters, uptime int64, tc totalCounters) func aggregateCounters(queries []QueryInfoAndCounters) QueryInfoAndCounters { qt := QueryInfoAndCounters{} for _, query := range queries { + qt.Count += query.Count qt.NScanned = append(qt.NScanned, query.NScanned...) qt.NReturned = append(qt.NReturned, query.NReturned...) qt.QueryTime = append(qt.QueryTime, query.QueryTime...) diff --git a/src/go/tests/profiler_docs_total_stats.want.json b/src/go/tests/profiler_docs_total_stats.want.json index 7bcdfeb0..3a06993d 100755 --- a/src/go/tests/profiler_docs_total_stats.want.json +++ b/src/go/tests/profiler_docs_total_stats.want.json @@ -6,7 +6,7 @@ "Fingerprint": "", "FirstSeen": "0001-01-01T00:00:00Z", "LastSeen": "0001-01-01T00:00:00Z", - "Count": 0, + "Count": 3, "QPS": 0, "Rank": 0, "Ratio": 134.33333333333334, From aec4ac03cbbcc49d9b8746e9903259979cfbb2b3 Mon Sep 17 00:00:00 2001 From: MATSUU Takuto Date: Wed, 13 Sep 2017 18:33:19 +0900 Subject: [PATCH 06/13] sort Ratio like Count Because Scanned.Max / Returned.Max is not meaningful data. --- src/go/pt-mongodb-query-digest/main.go | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/go/pt-mongodb-query-digest/main.go b/src/go/pt-mongodb-query-digest/main.go index dc87f8cd..2acd0cb0 100644 --- a/src/go/pt-mongodb-query-digest/main.go +++ b/src/go/pt-mongodb-query-digest/main.go @@ -427,15 +427,11 @@ func sortQueries(queries []stats.QueryStats, orderby []string) []stats.QueryStat case "ratio": f = func(c1, c2 *stats.QueryStats) bool { - ratio1 := c1.Scanned.Max / c1.Returned.Max - ratio2 := c2.Scanned.Max / c2.Returned.Max - return ratio1 < ratio2 + return c1.Ratio < c2.Ratio } case "-ratio": f = func(c1, c2 *stats.QueryStats) bool { - ratio1 := c1.Scanned.Max / c1.Returned.Max - ratio2 := c2.Scanned.Max / c2.Returned.Max - return ratio1 > ratio2 + return c1.Ratio > c2.Ratio } // From 8ce461a7c1931558f44f574694ad56ad257141c6 Mon Sep 17 00:00:00 2001 From: MATSUU Takuto Date: Wed, 13 Sep 2017 18:36:07 +0900 Subject: [PATCH 07/13] support versions less than 3.2.0 docsExamined is renamed from nscannedObjects in 3.2.0. --- src/go/mongolib/proto/system.profile.go | 2 ++ src/go/mongolib/stats/stats.go | 7 ++++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/go/mongolib/proto/system.profile.go b/src/go/mongolib/proto/system.profile.go index 3bb4494d..fb45b1f0 100644 --- a/src/go/mongolib/proto/system.profile.go +++ b/src/go/mongolib/proto/system.profile.go @@ -2,11 +2,13 @@ package proto import "time" +// docsExamined is renamed from nscannedObjects in 3.2.0 type SystemProfile struct { AllUsers []interface{} `bson:"allUsers"` Client string `bson:"client"` CursorExhausted bool `bson:"cursorExhausted"` DocsExamined int `bson:"docsExamined"` + NscannedObjects int `bson:"nscannedObjects"` ExecStats struct { Advanced int `bson:"advanced"` ExecutionTimeMillisEstimate int `bson:"executionTimeMillisEstimate"` diff --git a/src/go/mongolib/stats/stats.go b/src/go/mongolib/stats/stats.go index 0534b029..408377b2 100644 --- a/src/go/mongolib/stats/stats.go +++ b/src/go/mongolib/stats/stats.go @@ -91,7 +91,12 @@ func (s *Stats) Add(doc proto.SystemProfile) error { s.setQueryInfoAndCounters(key, qiac) } qiac.Count++ - qiac.NScanned = append(qiac.NScanned, float64(doc.DocsExamined)) + // docsExamined is renamed from nscannedObjects in 3.2.0. + if doc.NscannedObjects > 0 { + qiac.NScanned = append(qiac.NScanned, float64(doc.NscannedObjects)) + } else { + qiac.NScanned = append(qiac.NScanned, float64(doc.DocsExamined)) + } qiac.NReturned = append(qiac.NReturned, float64(doc.Nreturned)) qiac.QueryTime = append(qiac.QueryTime, float64(doc.Millis)) qiac.ResponseLength = append(qiac.ResponseLength, float64(doc.ResponseLength)) From c18fdc8a01867422a9d63a440cfc0880d8dd2c0f Mon Sep 17 00:00:00 2001 From: MATSUU Takuto Date: Wed, 13 Sep 2017 23:05:51 +0900 Subject: [PATCH 08/13] fix test --- src/go/tests/profiler_docs_total_stats.want.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/go/tests/profiler_docs_total_stats.want.json b/src/go/tests/profiler_docs_total_stats.want.json index 3a06993d..6b074890 100755 --- a/src/go/tests/profiler_docs_total_stats.want.json +++ b/src/go/tests/profiler_docs_total_stats.want.json @@ -7,7 +7,7 @@ "FirstSeen": "0001-01-01T00:00:00Z", "LastSeen": "0001-01-01T00:00:00Z", "Count": 3, - "QPS": 0, + "QPS": 3, "Rank": 0, "Ratio": 134.33333333333334, "QueryTime": { From 794999154c22e9a4ebdfd77e7f111c701f1cf358 Mon Sep 17 00:00:00 2001 From: Carlos Salguero Date: Tue, 19 Sep 2017 14:58:26 -0300 Subject: [PATCH 09/13] PT-187 Updated documentation for pt-table-checksum Added info & links regarding the use of CRC32 as the checksum algorithm. --- bin/pt-table-checksum | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/bin/pt-table-checksum b/bin/pt-table-checksum index c9dfbb08..02a32f42 100755 --- a/bin/pt-table-checksum +++ b/bin/pt-table-checksum @@ -11845,6 +11845,15 @@ can try something like the following: SET boundaries = COALESCE(CONCAT('id BETWEEN ', lower_boundary, ' AND ', upper_boundary), '1=1'); +Take into consideration that by default, pt-table-checksum use C checksums. +C is not a cryptographic algorithm and for that reason it is prone to have +collisions. On the other hand, C algorithm is faster and less CPU-intensive +than C and C. + +Related reading material: +Percona Toolkit UDFs: L +How to avoid hash collisions when using MySQL’s CRC32 function: L + =head1 LIMITATIONS =over From 89e700c3c38a1e02d9d818c7dc0b7b191ac9e330 Mon Sep 17 00:00:00 2001 From: Carlos Salguero Date: Thu, 21 Sep 2017 11:31:36 -0300 Subject: [PATCH 10/13] Revert "Turn off statement based binlog checks" --- bin/pt-table-checksum | 57 +++++++++++++++++++++---------------------- 1 file changed, 28 insertions(+), 29 deletions(-) diff --git a/bin/pt-table-checksum b/bin/pt-table-checksum index 02a32f42..32d084bf 100755 --- a/bin/pt-table-checksum +++ b/bin/pt-table-checksum @@ -9331,36 +9331,35 @@ sub main { die "Error setting SQL_MODE" . ": $EVAL_ERROR"; } + - if ( $o->get('check-binlog-format') ) { - # https://bugs.launchpad.net/percona-toolkit/+bug/919352 - # The tool shouldn't blindly attempt to change binlog_format; - # instead, it should check if it's already set to STATEMENT. - # This is becase starting with MySQL 5.1.29, changing the format - # requires a SUPER user. - if ( VersionParser->new($dbh) >= '5.1.5' ) { - $sql = 'SELECT @@binlog_format'; - PTDEBUG && _d($dbh, $sql); - my ($original_binlog_format) = $dbh->selectrow_array($sql); - PTDEBUG && _d('Original binlog_format:', $original_binlog_format); - if ( $original_binlog_format !~ /STATEMENT/i ) { - $sql = q{/*!50108 SET @@binlog_format := 'STATEMENT'*/}; - eval { - PTDEBUG && _d($dbh, $sql); - $dbh->do($sql); - }; - if ( $EVAL_ERROR ) { - die "Failed to $sql: $EVAL_ERROR\n" - . "This tool requires binlog_format=STATEMENT, " - . "but the current binlog_format is set to " - ."$original_binlog_format and an error occurred while " - . "attempting to change it. If running MySQL 5.1.29 or newer, " - . "setting binlog_format requires the SUPER privilege. " - . "You will need to manually set binlog_format to 'STATEMENT' " - . "before running this tool.\n"; - } - } - } + # https://bugs.launchpad.net/percona-toolkit/+bug/919352 + # The tool shouldn't blindly attempt to change binlog_format; + # instead, it should check if it's already set to STATEMENT. + # This is becase starting with MySQL 5.1.29, changing the format + # requires a SUPER user. + if ( VersionParser->new($dbh) >= '5.1.5' ) { + $sql = 'SELECT @@binlog_format'; + PTDEBUG && _d($dbh, $sql); + my ($original_binlog_format) = $dbh->selectrow_array($sql); + PTDEBUG && _d('Original binlog_format:', $original_binlog_format); + if ( $original_binlog_format !~ /STATEMENT/i ) { + $sql = q{/*!50108 SET @@binlog_format := 'STATEMENT'*/}; + eval { + PTDEBUG && _d($dbh, $sql); + $dbh->do($sql); + }; + if ( $EVAL_ERROR ) { + die "Failed to $sql: $EVAL_ERROR\n" + . "This tool requires binlog_format=STATEMENT, " + . "but the current binlog_format is set to " + ."$original_binlog_format and an error occurred while " + . "attempting to change it. If running MySQL 5.1.29 or newer, " + . "setting binlog_format requires the SUPER privilege. " + . "You will need to manually set binlog_format to 'STATEMENT' " + . "before running this tool.\n"; + } + } } # Set transaction isolation level. We set binlog_format to STATEMENT, From 4c15a3978476ab612143be4d1cc0c7506b6d9a66 Mon Sep 17 00:00:00 2001 From: Carlos Salguero Date: Mon, 25 Sep 2017 15:16:14 -0300 Subject: [PATCH 11/13] PT-200 Index with keyword 'unique' --- bin/pt-online-schema-change | 2 +- t/pt-online-schema-change/pt-196.t | 4 +- t/pt-online-schema-change/pt-200.t | 72 ++++++++++++++++++++++++++++++ 3 files changed, 74 insertions(+), 4 deletions(-) create mode 100644 t/pt-online-schema-change/pt-200.t diff --git a/bin/pt-online-schema-change b/bin/pt-online-schema-change index 47436bc8..be367429 100755 --- a/bin/pt-online-schema-change +++ b/bin/pt-online-schema-change @@ -10111,7 +10111,7 @@ sub get_unique_index_fields { $clean .= $suffix; my $fields = []; - my $fields_re = qr/(?:PRIMARY|UNIQUE)\s*(?:INDEX|KEY|)\s*(?:.*?)\s*\((.*?)\)/i; + my $fields_re = qr/\s(?:PRIMARY|UNIQUE)\s+(?:INDEX|KEY|)\s*(?:.*?)\s*\((.*?)\)/i; while($clean =~ /$fields_re/g) { push @$fields, [ split /\s*,\s*/, $1 ]; diff --git a/t/pt-online-schema-change/pt-196.t b/t/pt-online-schema-change/pt-196.t index 973f5b2e..9e9e4172 100644 --- a/t/pt-online-schema-change/pt-196.t +++ b/t/pt-online-schema-change/pt-196.t @@ -45,9 +45,7 @@ $sb->load_file('master', "$sample/pt-196.sql"); ($output, $exit_status) = full_output( sub { pt_online_schema_change::main(@args, "$master_dsn,D=test,t=test", - '--execute', '--alter', 'DROP COLUMN test', '--max-load', - 'THREADPOOL_IDLE_THREADS=1' - ), + '--execute', '--alter', 'DROP COLUMN test', ), }, ); diff --git a/t/pt-online-schema-change/pt-200.t b/t/pt-online-schema-change/pt-200.t new file mode 100644 index 00000000..5670146f --- /dev/null +++ b/t/pt-online-schema-change/pt-200.t @@ -0,0 +1,72 @@ +#!/usr/bin/env perl + +BEGIN { + die "The PERCONA_TOOLKIT_BRANCH environment variable is not set.\n" + unless $ENV{PERCONA_TOOLKIT_BRANCH} && -d $ENV{PERCONA_TOOLKIT_BRANCH}; + unshift @INC, "$ENV{PERCONA_TOOLKIT_BRANCH}/lib"; +}; + +use strict; +use warnings FATAL => 'all'; +use threads; + +use English qw(-no_match_vars); +use Test::More; + +use Data::Dumper; +use PerconaTest; +use Sandbox; +use SqlModes; +use File::Temp qw/ tempdir /; + +plan tests => 3; + +require "$trunk/bin/pt-online-schema-change"; + +my $dp = new DSNParser(opts=>$dsn_opts); +my $sb = new Sandbox(basedir => '/tmp', DSNParser => $dp); +my $master_dbh = $sb->get_dbh_for('master'); +my $master_dsn = 'h=127.1,P=12345,u=msandbox,p=msandbox'; + +if ( !$master_dbh ) { + plan skip_all => 'Cannot connect to sandbox master'; +} + +# The sandbox servers run with lock_wait_timeout=3 and it's not dynamic +# so we need to specify --set-vars innodb_lock_wait_timeout=3 else the +# tool will die. +my @args = (qw(--set-vars innodb_lock_wait_timeout=3)); +my $output; +my $exit_status; +my $sample = "t/pt-online-schema-change/samples/"; + +$sb->load_file('master', "$sample/pt-153.sql"); + +($output, $exit_status) = full_output( + sub { pt_online_schema_change::main(@args, "$master_dsn,D=test,t=t1", + '--execute', + '--alter', "ADD COLUMN unique_id BIGINT UNSIGNED NOT NULL DEFAULT 0, ADD INDEX(unique_id)", + ), + }, +); + +is( + $exit_status, + 0, + "PT-200 Adding a column named unique_xxx is not detected as an unique index", +); + +like( + $output, + qr/Successfully altered `test`.`t1`/s, + "PT-200 Adding field having 'unique' in the name", +); + +$master_dbh->do("DROP DATABASE IF EXISTS test"); + +# ############################################################################# +# Done. +# ############################################################################# +$sb->wipe_clean($master_dbh); +ok($sb->ok(), "Sandbox servers") or BAIL_OUT(__FILE__ . " broke the sandbox"); +done_testing; From b51d09d811b5a7cfee36e0498aebcff487f8d009 Mon Sep 17 00:00:00 2001 From: Carlos Salguero Date: Thu, 5 Oct 2017 15:19:57 -0300 Subject: [PATCH 12/13] PT-202 pt-online-schema-change fails with virtual columns Modified TableParser to ignore GENERATED columns from the columns list used to construct SELECTs/INSERTs --- bin/pt-archiver | 49 ++- bin/pt-duplicate-key-checker | 49 ++- bin/pt-find | 49 ++- bin/pt-heartbeat | 49 ++- bin/pt-index-usage | 49 ++- bin/pt-kill | 49 ++- bin/pt-online-schema-change | 56 ++- bin/pt-query-digest | 49 ++- bin/pt-table-checksum | 49 ++- bin/pt-table-sync | 49 ++- bin/pt-table-usage | 49 ++- lib/TableParser.pm | 49 ++- t/lib/TableParser.t | 398 +++++++++++++------ t/lib/samples/generated_cols.sql | 6 + t/pt-online-schema-change/pt-202.t | 76 ++++ t/pt-online-schema-change/samples/pt-202.sql | 9 + 16 files changed, 761 insertions(+), 323 deletions(-) create mode 100644 t/lib/samples/generated_cols.sql create mode 100644 t/pt-online-schema-change/pt-202.t create mode 100644 t/pt-online-schema-change/samples/pt-202.sql diff --git a/bin/pt-archiver b/bin/pt-archiver index 768af530..6f90ca17 100755 --- a/bin/pt-archiver +++ b/bin/pt-archiver @@ -1965,8 +1965,8 @@ sub parse { my %def_for; @def_for{@cols} = @defs; - my (@nums, @null); - my (%type_for, %is_nullable, %is_numeric, %is_autoinc); + my (@nums, @null, @non_generated); + my (%type_for, %is_nullable, %is_numeric, %is_autoinc, %is_generated); foreach my $col ( @cols ) { my $def = $def_for{$col}; @@ -1983,6 +1983,11 @@ sub parse { push @null, $col; $is_nullable{$col} = 1; } + if ( remove_quoted_text($def) =~ m/\WGENERATED\W/i ) { + $is_generated{$col} = 1; + } else { + push @non_generated, $col; + } $is_autoinc{$col} = $def =~ m/AUTO_INCREMENT/i ? 1 : 0; } @@ -1991,24 +1996,34 @@ sub parse { my ($charset) = $ddl =~ m/DEFAULT CHARSET=(\w+)/; return { - name => $name, - cols => \@cols, - col_posn => { map { $cols[$_] => $_ } 0..$#cols }, - is_col => { map { $_ => 1 } @cols }, - null_cols => \@null, - is_nullable => \%is_nullable, - is_autoinc => \%is_autoinc, - clustered_key => $clustered_key, - keys => $keys, - defs => \%def_for, - numeric_cols => \@nums, - is_numeric => \%is_numeric, - engine => $engine, - type_for => \%type_for, - charset => $charset, + name => $name, + cols => \@cols, + col_posn => { map { $cols[$_] => $_ } 0..$#cols }, + is_col => { map { $_ => 1 } @non_generated }, + null_cols => \@null, + is_nullable => \%is_nullable, + non_generated_cols => \@non_generated, + is_autoinc => \%is_autoinc, + is_generated => \%is_generated, + clustered_key => $clustered_key, + keys => $keys, + defs => \%def_for, + numeric_cols => \@nums, + is_numeric => \%is_numeric, + engine => $engine, + type_for => \%type_for, + charset => $charset, }; } +sub remove_quoted_text { + my ($string) = @_; + $string =~ s/[^\\]`[^`]*[^\\]`//g; + $string =~ s/[^\\]"[^"]*[^\\]"//g; + $string =~ s/[^\\]"[^"]*[^\\]"//g; + return $string; +} + sub sort_indexes { my ( $self, $tbl ) = @_; diff --git a/bin/pt-duplicate-key-checker b/bin/pt-duplicate-key-checker index f288a596..d82c75bc 100755 --- a/bin/pt-duplicate-key-checker +++ b/bin/pt-duplicate-key-checker @@ -352,8 +352,8 @@ sub parse { my %def_for; @def_for{@cols} = @defs; - my (@nums, @null); - my (%type_for, %is_nullable, %is_numeric, %is_autoinc); + my (@nums, @null, @non_generated); + my (%type_for, %is_nullable, %is_numeric, %is_autoinc, %is_generated); foreach my $col ( @cols ) { my $def = $def_for{$col}; @@ -370,6 +370,11 @@ sub parse { push @null, $col; $is_nullable{$col} = 1; } + if ( remove_quoted_text($def) =~ m/\WGENERATED\W/i ) { + $is_generated{$col} = 1; + } else { + push @non_generated, $col; + } $is_autoinc{$col} = $def =~ m/AUTO_INCREMENT/i ? 1 : 0; } @@ -378,24 +383,34 @@ sub parse { my ($charset) = $ddl =~ m/DEFAULT CHARSET=(\w+)/; return { - name => $name, - cols => \@cols, - col_posn => { map { $cols[$_] => $_ } 0..$#cols }, - is_col => { map { $_ => 1 } @cols }, - null_cols => \@null, - is_nullable => \%is_nullable, - is_autoinc => \%is_autoinc, - clustered_key => $clustered_key, - keys => $keys, - defs => \%def_for, - numeric_cols => \@nums, - is_numeric => \%is_numeric, - engine => $engine, - type_for => \%type_for, - charset => $charset, + name => $name, + cols => \@cols, + col_posn => { map { $cols[$_] => $_ } 0..$#cols }, + is_col => { map { $_ => 1 } @non_generated }, + null_cols => \@null, + is_nullable => \%is_nullable, + non_generated_cols => \@non_generated, + is_autoinc => \%is_autoinc, + is_generated => \%is_generated, + clustered_key => $clustered_key, + keys => $keys, + defs => \%def_for, + numeric_cols => \@nums, + is_numeric => \%is_numeric, + engine => $engine, + type_for => \%type_for, + charset => $charset, }; } +sub remove_quoted_text { + my ($string) = @_; + $string =~ s/[^\\]`[^`]*[^\\]`//g; + $string =~ s/[^\\]"[^"]*[^\\]"//g; + $string =~ s/[^\\]"[^"]*[^\\]"//g; + return $string; +} + sub sort_indexes { my ( $self, $tbl ) = @_; diff --git a/bin/pt-find b/bin/pt-find index 4f9c7bf4..fc02d69e 100755 --- a/bin/pt-find +++ b/bin/pt-find @@ -1880,8 +1880,8 @@ sub parse { my %def_for; @def_for{@cols} = @defs; - my (@nums, @null); - my (%type_for, %is_nullable, %is_numeric, %is_autoinc); + my (@nums, @null, @non_generated); + my (%type_for, %is_nullable, %is_numeric, %is_autoinc, %is_generated); foreach my $col ( @cols ) { my $def = $def_for{$col}; @@ -1898,6 +1898,11 @@ sub parse { push @null, $col; $is_nullable{$col} = 1; } + if ( remove_quoted_text($def) =~ m/\WGENERATED\W/i ) { + $is_generated{$col} = 1; + } else { + push @non_generated, $col; + } $is_autoinc{$col} = $def =~ m/AUTO_INCREMENT/i ? 1 : 0; } @@ -1906,24 +1911,34 @@ sub parse { my ($charset) = $ddl =~ m/DEFAULT CHARSET=(\w+)/; return { - name => $name, - cols => \@cols, - col_posn => { map { $cols[$_] => $_ } 0..$#cols }, - is_col => { map { $_ => 1 } @cols }, - null_cols => \@null, - is_nullable => \%is_nullable, - is_autoinc => \%is_autoinc, - clustered_key => $clustered_key, - keys => $keys, - defs => \%def_for, - numeric_cols => \@nums, - is_numeric => \%is_numeric, - engine => $engine, - type_for => \%type_for, - charset => $charset, + name => $name, + cols => \@cols, + col_posn => { map { $cols[$_] => $_ } 0..$#cols }, + is_col => { map { $_ => 1 } @non_generated }, + null_cols => \@null, + is_nullable => \%is_nullable, + non_generated_cols => \@non_generated, + is_autoinc => \%is_autoinc, + is_generated => \%is_generated, + clustered_key => $clustered_key, + keys => $keys, + defs => \%def_for, + numeric_cols => \@nums, + is_numeric => \%is_numeric, + engine => $engine, + type_for => \%type_for, + charset => $charset, }; } +sub remove_quoted_text { + my ($string) = @_; + $string =~ s/[^\\]`[^`]*[^\\]`//g; + $string =~ s/[^\\]"[^"]*[^\\]"//g; + $string =~ s/[^\\]"[^"]*[^\\]"//g; + return $string; +} + sub sort_indexes { my ( $self, $tbl ) = @_; diff --git a/bin/pt-heartbeat b/bin/pt-heartbeat index 667b037a..5b1ade9b 100755 --- a/bin/pt-heartbeat +++ b/bin/pt-heartbeat @@ -3499,8 +3499,8 @@ sub parse { my %def_for; @def_for{@cols} = @defs; - my (@nums, @null); - my (%type_for, %is_nullable, %is_numeric, %is_autoinc); + my (@nums, @null, @non_generated); + my (%type_for, %is_nullable, %is_numeric, %is_autoinc, %is_generated); foreach my $col ( @cols ) { my $def = $def_for{$col}; @@ -3517,6 +3517,11 @@ sub parse { push @null, $col; $is_nullable{$col} = 1; } + if ( remove_quoted_text($def) =~ m/\WGENERATED\W/i ) { + $is_generated{$col} = 1; + } else { + push @non_generated, $col; + } $is_autoinc{$col} = $def =~ m/AUTO_INCREMENT/i ? 1 : 0; } @@ -3525,24 +3530,34 @@ sub parse { my ($charset) = $ddl =~ m/DEFAULT CHARSET=(\w+)/; return { - name => $name, - cols => \@cols, - col_posn => { map { $cols[$_] => $_ } 0..$#cols }, - is_col => { map { $_ => 1 } @cols }, - null_cols => \@null, - is_nullable => \%is_nullable, - is_autoinc => \%is_autoinc, - clustered_key => $clustered_key, - keys => $keys, - defs => \%def_for, - numeric_cols => \@nums, - is_numeric => \%is_numeric, - engine => $engine, - type_for => \%type_for, - charset => $charset, + name => $name, + cols => \@cols, + col_posn => { map { $cols[$_] => $_ } 0..$#cols }, + is_col => { map { $_ => 1 } @non_generated }, + null_cols => \@null, + is_nullable => \%is_nullable, + non_generated_cols => \@non_generated, + is_autoinc => \%is_autoinc, + is_generated => \%is_generated, + clustered_key => $clustered_key, + keys => $keys, + defs => \%def_for, + numeric_cols => \@nums, + is_numeric => \%is_numeric, + engine => $engine, + type_for => \%type_for, + charset => $charset, }; } +sub remove_quoted_text { + my ($string) = @_; + $string =~ s/[^\\]`[^`]*[^\\]`//g; + $string =~ s/[^\\]"[^"]*[^\\]"//g; + $string =~ s/[^\\]"[^"]*[^\\]"//g; + return $string; +} + sub sort_indexes { my ( $self, $tbl ) = @_; diff --git a/bin/pt-index-usage b/bin/pt-index-usage index c58cacaa..72b4b55c 100755 --- a/bin/pt-index-usage +++ b/bin/pt-index-usage @@ -3119,8 +3119,8 @@ sub parse { my %def_for; @def_for{@cols} = @defs; - my (@nums, @null); - my (%type_for, %is_nullable, %is_numeric, %is_autoinc); + my (@nums, @null, @non_generated); + my (%type_for, %is_nullable, %is_numeric, %is_autoinc, %is_generated); foreach my $col ( @cols ) { my $def = $def_for{$col}; @@ -3137,6 +3137,11 @@ sub parse { push @null, $col; $is_nullable{$col} = 1; } + if ( remove_quoted_text($def) =~ m/\WGENERATED\W/i ) { + $is_generated{$col} = 1; + } else { + push @non_generated, $col; + } $is_autoinc{$col} = $def =~ m/AUTO_INCREMENT/i ? 1 : 0; } @@ -3145,24 +3150,34 @@ sub parse { my ($charset) = $ddl =~ m/DEFAULT CHARSET=(\w+)/; return { - name => $name, - cols => \@cols, - col_posn => { map { $cols[$_] => $_ } 0..$#cols }, - is_col => { map { $_ => 1 } @cols }, - null_cols => \@null, - is_nullable => \%is_nullable, - is_autoinc => \%is_autoinc, - clustered_key => $clustered_key, - keys => $keys, - defs => \%def_for, - numeric_cols => \@nums, - is_numeric => \%is_numeric, - engine => $engine, - type_for => \%type_for, - charset => $charset, + name => $name, + cols => \@cols, + col_posn => { map { $cols[$_] => $_ } 0..$#cols }, + is_col => { map { $_ => 1 } @non_generated }, + null_cols => \@null, + is_nullable => \%is_nullable, + non_generated_cols => \@non_generated, + is_autoinc => \%is_autoinc, + is_generated => \%is_generated, + clustered_key => $clustered_key, + keys => $keys, + defs => \%def_for, + numeric_cols => \@nums, + is_numeric => \%is_numeric, + engine => $engine, + type_for => \%type_for, + charset => $charset, }; } +sub remove_quoted_text { + my ($string) = @_; + $string =~ s/[^\\]`[^`]*[^\\]`//g; + $string =~ s/[^\\]"[^"]*[^\\]"//g; + $string =~ s/[^\\]"[^"]*[^\\]"//g; + return $string; +} + sub sort_indexes { my ( $self, $tbl ) = @_; diff --git a/bin/pt-kill b/bin/pt-kill index 11663760..ba95733b 100755 --- a/bin/pt-kill +++ b/bin/pt-kill @@ -2945,8 +2945,8 @@ sub parse { my %def_for; @def_for{@cols} = @defs; - my (@nums, @null); - my (%type_for, %is_nullable, %is_numeric, %is_autoinc); + my (@nums, @null, @non_generated); + my (%type_for, %is_nullable, %is_numeric, %is_autoinc, %is_generated); foreach my $col ( @cols ) { my $def = $def_for{$col}; @@ -2963,6 +2963,11 @@ sub parse { push @null, $col; $is_nullable{$col} = 1; } + if ( remove_quoted_text($def) =~ m/\WGENERATED\W/i ) { + $is_generated{$col} = 1; + } else { + push @non_generated, $col; + } $is_autoinc{$col} = $def =~ m/AUTO_INCREMENT/i ? 1 : 0; } @@ -2971,24 +2976,34 @@ sub parse { my ($charset) = $ddl =~ m/DEFAULT CHARSET=(\w+)/; return { - name => $name, - cols => \@cols, - col_posn => { map { $cols[$_] => $_ } 0..$#cols }, - is_col => { map { $_ => 1 } @cols }, - null_cols => \@null, - is_nullable => \%is_nullable, - is_autoinc => \%is_autoinc, - clustered_key => $clustered_key, - keys => $keys, - defs => \%def_for, - numeric_cols => \@nums, - is_numeric => \%is_numeric, - engine => $engine, - type_for => \%type_for, - charset => $charset, + name => $name, + cols => \@cols, + col_posn => { map { $cols[$_] => $_ } 0..$#cols }, + is_col => { map { $_ => 1 } @non_generated }, + null_cols => \@null, + is_nullable => \%is_nullable, + non_generated_cols => \@non_generated, + is_autoinc => \%is_autoinc, + is_generated => \%is_generated, + clustered_key => $clustered_key, + keys => $keys, + defs => \%def_for, + numeric_cols => \@nums, + is_numeric => \%is_numeric, + engine => $engine, + type_for => \%type_for, + charset => $charset, }; } +sub remove_quoted_text { + my ($string) = @_; + $string =~ s/[^\\]`[^`]*[^\\]`//g; + $string =~ s/[^\\]"[^"]*[^\\]"//g; + $string =~ s/[^\\]"[^"]*[^\\]"//g; + return $string; +} + sub sort_indexes { my ( $self, $tbl ) = @_; diff --git a/bin/pt-online-schema-change b/bin/pt-online-schema-change index be367429..bf4fcaa8 100755 --- a/bin/pt-online-schema-change +++ b/bin/pt-online-schema-change @@ -56,7 +56,7 @@ BEGIN { { package Percona::Toolkit; -our $VERSION = '3.0.4'; +our $VERSION = '3.0.5'; use strict; use warnings FATAL => 'all'; @@ -3309,8 +3309,8 @@ sub parse { my %def_for; @def_for{@cols} = @defs; - my (@nums, @null); - my (%type_for, %is_nullable, %is_numeric, %is_autoinc); + my (@nums, @null, @non_generated); + my (%type_for, %is_nullable, %is_numeric, %is_autoinc, %is_generated); foreach my $col ( @cols ) { my $def = $def_for{$col}; @@ -3327,6 +3327,11 @@ sub parse { push @null, $col; $is_nullable{$col} = 1; } + if ( remove_quoted_text($def) =~ m/\WGENERATED\W/i ) { + $is_generated{$col} = 1; + } else { + push @non_generated, $col; + } $is_autoinc{$col} = $def =~ m/AUTO_INCREMENT/i ? 1 : 0; } @@ -3335,24 +3340,34 @@ sub parse { my ($charset) = $ddl =~ m/DEFAULT CHARSET=(\w+)/; return { - name => $name, - cols => \@cols, - col_posn => { map { $cols[$_] => $_ } 0..$#cols }, - is_col => { map { $_ => 1 } @cols }, - null_cols => \@null, - is_nullable => \%is_nullable, - is_autoinc => \%is_autoinc, - clustered_key => $clustered_key, - keys => $keys, - defs => \%def_for, - numeric_cols => \@nums, - is_numeric => \%is_numeric, - engine => $engine, - type_for => \%type_for, - charset => $charset, + name => $name, + cols => \@cols, + col_posn => { map { $cols[$_] => $_ } 0..$#cols }, + is_col => { map { $_ => 1 } @non_generated }, + null_cols => \@null, + is_nullable => \%is_nullable, + non_generated_cols => \@non_generated, + is_autoinc => \%is_autoinc, + is_generated => \%is_generated, + clustered_key => $clustered_key, + keys => $keys, + defs => \%def_for, + numeric_cols => \@nums, + is_numeric => \%is_numeric, + engine => $engine, + type_for => \%type_for, + charset => $charset, }; } +sub remove_quoted_text { + my ($string) = @_; + $string =~ s/[^\\]`[^`]*[^\\]`//g; + $string =~ s/[^\\]"[^"]*[^\\]"//g; + $string =~ s/[^\\]"[^"]*[^\\]"//g; + return $string; +} + sub sort_indexes { my ( $self, $tbl ) = @_; @@ -11582,6 +11597,11 @@ The tool exits with an error if the host is a cluster node and the table is MyISAM or is being converted to MyISAM (C), or if C is not C. There is no way to disable these checks. +=head1 MySQL 5.7+ Generated columns + +The tools ignores MySQL 5.7+ C columns since the value for those columns +is generated according to the expresion used to compute column values. + =head1 OUTPUT The tool prints information about its activities to STDOUT so that you can see diff --git a/bin/pt-query-digest b/bin/pt-query-digest index f01767a5..1e504c8e 100755 --- a/bin/pt-query-digest +++ b/bin/pt-query-digest @@ -8857,8 +8857,8 @@ sub parse { my %def_for; @def_for{@cols} = @defs; - my (@nums, @null); - my (%type_for, %is_nullable, %is_numeric, %is_autoinc); + my (@nums, @null, @non_generated); + my (%type_for, %is_nullable, %is_numeric, %is_autoinc, %is_generated); foreach my $col ( @cols ) { my $def = $def_for{$col}; @@ -8875,6 +8875,11 @@ sub parse { push @null, $col; $is_nullable{$col} = 1; } + if ( remove_quoted_text($def) =~ m/\WGENERATED\W/i ) { + $is_generated{$col} = 1; + } else { + push @non_generated, $col; + } $is_autoinc{$col} = $def =~ m/AUTO_INCREMENT/i ? 1 : 0; } @@ -8883,24 +8888,34 @@ sub parse { my ($charset) = $ddl =~ m/DEFAULT CHARSET=(\w+)/; return { - name => $name, - cols => \@cols, - col_posn => { map { $cols[$_] => $_ } 0..$#cols }, - is_col => { map { $_ => 1 } @cols }, - null_cols => \@null, - is_nullable => \%is_nullable, - is_autoinc => \%is_autoinc, - clustered_key => $clustered_key, - keys => $keys, - defs => \%def_for, - numeric_cols => \@nums, - is_numeric => \%is_numeric, - engine => $engine, - type_for => \%type_for, - charset => $charset, + name => $name, + cols => \@cols, + col_posn => { map { $cols[$_] => $_ } 0..$#cols }, + is_col => { map { $_ => 1 } @non_generated }, + null_cols => \@null, + is_nullable => \%is_nullable, + non_generated_cols => \@non_generated, + is_autoinc => \%is_autoinc, + is_generated => \%is_generated, + clustered_key => $clustered_key, + keys => $keys, + defs => \%def_for, + numeric_cols => \@nums, + is_numeric => \%is_numeric, + engine => $engine, + type_for => \%type_for, + charset => $charset, }; } +sub remove_quoted_text { + my ($string) = @_; + $string =~ s/[^\\]`[^`]*[^\\]`//g; + $string =~ s/[^\\]"[^"]*[^\\]"//g; + $string =~ s/[^\\]"[^"]*[^\\]"//g; + return $string; +} + sub sort_indexes { my ( $self, $tbl ) = @_; diff --git a/bin/pt-table-checksum b/bin/pt-table-checksum index 32d084bf..be89bb12 100755 --- a/bin/pt-table-checksum +++ b/bin/pt-table-checksum @@ -4446,8 +4446,8 @@ sub parse { my %def_for; @def_for{@cols} = @defs; - my (@nums, @null); - my (%type_for, %is_nullable, %is_numeric, %is_autoinc); + my (@nums, @null, @non_generated); + my (%type_for, %is_nullable, %is_numeric, %is_autoinc, %is_generated); foreach my $col ( @cols ) { my $def = $def_for{$col}; @@ -4464,6 +4464,11 @@ sub parse { push @null, $col; $is_nullable{$col} = 1; } + if ( remove_quoted_text($def) =~ m/\WGENERATED\W/i ) { + $is_generated{$col} = 1; + } else { + push @non_generated, $col; + } $is_autoinc{$col} = $def =~ m/AUTO_INCREMENT/i ? 1 : 0; } @@ -4472,24 +4477,34 @@ sub parse { my ($charset) = $ddl =~ m/DEFAULT CHARSET=(\w+)/; return { - name => $name, - cols => \@cols, - col_posn => { map { $cols[$_] => $_ } 0..$#cols }, - is_col => { map { $_ => 1 } @cols }, - null_cols => \@null, - is_nullable => \%is_nullable, - is_autoinc => \%is_autoinc, - clustered_key => $clustered_key, - keys => $keys, - defs => \%def_for, - numeric_cols => \@nums, - is_numeric => \%is_numeric, - engine => $engine, - type_for => \%type_for, - charset => $charset, + name => $name, + cols => \@cols, + col_posn => { map { $cols[$_] => $_ } 0..$#cols }, + is_col => { map { $_ => 1 } @non_generated }, + null_cols => \@null, + is_nullable => \%is_nullable, + non_generated_cols => \@non_generated, + is_autoinc => \%is_autoinc, + is_generated => \%is_generated, + clustered_key => $clustered_key, + keys => $keys, + defs => \%def_for, + numeric_cols => \@nums, + is_numeric => \%is_numeric, + engine => $engine, + type_for => \%type_for, + charset => $charset, }; } +sub remove_quoted_text { + my ($string) = @_; + $string =~ s/[^\\]`[^`]*[^\\]`//g; + $string =~ s/[^\\]"[^"]*[^\\]"//g; + $string =~ s/[^\\]"[^"]*[^\\]"//g; + return $string; +} + sub sort_indexes { my ( $self, $tbl ) = @_; diff --git a/bin/pt-table-sync b/bin/pt-table-sync index 0f2bdd05..30fff765 100755 --- a/bin/pt-table-sync +++ b/bin/pt-table-sync @@ -2864,8 +2864,8 @@ sub parse { my %def_for; @def_for{@cols} = @defs; - my (@nums, @null); - my (%type_for, %is_nullable, %is_numeric, %is_autoinc); + my (@nums, @null, @non_generated); + my (%type_for, %is_nullable, %is_numeric, %is_autoinc, %is_generated); foreach my $col ( @cols ) { my $def = $def_for{$col}; @@ -2882,6 +2882,11 @@ sub parse { push @null, $col; $is_nullable{$col} = 1; } + if ( remove_quoted_text($def) =~ m/\WGENERATED\W/i ) { + $is_generated{$col} = 1; + } else { + push @non_generated, $col; + } $is_autoinc{$col} = $def =~ m/AUTO_INCREMENT/i ? 1 : 0; } @@ -2890,24 +2895,34 @@ sub parse { my ($charset) = $ddl =~ m/DEFAULT CHARSET=(\w+)/; return { - name => $name, - cols => \@cols, - col_posn => { map { $cols[$_] => $_ } 0..$#cols }, - is_col => { map { $_ => 1 } @cols }, - null_cols => \@null, - is_nullable => \%is_nullable, - is_autoinc => \%is_autoinc, - clustered_key => $clustered_key, - keys => $keys, - defs => \%def_for, - numeric_cols => \@nums, - is_numeric => \%is_numeric, - engine => $engine, - type_for => \%type_for, - charset => $charset, + name => $name, + cols => \@cols, + col_posn => { map { $cols[$_] => $_ } 0..$#cols }, + is_col => { map { $_ => 1 } @non_generated }, + null_cols => \@null, + is_nullable => \%is_nullable, + non_generated_cols => \@non_generated, + is_autoinc => \%is_autoinc, + is_generated => \%is_generated, + clustered_key => $clustered_key, + keys => $keys, + defs => \%def_for, + numeric_cols => \@nums, + is_numeric => \%is_numeric, + engine => $engine, + type_for => \%type_for, + charset => $charset, }; } +sub remove_quoted_text { + my ($string) = @_; + $string =~ s/[^\\]`[^`]*[^\\]`//g; + $string =~ s/[^\\]"[^"]*[^\\]"//g; + $string =~ s/[^\\]"[^"]*[^\\]"//g; + return $string; +} + sub sort_indexes { my ( $self, $tbl ) = @_; diff --git a/bin/pt-table-usage b/bin/pt-table-usage index 5dcdd8e9..829d0912 100755 --- a/bin/pt-table-usage +++ b/bin/pt-table-usage @@ -6795,8 +6795,8 @@ sub parse { my %def_for; @def_for{@cols} = @defs; - my (@nums, @null); - my (%type_for, %is_nullable, %is_numeric, %is_autoinc); + my (@nums, @null, @non_generated); + my (%type_for, %is_nullable, %is_numeric, %is_autoinc, %is_generated); foreach my $col ( @cols ) { my $def = $def_for{$col}; @@ -6813,6 +6813,11 @@ sub parse { push @null, $col; $is_nullable{$col} = 1; } + if ( remove_quoted_text($def) =~ m/\WGENERATED\W/i ) { + $is_generated{$col} = 1; + } else { + push @non_generated, $col; + } $is_autoinc{$col} = $def =~ m/AUTO_INCREMENT/i ? 1 : 0; } @@ -6821,24 +6826,34 @@ sub parse { my ($charset) = $ddl =~ m/DEFAULT CHARSET=(\w+)/; return { - name => $name, - cols => \@cols, - col_posn => { map { $cols[$_] => $_ } 0..$#cols }, - is_col => { map { $_ => 1 } @cols }, - null_cols => \@null, - is_nullable => \%is_nullable, - is_autoinc => \%is_autoinc, - clustered_key => $clustered_key, - keys => $keys, - defs => \%def_for, - numeric_cols => \@nums, - is_numeric => \%is_numeric, - engine => $engine, - type_for => \%type_for, - charset => $charset, + name => $name, + cols => \@cols, + col_posn => { map { $cols[$_] => $_ } 0..$#cols }, + is_col => { map { $_ => 1 } @non_generated }, + null_cols => \@null, + is_nullable => \%is_nullable, + non_generated_cols => \@non_generated, + is_autoinc => \%is_autoinc, + is_generated => \%is_generated, + clustered_key => $clustered_key, + keys => $keys, + defs => \%def_for, + numeric_cols => \@nums, + is_numeric => \%is_numeric, + engine => $engine, + type_for => \%type_for, + charset => $charset, }; } +sub remove_quoted_text { + my ($string) = @_; + $string =~ s/[^\\]`[^`]*[^\\]`//g; + $string =~ s/[^\\]"[^"]*[^\\]"//g; + $string =~ s/[^\\]"[^"]*[^\\]"//g; + return $string; +} + sub sort_indexes { my ( $self, $tbl ) = @_; diff --git a/lib/TableParser.pm b/lib/TableParser.pm index 4b5904bb..511c5c67 100644 --- a/lib/TableParser.pm +++ b/lib/TableParser.pm @@ -159,8 +159,8 @@ sub parse { # Find column types, whether numeric, whether nullable, whether # auto-increment. - my (@nums, @null); - my (%type_for, %is_nullable, %is_numeric, %is_autoinc); + my (@nums, @null, @non_generated); + my (%type_for, %is_nullable, %is_numeric, %is_autoinc, %is_generated); foreach my $col ( @cols ) { my $def = $def_for{$col}; @@ -180,6 +180,11 @@ sub parse { push @null, $col; $is_nullable{$col} = 1; } + if ( remove_quoted_text($def) =~ m/\WGENERATED\W/i ) { + $is_generated{$col} = 1; + } else { + push @non_generated, $col; + } $is_autoinc{$col} = $def =~ m/AUTO_INCREMENT/i ? 1 : 0; } @@ -191,24 +196,34 @@ sub parse { my ($charset) = $ddl =~ m/DEFAULT CHARSET=(\w+)/; return { - name => $name, - cols => \@cols, - col_posn => { map { $cols[$_] => $_ } 0..$#cols }, - is_col => { map { $_ => 1 } @cols }, - null_cols => \@null, - is_nullable => \%is_nullable, - is_autoinc => \%is_autoinc, - clustered_key => $clustered_key, - keys => $keys, - defs => \%def_for, - numeric_cols => \@nums, - is_numeric => \%is_numeric, - engine => $engine, - type_for => \%type_for, - charset => $charset, + name => $name, + cols => \@cols, + col_posn => { map { $cols[$_] => $_ } 0..$#cols }, + is_col => { map { $_ => 1 } @non_generated }, + null_cols => \@null, + is_nullable => \%is_nullable, + non_generated_cols => \@non_generated, + is_autoinc => \%is_autoinc, + is_generated => \%is_generated, + clustered_key => $clustered_key, + keys => $keys, + defs => \%def_for, + numeric_cols => \@nums, + is_numeric => \%is_numeric, + engine => $engine, + type_for => \%type_for, + charset => $charset, }; } +sub remove_quoted_text { + my ($string) = @_; + $string =~ s/[^\\]`[^`]*[^\\]`//g; + $string =~ s/[^\\]"[^"]*[^\\]"//g; + $string =~ s/[^\\]"[^"]*[^\\]"//g; + return $string; +} + # Sorts indexes in this order: PRIMARY, unique, non-nullable, any (shortest # first, alphabetical). Only BTREE indexes are considered. # TODO: consider length as # of bytes instead of # of columns. diff --git a/t/lib/TableParser.t b/t/lib/TableParser.t index 8a063ee6..91137219 100644 --- a/t/lib/TableParser.t +++ b/t/lib/TableParser.t @@ -11,6 +11,7 @@ use warnings FATAL => 'all'; use English qw(-no_match_vars); use Test::More; +use Text::Diff; use TableParser; use Quoter; use DSNParser; @@ -71,21 +72,23 @@ like($EVAL_ERROR, qr/quoting/, 'No quoting'); $tbl = $tp->parse( load_file('t/lib/samples/t1.sql') ); is_deeply( $tbl, - { cols => [qw(a)], - col_posn => { a => 0 }, - is_col => { a => 1 }, - is_autoinc => { a => 0 }, - null_cols => [qw(a)], - is_nullable => { a => 1 }, - clustered_key => undef, - keys => {}, - defs => { a => ' `a` int(11) default NULL' }, - numeric_cols => [qw(a)], - is_numeric => { a => 1 }, - engine => 'MyISAM', - type_for => { a => 'int' }, - name => 't1', - charset => 'latin1', + { cols => [qw(a)], + col_posn => { a => 0 }, + is_col => { a => 1 }, + is_autoinc => { a => 0 }, + null_cols => [qw(a)], + is_generated => {}, + non_generated_cols => [ 'a' ], + is_nullable => { a => 1 }, + clustered_key => undef, + keys => {}, + defs => { a => ' `a` int(11) default NULL' }, + numeric_cols => [qw(a)], + is_numeric => { a => 1 }, + engine => 'MyISAM', + type_for => { a => 'int' }, + name => 't1', + charset => 'latin1', }, 'Basic table is OK', ); @@ -94,11 +97,13 @@ $tbl = $tp->parse( load_file('t/lib/samples/TableParser-prefix_idx.sql') ); is_deeply( $tbl, { - name => 't1', - cols => [ 'a', 'b' ], - col_posn => { a => 0, b => 1 }, - is_col => { a => 1, b => 1 }, - is_autoinc => { 'a' => 0, 'b' => 0 }, + name => 't1', + cols => [ 'a', 'b' ], + non_generated_cols => [ 'a', 'b' ], + col_posn => { a => 0, b => 1 }, + is_col => { a => 1, b => 1 }, + is_autoinc => { 'a' => 0, 'b' => 0 }, + is_generated => {}, null_cols => [ 'a', 'b' ], is_nullable => { 'a' => 1, 'b' => 1 }, clustered_key => undef, @@ -169,6 +174,12 @@ is_deeply( length replacement_cost rating special_features last_update) ], + non_generated_cols => [ + qw(film_id title description release_year language_id + original_language_id rental_duration rental_rate + length replacement_cost rating special_features + last_update) + ], col_posn => { film_id => 0, title => 1, @@ -214,6 +225,7 @@ is_deeply( special_features => 1, last_update => 1, }, + is_generated => {}, null_cols => [qw(description release_year original_language_id length rating special_features )], is_nullable => { description => 1, @@ -342,25 +354,109 @@ throws_ok ( $tbl = $tp->parse( load_file('t/lib/samples/temporary_table.sql') ); is_deeply( $tbl, - { cols => [qw(a)], - col_posn => { a => 0 }, - is_col => { a => 1 }, - is_autoinc => { a => 0 }, - null_cols => [qw(a)], - is_nullable => { a => 1 }, - clustered_key => undef, - keys => {}, - defs => { a => ' `a` int(11) default NULL' }, - numeric_cols => [qw(a)], - is_numeric => { a => 1 }, - engine => 'MyISAM', - type_for => { a => 'int' }, - name => 't', - charset => 'latin1', + { cols => [qw(a)], + non_generated_cols => [qw(a)], + col_posn => { a => 0 }, + is_col => { a => 1 }, + is_autoinc => { a => 0 }, + is_generated => {}, + null_cols => [qw(a)], + is_nullable => { a => 1 }, + clustered_key => undef, + keys => {}, + defs => { a => ' `a` int(11) default NULL' }, + numeric_cols => [qw(a)], + is_numeric => { a => 1 }, + engine => 'MyISAM', + type_for => { a => 'int' }, + name => 't', + charset => 'latin1', }, 'Temporary table', ); +my $want = { 'is_autoinc' => { + 'sort_order' => 0, + 'pfk-source_instrument_id' => 0, + 'pfk-related_instrument_id' => 0 + }, + 'null_cols' => [], + 'numeric_cols' => [ + 'pfk-source_instrument_id', 'pfk-related_instrument_id', + 'sort_order' + ], + 'cols' => [ + 'pfk-source_instrument_id', 'pfk-related_instrument_id', + 'sort_order' + ], + 'non_generated_cols' => [ + 'pfk-source_instrument_id', 'pfk-related_instrument_id', + 'sort_order' + ], + is_autogenerated => {}, + 'col_posn' => { + 'sort_order' => 2, + 'pfk-source_instrument_id' => 0, + 'pfk-related_instrument_id' => 1 + }, + clustered_key => 'PRIMARY', + 'keys' => { + 'sort_order' => { + 'is_unique' => 0, + 'is_col' => { 'sort_order' => 1 }, + 'name' => 'sort_order', + 'type' => 'BTREE', + 'col_prefixes' => [ undef ], + 'is_nullable' => 0, + 'colnames' => '`sort_order`', + 'cols' => [ 'sort_order' ], + ddl => 'KEY `sort_order` (`sort_order`)', + }, + 'PRIMARY' => { + 'is_unique' => 1, + 'is_col' => { + 'pfk-source_instrument_id' => 1, + 'pfk-related_instrument_id' => 1 + }, + 'name' => 'PRIMARY', + 'type' => 'BTREE', + 'col_prefixes' => [ undef, undef ], + 'is_nullable' => 0, + 'colnames' => + '`pfk-source_instrument_id`,`pfk-related_instrument_id`', + 'cols' => + [ 'pfk-source_instrument_id', 'pfk-related_instrument_id' ], + ddl => 'PRIMARY KEY (`pfk-source_instrument_id`,`pfk-related_instrument_id`),', + } + }, + 'defs' => { + 'sort_order' => ' `sort_order` int(11) NOT NULL', + 'pfk-source_instrument_id' => + ' `pfk-source_instrument_id` int(10) unsigned NOT NULL', + 'pfk-related_instrument_id' => + ' `pfk-related_instrument_id` int(10) unsigned NOT NULL' + }, + 'engine' => 'InnoDB', + 'is_col' => { + 'sort_order' => 1, + 'pfk-source_instrument_id' => 1, + 'pfk-related_instrument_id' => 1 + }, + 'is_numeric' => { + 'sort_order' => 1, + 'pfk-source_instrument_id' => 1, + 'pfk-related_instrument_id' => 1 + }, + 'type_for' => { + 'sort_order' => 'int', + 'pfk-source_instrument_id' => 'int', + 'pfk-related_instrument_id' => 'int' + }, + 'is_nullable' => {}, + name => 'instrument_relation', + charset => 'latin1', + }; + $tbl = $tp->parse( load_file('t/lib/samples/hyphentest.sql') ); is_deeply( $tbl, @@ -378,6 +474,11 @@ is_deeply( 'pfk-source_instrument_id', 'pfk-related_instrument_id', 'sort_order' ], + 'non_generated_cols' => [ + 'pfk-source_instrument_id', 'pfk-related_instrument_id', + 'sort_order' + ], + is_generated => {}, 'col_posn' => { 'sort_order' => 2, 'pfk-source_instrument_id' => 0, @@ -446,13 +547,14 @@ is_deeply( $tbl = $tp->parse( load_file('t/lib/samples/ndb_table.sql') ); is_deeply( $tbl, - { cols => [qw(id)], - col_posn => { id => 0 }, - is_col => { id => 1 }, - is_autoinc => { id => 1 }, - null_cols => [], - is_nullable => {}, - clustered_key => undef, + { cols => [qw(id)], + non_generated_cols => [qw(id)], + col_posn => { id => 0 }, + is_col => { id => 1 }, + is_autoinc => { id => 1 }, + null_cols => [], + is_nullable => {}, + clustered_key => undef, keys => { PRIMARY => { cols => [qw(id)], @@ -473,6 +575,7 @@ is_deeply( type_for => { id => 'bigint' }, name => 'pipo', charset => 'latin1', + is_generated => {}, }, 'NDB table', ); @@ -481,9 +584,11 @@ $tbl = $tp->parse( load_file('t/lib/samples/mixed-case.sql') ); is_deeply( $tbl, { cols => [qw(a b mixedcol)], + non_generated_cols => [qw(a b mixedcol)], col_posn => { a => 0, b => 1, mixedcol => 2 }, is_col => { a => 1, b => 1, mixedcol => 1 }, is_autoinc => { a => 0, b => 0, mixedcol => 0 }, + is_generated => {}, null_cols => [qw(a b mixedcol)], is_nullable => { a => 1, b => 1, mixedcol => 1 }, clustered_key => undef, @@ -518,14 +623,15 @@ is_deeply( $tbl = $tp->parse( load_file('t/lib/samples/one_key.sql') ); is_deeply( $tbl, - { cols => [qw(a b)], - col_posn => { a => 0, b => 1 }, - is_col => { a => 1, b => 1 }, - is_autoinc => { a => 0, b => 0 }, - null_cols => [qw(b)], - is_nullable => { b => 1 }, - clustered_key => undef, - keys => { + { cols => [qw(a b)], + non_generated_cols => [qw(a b)], + col_posn => { a => 0, b => 1 }, + is_col => { a => 1, b => 1 }, + is_autoinc => { a => 0, b => 0 }, + null_cols => [qw(b)], + is_nullable => { b => 1 }, + clustered_key => undef, + keys => { PRIMARY => { colnames => '`a`', cols => [qw(a)], @@ -538,16 +644,17 @@ is_deeply( ddl => 'PRIMARY KEY (`a`)', }, }, - defs => { + defs => { a => ' `a` int(11) NOT NULL', b => ' `b` char(50) default NULL', }, - numeric_cols => [qw(a)], - is_numeric => { a => 1 }, - engine => 'MyISAM', - type_for => { a => 'int', b => 'char' }, - name => 't2', - charset => 'latin1', + numeric_cols => [qw(a)], + is_numeric => { a => 1 }, + is_generated => {}, + engine => 'MyISAM', + type_for => { a => 'int', b => 'char' }, + name => 't2', + charset => 'latin1', }, 'No clustered key on MyISAM table' ); @@ -751,21 +858,23 @@ cmp_ddls('v5.0 vs. v5.1', 't/lib/samples/issue_109-01-v50.sql', 't/lib/samples/i $tbl = $tp->parse( load_file('t/lib/samples/issue_132.sql') ); is_deeply( $tbl, - { cols => [qw(country)], - col_posn => { country => 0 }, - is_col => { country => 1 }, - is_autoinc => { country => 0 }, - null_cols => [qw(country)], - is_nullable => { country => 1 }, - clustered_key => undef, - keys => {}, - defs => { country => " `country` enum('','Cote D`ivoire') default NULL"}, - numeric_cols => [], - is_numeric => {}, - engine => 'MyISAM', - type_for => { country => 'enum' }, - name => 'issue_132', - charset => 'latin1', + { cols => [qw(country)], + non_generated_cols => [qw(country)], + col_posn => { country => 0 }, + is_col => { country => 1 }, + is_autoinc => { country => 0 }, + is_generated => {}, + null_cols => [qw(country)], + is_nullable => { country => 1 }, + clustered_key => undef, + keys => {}, + defs => { country => " `country` enum('','Cote D`ivoire') default NULL"}, + numeric_cols => [], + is_numeric => {}, + engine => 'MyISAM', + type_for => { country => 'enum' }, + name => 'issue_132', + charset => 'latin1', }, 'ENUM col with backtick in value (issue 132)' ); @@ -787,21 +896,23 @@ is( $tbl = $tp->parse( load_file('t/lib/samples/issue_330_backtick_pair_in_col_comments.sql') ); is_deeply( $tbl, - { cols => [qw(a)], - col_posn => { a => 0 }, - is_col => { a => 1 }, - is_autoinc => { a => 0 }, - null_cols => [qw(a)], - is_nullable => { a => 1 }, - clustered_key => undef, - keys => {}, - defs => { a => " `a` int(11) DEFAULT NULL COMMENT 'issue_330 `alex`'" }, - numeric_cols => [qw(a)], - is_numeric => { a => 1 }, - engine => 'MyISAM', - type_for => { a => 'int' }, - name => 'issue_330', - charset => 'latin1', + { cols => [qw(a)], + non_generated_cols => [qw(a)], + col_posn => { a => 0 }, + is_col => { a => 1 }, + is_generated => {}, + is_autoinc => { a => 0 }, + null_cols => [qw(a)], + is_nullable => { a => 1 }, + clustered_key => undef, + keys => {}, + defs => { a => " `a` int(11) DEFAULT NULL COMMENT 'issue_330 `alex`'" }, + numeric_cols => [qw(a)], + is_numeric => { a => 1 }, + engine => 'MyISAM', + type_for => { a => 'int' }, + name => 'issue_330', + charset => 'latin1', }, 'issue with pairing backticks in column comments (issue 330)' ); @@ -810,24 +921,26 @@ is_deeply( $tbl = $tp->parse( load_file('t/lib/samples/issue_pt-193_backtick_in_col_comments.sql') ); is_deeply( $tbl, - { cols => [qw(id f22abcde f23abc)], - col_posn => { id => 0, f22abcde => 1, f23abc => 2 }, - is_col => { id => 1, f22abcde => 1, f23abc => 1 }, - is_autoinc => { id => 1, f22abcde => 0, f23abc => 0 }, - null_cols => [qw(f22abcde)], - is_nullable => { f22abcde => 1}, - clustered_key => undef, - keys => {}, - defs => { id => " `id` int(11) NOT NULL AUTO_INCREMENT", - "f22abcde" => " `f22abcde` int(10) unsigned DEFAULT NULL COMMENT 'xxx`XXx'", - "f23abc" => " `f23abc` int(10) unsigned NOT NULL DEFAULT '255' COMMENT \"`yyy\"" - }, - numeric_cols => [qw(id f22abcde f23abc)], - is_numeric => { id => 1, f22abcde => 1, f23abc => 1 }, - engine => 'InnoDB', - type_for => { id => 'int', f22abcde => 'int', f23abc => 'int' }, - name => 't3', - charset => 'latin1', + { cols => [qw(id f22abcde f23abc)], + non_generated_cols => [qw(id f22abcde f23abc)], + col_posn => { id => 0, f22abcde => 1, f23abc => 2 }, + is_col => { id => 1, f22abcde => 1, f23abc => 1 }, + is_autoinc => { id => 1, f22abcde => 0, f23abc => 0 }, + is_generated => {}, + null_cols => [qw(f22abcde)], + is_nullable => { f22abcde => 1}, + clustered_key => undef, + keys => {}, + defs => { id => " `id` int(11) NOT NULL AUTO_INCREMENT", + "f22abcde" => " `f22abcde` int(10) unsigned DEFAULT NULL COMMENT 'xxx`XXx'", + "f23abc" => " `f23abc` int(10) unsigned NOT NULL DEFAULT '255' COMMENT \"`yyy\"" + }, + numeric_cols => [qw(id f22abcde f23abc)], + is_numeric => { id => 1, f22abcde => 1, f23abc => 1 }, + engine => 'InnoDB', + type_for => { id => 'int', f22abcde => 'int', f23abc => 'int' }, + name => 't3', + charset => 'latin1', }, 'issue with pairing backticks in column comments (issue 330)' ); @@ -874,12 +987,14 @@ is_deeply( clustered_key => undef, col_posn => { 'first, last' => 1, id => 0 }, cols => [ 'id', 'first, last' ], + non_generated_cols => [ 'id', 'first, last' ], defs => { 'first, last' => ' `first, last` varchar(32) default NULL', id => ' `id` int(11) NOT NULL auto_increment', }, engine => 'MyISAM', is_autoinc => { 'first, last' => 0, id => 1 }, + is_generated => {}, is_col => { 'first, last' => 1, id => 1 }, is_nullable => { 'first, last' => 1 }, is_numeric => { id => 1 }, @@ -1000,22 +1115,24 @@ $tbl = $tp->parse(load_file('t/lib/samples/triple-quoted-col.sql')); is_deeply( $tbl, { - clustered_key => undef, - col_posn => { 'foo' => 0, bar => 1 }, - cols => [ 'foo', 'bar' ], - defs => { + clustered_key => undef, + col_posn => { 'foo' => 0, bar => 1 }, + is_generated => {}, + cols => [ 'foo', 'bar' ], + non_generated_cols => [ 'foo', 'bar' ], + defs => { 'foo' => ' `foo` int(11) DEFAULT NULL', 'bar' => ' ```bar``` int(11) DEFAULT NULL', }, - engine => 'InnoDB', - is_autoinc => { foo => 0, bar => 0 }, - is_col => { foo => 1, bar => 1 }, - is_nullable => { foo => 1, bar => 1 }, - is_numeric => { foo => 1, bar => 1 }, - name => 't', - null_cols => [ 'foo', 'bar' ], - numeric_cols => [ 'foo', 'bar' ], - type_for => { + engine => 'InnoDB', + is_autoinc => { foo => 0, bar => 0 }, + is_col => { foo => 1, bar => 1 }, + is_nullable => { foo => 1, bar => 1 }, + is_numeric => { foo => 1, bar => 1 }, + name => 't', + null_cols => [ 'foo', 'bar' ], + numeric_cols => [ 'foo', 'bar' ], + type_for => { foo => 'int', bar => 'int', }, @@ -1025,6 +1142,51 @@ is_deeply( 'Literal backticks (bug 1462904)' ); +SKIP: { + skip "generated column tests require MySQL 5.7+", 1 + if $sandbox_version lt '5.7'; + $tbl = $tp->parse( load_file('t/lib/samples/generated_cols.sql') ); + is_deeply( + $tbl, + { + charset => 'utf8mb4', + clustered_key => 'PRIMARY', + col_posn => { column2 => 1, column3 => 2, id => 0 }, + cols => [ 'id', 'column2', 'column3' ], + non_generated_cols => [ 'id', 'column2' ], + defs => { + column2 => ' `column2` int(11) DEFAULT NULL', + column3 => ' `column3` int(11) GENERATED ALWAYS AS ((`column2` + 1)) STORED', + id => ' `id` int(11) NOT NULL' + }, + engine => 'InnoDB', + is_autoinc => { column2 => 0, column3 => 0, id => 0 }, + is_col => { column2 => 1, column3 => 1, id => 1 }, + is_generated => { column3 => 1 }, + is_nullable => { column2 => 1, column3 => 1 }, + is_numeric => { column2 => 1, column3 => 1, id => 1 }, + keys => { + PRIMARY => { + col_prefixes => [ undef ], + colnames => '`id`', + cols => [ 'id' ], + ddl => 'PRIMARY KEY (`id`)', + is_col => { id => 1 }, + is_nullable => 0, + is_unique => 1, + name => 'PRIMARY', + type => 'BTREE' + } + }, + name => 't1', + null_cols => [ 'column2', 'column3' ], + numeric_cols => [ 'id', 'column2', 'column3' ], + type_for => { column2 => 'int', column3 => 'int', id => 'int' } + }, + 'Generated columns is OK', + ) or die Data::Dumper::Dumper($tbl); +} + # ############################################################################# # Done. # ############################################################################# diff --git a/t/lib/samples/generated_cols.sql b/t/lib/samples/generated_cols.sql new file mode 100644 index 00000000..d51e964b --- /dev/null +++ b/t/lib/samples/generated_cols.sql @@ -0,0 +1,6 @@ +CREATE TABLE `t1` ( + `ID` int(11) NOT NULL, + `Column2` int(11) DEFAULT NULL, + `Column3` int(11) GENERATED ALWAYS AS ((`Column2` + 1)) STORED, + PRIMARY KEY (`ID`) +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci diff --git a/t/pt-online-schema-change/pt-202.t b/t/pt-online-schema-change/pt-202.t new file mode 100644 index 00000000..d02643fd --- /dev/null +++ b/t/pt-online-schema-change/pt-202.t @@ -0,0 +1,76 @@ +#!/usr/bin/env perl + +BEGIN { + die "The PERCONA_TOOLKIT_BRANCH environment variable is not set.\n" + unless $ENV{PERCONA_TOOLKIT_BRANCH} && -d $ENV{PERCONA_TOOLKIT_BRANCH}; + unshift @INC, "$ENV{PERCONA_TOOLKIT_BRANCH}/lib"; +}; + +use strict; +use warnings FATAL => 'all'; +use threads; + +use English qw(-no_match_vars); +use Test::More; + +use Data::Dumper; +use PerconaTest; +use Sandbox; +use SqlModes; +use File::Temp qw/ tempdir /; + +require "$trunk/bin/pt-online-schema-change"; + +my $dp = new DSNParser(opts=>$dsn_opts); +my $sb = new Sandbox(basedir => '/tmp', DSNParser => $dp); +my $master_dbh = $sb->get_dbh_for('master'); +my $master_dsn = 'h=127.1,P=12345,u=msandbox,p=msandbox'; + +if ( !$master_dbh ) { + plan skip_all => 'Cannot connect to sandbox master'; +} + +if ($sandbox_version lt '5.7') { + plan skip_all => "generated column tests require MySQL 5.7+"; +} + +plan tests => 3; + +# The sandbox servers run with lock_wait_timeout=3 and it's not dynamic +# so we need to specify --set-vars innodb_lock_wait_timeout=3 else the +# tool will die. +my @args = (qw(--set-vars innodb_lock_wait_timeout=3)); +my $output; +my $exit_status; +my $sample = "t/pt-online-schema-change/samples/"; + +$sb->load_file('master', "$sample/pt-202.sql"); + +($output, $exit_status) = full_output( + sub { pt_online_schema_change::main(@args, "$master_dsn,D=test,t=t1", + '--execute', + '--alter', "ADD COLUMN `Column4` VARCHAR(45) NULL AFTER `Column3`", + ), + }, +); + +is( + $exit_status, + 0, + "PT-202 Altering table having generated columns exit status 0", +); + +like( + $output, + qr/Successfully altered `test`.`t1`/s, + "PT-202 Altering table having generated columns success", +); + +$master_dbh->do("DROP DATABASE IF EXISTS test"); + +# ############################################################################# +# Done. +# ############################################################################# +$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/samples/pt-202.sql b/t/pt-online-schema-change/samples/pt-202.sql new file mode 100644 index 00000000..721780d2 --- /dev/null +++ b/t/pt-online-schema-change/samples/pt-202.sql @@ -0,0 +1,9 @@ +CREATE SCHEMA IF NOT EXISTS test; +USE test; +DROP TABLE IF EXISTS t1; +CREATE TABLE `test`.`t1` ( +`ID` int(11) NOT NULL, +`Column2` int(11) DEFAULT NULL, +`Column3` int(11) GENERATED ALWAYS AS ((`Column2` + 1)) STORED, +PRIMARY KEY (`ID`) +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci; From 228d87e108e76328a7571bfcfd627b1db6ff6590 Mon Sep 17 00:00:00 2001 From: Kamil Dziedzic Date: Thu, 5 Oct 2017 21:36:18 +0200 Subject: [PATCH 13/13] Fix expecting iter.Close() Adding prof.Stop() ensures iterator gets closed --- src/go/mongolib/profiler/profiler_test.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/go/mongolib/profiler/profiler_test.go b/src/go/mongolib/profiler/profiler_test.go index c3289396..2e112ae9 100644 --- a/src/go/mongolib/profiler/profiler_test.go +++ b/src/go/mongolib/profiler/profiler_test.go @@ -89,6 +89,7 @@ func TestRegularIterator(t *testing.T) { }, } prof.Start() + defer prof.Stop() select { case queries := <-prof.QueriesChan(): if !reflect.DeepEqual(queries, want) { @@ -150,6 +151,7 @@ func TestIteratorTimeout(t *testing.T) { } prof.Start() + defer prof.Stop() gotTimeout := false // Get a timeout @@ -174,8 +176,6 @@ func TestIteratorTimeout(t *testing.T) { case <-time.After(2 * time.Second): t.Error("Didn't get any query after 2 seconds") } - - prof.Stop() } func TestTailIterator(t *testing.T) { @@ -251,6 +251,7 @@ func TestTailIterator(t *testing.T) { }, } prof.Start() + defer prof.Stop() index := 0 // Since the mocked iterator has a Sleep(1500 ms) between Next methods calls, // we are going to have two ticker ticks and on every tick it will return one document. @@ -300,6 +301,7 @@ func TestCalcStats(t *testing.T) { prof := NewProfiler(iter, filters, nil, s) prof.Start() + defer prof.Stop() select { case queries := <-prof.QueriesChan(): s := queries.CalcQueriesStats(1) @@ -349,6 +351,7 @@ func TestCalcTotalStats(t *testing.T) { prof := NewProfiler(iter, filters, nil, s) prof.Start() + defer prof.Stop() select { case queries := <-prof.QueriesChan(): s := queries.CalcTotalQueriesStats(1)