From 5edbd992e599a4956fa51946e705e99df7d82bde Mon Sep 17 00:00:00 2001 From: Rohit Nayak Date: Wed, 23 Sep 2020 00:18:08 +0200 Subject: [PATCH 1/5] Send an OTHER if a SET is found in the binlog Signed-off-by: Rohit Nayak --- .../tabletserver/vstreamer/vstreamer.go | 5 +- .../tabletserver/vstreamer/vstreamer_test.go | 47 +++++++++++++++++++ 2 files changed, 50 insertions(+), 2 deletions(-) diff --git a/go/vt/vttablet/tabletserver/vstreamer/vstreamer.go b/go/vt/vttablet/tabletserver/vstreamer/vstreamer.go index 58c29c3fd05..2c9f578f9fd 100644 --- a/go/vt/vttablet/tabletserver/vstreamer/vstreamer.go +++ b/go/vt/vttablet/tabletserver/vstreamer/vstreamer.go @@ -316,7 +316,6 @@ func (vs *vstreamer) parseEvents(ctx context.Context, events <-chan mysql.Binlog // parseEvent parses an event from the binlog and converts it to a list of VEvents. func (vs *vstreamer) parseEvent(ev mysql.BinlogEvent) ([]*binlogdatapb.VEvent, error) { - if !ev.IsValid() { return nil, fmt.Errorf("can't parse binlog event: invalid data: %#v", ev) } @@ -383,6 +382,7 @@ func (vs *vstreamer) parseEvent(ev mysql.BinlogEvent) ([]*binlogdatapb.VEvent, e } // Insert/Delete/Update are supported only to be used in the context of external mysql streams where source databases // could be using SBR. Vitess itself will never run into cases where it needs to consume non rbr statements. + switch cat := sqlparser.Preview(q.SQL); cat { case sqlparser.StmtInsert: mustSend := mustSendStmt(q, params.DbName) @@ -455,7 +455,7 @@ func (vs *vstreamer) parseEvent(ev mysql.BinlogEvent) ([]*binlogdatapb.VEvent, e Statement: q.SQL, }) } - case sqlparser.StmtOther, sqlparser.StmtPriv: + case sqlparser.StmtOther, sqlparser.StmtPriv, sqlparser.StmtSet: // These are either: // 1) DBA statements like REPAIR that can be ignored. // 2) Privilege-altering statements like GRANT/REVOKE @@ -535,6 +535,7 @@ func (vs *vstreamer) parseEvent(ev mysql.BinlogEvent) ([]*binlogdatapb.VEvent, e if err != nil { return nil, err } + } for _, vevent := range vevents { vevent.Timestamp = int64(ev.Timestamp()) diff --git a/go/vt/vttablet/tabletserver/vstreamer/vstreamer_test.go b/go/vt/vttablet/tabletserver/vstreamer/vstreamer_test.go index d10e757a7cc..3831cd6bd45 100644 --- a/go/vt/vttablet/tabletserver/vstreamer/vstreamer_test.go +++ b/go/vt/vttablet/tabletserver/vstreamer/vstreamer_test.go @@ -43,6 +43,45 @@ type testcase struct { output [][]string } +func TestSetStatement(t *testing.T) { + if testing.Short() { + t.Skip() + } + + execStatements(t, []string{ + "create table t1(id int, val varbinary(128), primary key(id))", + }) + defer execStatements(t, []string{ + "drop table t1", + }) + engine.se.Reload(context.Background()) + testcases := []testcase{{ + input: []string{ + "begin", + "insert into t1 values (1, 'aaa')", + "commit", + "set global log_builtin_as_identified_by_password=1", // without this the set password is converted to an alter user + "SET PASSWORD FOR 'vt_appdebug'@'localhost'='abc123'", + "set global log_builtin_as_identified_by_password=0", + "SET PASSWORD FOR 'vt_appdebug'@'localhost'='abc124'", + }, + output: [][]string{{ + `begin`, + `type:FIELD field_event: fields: > `, + `type:ROW row_event: > > `, + `gtid`, + `commit`, + }, { + `gtid`, + `other`, + }, { + `gtid`, + `ddl`, + }}, + }} + runCases(t, nil, testcases, "current", nil) +} + func TestVersion(t *testing.T) { if testing.Short() { t.Skip() @@ -1649,6 +1688,14 @@ func expectLog(ctx context.Context, t *testing.T, input interface{}, ch <-chan [ if evs[i].Type != binlogdatapb.VEventType_COMMIT { t.Fatalf("%v (%d): event: %v, want commit", input, i, evs[i]) } + case "other": + if evs[i].Type != binlogdatapb.VEventType_OTHER { + t.Fatalf("%v (%d): event: %v, want other", input, i, evs[i]) + } + case "ddl": + if evs[i].Type != binlogdatapb.VEventType_DDL { + t.Fatalf("%v (%d): event: %v, want ddl", input, i, evs[i]) + } default: evs[i].Timestamp = 0 if evs[i].Type == binlogdatapb.VEventType_FIELD { From caed1552c9db2e6f5388a079b35c110f8df7a1dc Mon Sep 17 00:00:00 2001 From: Rohit Nayak Date: Wed, 23 Sep 2020 09:21:31 +0200 Subject: [PATCH 2/5] Use hash pwd for MariaDB Signed-off-by: Rohit Nayak --- go/vt/vttablet/tabletserver/vstreamer/vstreamer_test.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/go/vt/vttablet/tabletserver/vstreamer/vstreamer_test.go b/go/vt/vttablet/tabletserver/vstreamer/vstreamer_test.go index 3831cd6bd45..84dddb959d3 100644 --- a/go/vt/vttablet/tabletserver/vstreamer/vstreamer_test.go +++ b/go/vt/vttablet/tabletserver/vstreamer/vstreamer_test.go @@ -61,9 +61,7 @@ func TestSetStatement(t *testing.T) { "insert into t1 values (1, 'aaa')", "commit", "set global log_builtin_as_identified_by_password=1", // without this the set password is converted to an alter user - "SET PASSWORD FOR 'vt_appdebug'@'localhost'='abc123'", - "set global log_builtin_as_identified_by_password=0", - "SET PASSWORD FOR 'vt_appdebug'@'localhost'='abc124'", + "SET PASSWORD FOR 'vt_appdebug'@'localhost'='*CDE65254CC57BC0C3D0A85509B5CEA654126BF56'", }, output: [][]string{{ `begin`, @@ -74,9 +72,6 @@ func TestSetStatement(t *testing.T) { }, { `gtid`, `other`, - }, { - `gtid`, - `ddl`, }}, }} runCases(t, nil, testcases, "current", nil) From 4b00bbb927b3f823c0817dc491ad0b7ba5b59ce1 Mon Sep 17 00:00:00 2001 From: Rohit Nayak Date: Wed, 23 Sep 2020 12:00:30 +0200 Subject: [PATCH 3/5] Fix tests for MariaDB Signed-off-by: Rohit Nayak --- .../tabletserver/vstreamer/testenv/testenv.go | 4 +++- .../tabletserver/vstreamer/vstreamer_test.go | 17 ++++++++++------- go/vt/vttest/environment.go | 10 ++++++++++ 3 files changed, 23 insertions(+), 8 deletions(-) diff --git a/go/vt/vttablet/tabletserver/vstreamer/testenv/testenv.go b/go/vt/vttablet/tabletserver/vstreamer/testenv/testenv.go index 2336641132d..905e4fc481b 100644 --- a/go/vt/vttablet/tabletserver/vstreamer/testenv/testenv.go +++ b/go/vt/vttablet/tabletserver/vstreamer/testenv/testenv.go @@ -51,6 +51,8 @@ type Env struct { Dbcfgs *dbconfigs.DBConfigs Mysqld *mysqlctl.Mysqld SchemaEngine *schema.Engine + + Flavor string } // Init initializes an Env. @@ -94,7 +96,7 @@ func Init() (*Env, error) { os.RemoveAll(te.cluster.Config.SchemaDir) return nil, fmt.Errorf("could not launch mysql: %v", err) } - + te.Flavor = te.cluster.Env.Flavor() te.Dbcfgs = dbconfigs.NewTestDBConfigs(te.cluster.MySQLConnParams(), te.cluster.MySQLAppDebugConnParams(), te.cluster.DbName()) config := tabletenv.NewDefaultConfig() config.DB = te.Dbcfgs diff --git a/go/vt/vttablet/tabletserver/vstreamer/vstreamer_test.go b/go/vt/vttablet/tabletserver/vstreamer/vstreamer_test.go index 84dddb959d3..a1b00338803 100644 --- a/go/vt/vttablet/tabletserver/vstreamer/vstreamer_test.go +++ b/go/vt/vttablet/tabletserver/vstreamer/vstreamer_test.go @@ -55,14 +55,17 @@ func TestSetStatement(t *testing.T) { "drop table t1", }) engine.se.Reload(context.Background()) + queries := []string{ + "begin", + "insert into t1 values (1, 'aaa')", + "commit", + } + if !strings.Contains(strings.ToLower(env.Flavor), "mariadb") { + queries = append(queries, "set global log_builtin_as_identified_by_password=1") + } + queries = append(queries, "SET PASSWORD FOR 'vt_appdebug'@'localhost'='*CDE65254CC57BC0C3D0A85509B5CEA654126BF56'") testcases := []testcase{{ - input: []string{ - "begin", - "insert into t1 values (1, 'aaa')", - "commit", - "set global log_builtin_as_identified_by_password=1", // without this the set password is converted to an alter user - "SET PASSWORD FOR 'vt_appdebug'@'localhost'='*CDE65254CC57BC0C3D0A85509B5CEA654126BF56'", - }, + input: queries, output: [][]string{{ `begin`, `type:FIELD field_event: fields: > `, diff --git a/go/vt/vttest/environment.go b/go/vt/vttest/environment.go index 130791bce4b..2953c983f7f 100644 --- a/go/vt/vttest/environment.go +++ b/go/vt/vttest/environment.go @@ -85,6 +85,9 @@ type Environment interface { // any temporary data in the environment. Environments that can // last through several test runs do not need to implement it. TearDown() error + + // Flavor is set to the MySQL Server flavor used for testing + Flavor() string } // LocalTestEnv is an Environment implementation for local testing @@ -94,6 +97,7 @@ type LocalTestEnv struct { TmpPath string DefaultMyCnf []string Env []string + MySQLFlavor string } // DefaultMySQLFlavor is the MySQL flavor used by vttest when MYSQL_FLAVOR is not @@ -202,6 +206,11 @@ func (env *LocalTestEnv) TearDown() error { return os.RemoveAll(env.TmpPath) } +// Flavor implements Flavor for LocalTestEnv. +func (env *LocalTestEnv) Flavor() string { + return env.MySQLFlavor +} + func tmpdir(dataroot string) (dir string, err error) { dir, err = ioutil.TempDir(dataroot, "vttest") return @@ -257,6 +266,7 @@ func NewLocalTestEnvWithDirectory(flavor string, basePort int, directory string) BasePort: basePort, TmpPath: directory, DefaultMyCnf: mycnf, + MySQLFlavor: flavor, Env: []string{ fmt.Sprintf("VTDATAROOT=%s", directory), fmt.Sprintf("MYSQL_FLAVOR=%s", flavor), From abeb25091c6ba78ad17762e0f67f64dfdfd65c55 Mon Sep 17 00:00:00 2001 From: Rohit Nayak Date: Wed, 23 Sep 2020 15:45:45 +0200 Subject: [PATCH 4/5] Detect if MariaDB using query Signed-off-by: Rohit Nayak --- .../tabletserver/vstreamer/testenv/testenv.go | 3 --- .../tabletserver/vstreamer/vstreamer_test.go | 18 +++++++++++++++++- go/vt/vttest/environment.go | 10 ---------- 3 files changed, 17 insertions(+), 14 deletions(-) diff --git a/go/vt/vttablet/tabletserver/vstreamer/testenv/testenv.go b/go/vt/vttablet/tabletserver/vstreamer/testenv/testenv.go index 905e4fc481b..50032ba4e37 100644 --- a/go/vt/vttablet/tabletserver/vstreamer/testenv/testenv.go +++ b/go/vt/vttablet/tabletserver/vstreamer/testenv/testenv.go @@ -51,8 +51,6 @@ type Env struct { Dbcfgs *dbconfigs.DBConfigs Mysqld *mysqlctl.Mysqld SchemaEngine *schema.Engine - - Flavor string } // Init initializes an Env. @@ -96,7 +94,6 @@ func Init() (*Env, error) { os.RemoveAll(te.cluster.Config.SchemaDir) return nil, fmt.Errorf("could not launch mysql: %v", err) } - te.Flavor = te.cluster.Env.Flavor() te.Dbcfgs = dbconfigs.NewTestDBConfigs(te.cluster.MySQLConnParams(), te.cluster.MySQLAppDebugConnParams(), te.cluster.DbName()) config := tabletenv.NewDefaultConfig() config.DB = te.Dbcfgs diff --git a/go/vt/vttablet/tabletserver/vstreamer/vstreamer_test.go b/go/vt/vttablet/tabletserver/vstreamer/vstreamer_test.go index a1b00338803..f47cd2bd8de 100644 --- a/go/vt/vttablet/tabletserver/vstreamer/vstreamer_test.go +++ b/go/vt/vttablet/tabletserver/vstreamer/vstreamer_test.go @@ -43,7 +43,23 @@ type testcase struct { output [][]string } +func isMariaDB(t *testing.T) bool { + qr, err := env.Mysqld.FetchSuperQuery(context.Background(), "select version()") + require.NoError(t, err) + require.NotNil(t, qr) + require.NotNil(t, qr.Rows) + require.NotEqual(t, len(qr.Rows), 0) + version := qr.Rows[0][0].String() + if strings.Contains(strings.ToLower(version), "mariadb") { + log.Infof("Flavor IS MariaDB\n") + return true + } + log.Infof("Flavor is NOT MariaDB\n") + return false +} + func TestSetStatement(t *testing.T) { + if testing.Short() { t.Skip() } @@ -60,7 +76,7 @@ func TestSetStatement(t *testing.T) { "insert into t1 values (1, 'aaa')", "commit", } - if !strings.Contains(strings.ToLower(env.Flavor), "mariadb") { + if !isMariaDB(t) { queries = append(queries, "set global log_builtin_as_identified_by_password=1") } queries = append(queries, "SET PASSWORD FOR 'vt_appdebug'@'localhost'='*CDE65254CC57BC0C3D0A85509B5CEA654126BF56'") diff --git a/go/vt/vttest/environment.go b/go/vt/vttest/environment.go index 2953c983f7f..130791bce4b 100644 --- a/go/vt/vttest/environment.go +++ b/go/vt/vttest/environment.go @@ -85,9 +85,6 @@ type Environment interface { // any temporary data in the environment. Environments that can // last through several test runs do not need to implement it. TearDown() error - - // Flavor is set to the MySQL Server flavor used for testing - Flavor() string } // LocalTestEnv is an Environment implementation for local testing @@ -97,7 +94,6 @@ type LocalTestEnv struct { TmpPath string DefaultMyCnf []string Env []string - MySQLFlavor string } // DefaultMySQLFlavor is the MySQL flavor used by vttest when MYSQL_FLAVOR is not @@ -206,11 +202,6 @@ func (env *LocalTestEnv) TearDown() error { return os.RemoveAll(env.TmpPath) } -// Flavor implements Flavor for LocalTestEnv. -func (env *LocalTestEnv) Flavor() string { - return env.MySQLFlavor -} - func tmpdir(dataroot string) (dir string, err error) { dir, err = ioutil.TempDir(dataroot, "vttest") return @@ -266,7 +257,6 @@ func NewLocalTestEnvWithDirectory(flavor string, basePort int, directory string) BasePort: basePort, TmpPath: directory, DefaultMyCnf: mycnf, - MySQLFlavor: flavor, Env: []string{ fmt.Sprintf("VTDATAROOT=%s", directory), fmt.Sprintf("MYSQL_FLAVOR=%s", flavor), From 99c1bd9d280b628dc166604201d9b93e3cf95061 Mon Sep 17 00:00:00 2001 From: Rohit Nayak Date: Wed, 23 Sep 2020 19:13:13 +0200 Subject: [PATCH 5/5] Only test SetStatement on limited flavors Signed-off-by: Rohit Nayak --- .../tabletserver/vstreamer/vstreamer_test.go | 22 +++++++++---------- 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/go/vt/vttablet/tabletserver/vstreamer/vstreamer_test.go b/go/vt/vttablet/tabletserver/vstreamer/vstreamer_test.go index f47cd2bd8de..e2675de7814 100644 --- a/go/vt/vttablet/tabletserver/vstreamer/vstreamer_test.go +++ b/go/vt/vttablet/tabletserver/vstreamer/vstreamer_test.go @@ -43,18 +43,13 @@ type testcase struct { output [][]string } -func isMariaDB(t *testing.T) bool { - qr, err := env.Mysqld.FetchSuperQuery(context.Background(), "select version()") +func checkIfOptionIsSupported(t *testing.T, variable string) bool { + qr, err := env.Mysqld.FetchSuperQuery(context.Background(), fmt.Sprintf("show variables like '%s'", variable)) require.NoError(t, err) require.NotNil(t, qr) - require.NotNil(t, qr.Rows) - require.NotEqual(t, len(qr.Rows), 0) - version := qr.Rows[0][0].String() - if strings.Contains(strings.ToLower(version), "mariadb") { - log.Infof("Flavor IS MariaDB\n") + if qr.Rows != nil && len(qr.Rows) == 1 { return true } - log.Infof("Flavor is NOT MariaDB\n") return false } @@ -63,6 +58,11 @@ func TestSetStatement(t *testing.T) { if testing.Short() { t.Skip() } + if !checkIfOptionIsSupported(t, "log_builtin_as_identified_by_password") { + // the combination of setting this option and support for "set password" only works on a few flavors + log.Info("Cannot test SetStatement on this flavor") + return + } execStatements(t, []string{ "create table t1(id int, val varbinary(128), primary key(id))", @@ -75,11 +75,9 @@ func TestSetStatement(t *testing.T) { "begin", "insert into t1 values (1, 'aaa')", "commit", + "set global log_builtin_as_identified_by_password=1", + "SET PASSWORD FOR 'vt_appdebug'@'localhost'='*AA17DA66C7C714557F5485E84BCAFF2C209F2F53'", //select password('vtappdebug_password'); } - if !isMariaDB(t) { - queries = append(queries, "set global log_builtin_as_identified_by_password=1") - } - queries = append(queries, "SET PASSWORD FOR 'vt_appdebug'@'localhost'='*CDE65254CC57BC0C3D0A85509B5CEA654126BF56'") testcases := []testcase{{ input: queries, output: [][]string{{