From 92083793acc46a2a125d6787c30c144518fc51cd Mon Sep 17 00:00:00 2001 From: Sugu Sougoumarane Date: Sun, 22 Dec 2019 12:19:02 -0800 Subject: [PATCH 1/4] tests: fix various tests Fixes #5532 Fixes #5569 Fixes #5571 With this fix, unit tests pass for all flavors. Also fix test.go to cover the newer flavors. Signed-off-by: Sugu Sougoumarane --- go.mod | 5 +- go.sum | 4 + go/mysql/client.go | 2 +- go/mysql/flavor_filepos.go | 10 +- .../{server_flaky_test.go => server_test.go} | 58 +++++------ .../vreplication/vstreamer_client_test.go | 4 + ...reamer_flaky_test.go => vstreamer_test.go} | 96 ++++++++++++------- test.go | 2 +- 8 files changed, 114 insertions(+), 67 deletions(-) rename go/mysql/{server_flaky_test.go => server_test.go} (97%) rename go/vt/vttablet/tabletserver/vstreamer/{vstreamer_flaky_test.go => vstreamer_test.go} (95%) diff --git a/go.mod b/go.mod index dcd3d9c7509..9ff92cdcb8e 100644 --- a/go.mod +++ b/go.mod @@ -25,7 +25,6 @@ require ( github.com/golang/mock v1.3.1 github.com/golang/protobuf v1.3.2 github.com/golang/snappy v0.0.0-20170215233205-553a64147049 - github.com/google/btree v1.0.0 // indirect github.com/google/shlex v0.0.0-20181106134648-c34317bd91bf // indirect github.com/gorilla/websocket v0.0.0-20160912153041-2d1e4548da23 github.com/grpc-ecosystem/go-grpc-middleware v1.1.0 @@ -50,8 +49,6 @@ require ( github.com/minio/minio-go v0.0.0-20190131015406-c8a261de75c1 github.com/mitchellh/go-testing-interface v1.0.0 // indirect github.com/mitchellh/mapstructure v1.1.2 // indirect - github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect - github.com/modern-go/reflect2 v1.0.1 // indirect github.com/olekukonko/tablewriter v0.0.0-20160115111002-cca8bbc07984 github.com/opentracing-contrib/go-grpc v0.0.0-20180928155321-4b5a12d3ff02 github.com/opentracing/opentracing-go v1.1.0 @@ -60,7 +57,7 @@ require ( github.com/pkg/errors v0.8.1 github.com/prometheus/client_golang v1.1.0 github.com/prometheus/client_model v0.0.0-20190812154241-14fe0d1b01d4 // indirect - github.com/prometheus/common v0.7.0 // indirect + github.com/prometheus/common v0.7.0 github.com/prometheus/procfs v0.0.5 // indirect github.com/satori/go.uuid v0.0.0-20160713180306-0aa62d5ddceb // indirect github.com/stretchr/testify v1.4.0 diff --git a/go.sum b/go.sum index 736f4211d44..c8c2d3a4f08 100644 --- a/go.sum +++ b/go.sum @@ -14,8 +14,10 @@ github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03 github.com/BurntSushi/xgb v0.0.0-20160522181843-27f122750802/go.mod h1:IVnqGOEym/WlBOVXweHU+Q+/VP0lqqI8lqeDx9IjBqo= github.com/DataDog/datadog-go v2.2.0+incompatible/go.mod h1:LButxg5PwREeZtORoXG3tL4fMGNddJ+vMq1mwgfaqoQ= github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc= +github.com/alecthomas/template v0.0.0-20190718012654-fb15b899a751 h1:JYp7IbQjafoB+tBA3gMyHYHrpOtNuDiK/uB5uXxq5wM= github.com/alecthomas/template v0.0.0-20190718012654-fb15b899a751/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc= github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0= +github.com/alecthomas/units v0.0.0-20190717042225-c3de453c63f4 h1:Hs82Z41s6SdL1CELW+XaDYmOH4hkBN4/N9og/AsOv7E= github.com/alecthomas/units v0.0.0-20190717042225-c3de453c63f4/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0= github.com/armon/go-metrics v0.0.0-20180917152333-f0300d1749da/go.mod h1:Q73ZrmVTwzkszR9V5SSuryQ31EELlFMUz1kKyl939pY= github.com/armon/go-metrics v0.0.0-20190430140413-ec5e00d3c878 h1:EFSB7Zo9Eg91v7MJPVsifUysc/wPdN+NOnVe6bWbdBM= @@ -232,6 +234,7 @@ github.com/satori/go.uuid v0.0.0-20160713180306-0aa62d5ddceb/go.mod h1:dA0hQrYB0 github.com/sean-/seed v0.0.0-20170313163322-e2103e2c3529 h1:nn5Wsu0esKSJiIVhscUtVbo7ada43DJhG55ua/hjS5I= github.com/sean-/seed v0.0.0-20170313163322-e2103e2c3529/go.mod h1:DxrIzT+xaE7yg65j358z/aeFdxmN0P9QXhEzd20vsDc= github.com/sirupsen/logrus v1.2.0/go.mod h1:LxeOpSwHxABJmUn/MG1IvRgCAasNZTLOkJPxbbu5VWo= +github.com/sirupsen/logrus v1.4.2 h1:SPIRibHv4MatM3XXNO2BJeFLZwZ2LvZgfQ5+UNI2im4= github.com/sirupsen/logrus v1.4.2/go.mod h1:tLMulIdttU9McNUspp0xgXVQah82FyeX6MwdIuYE2rE= github.com/smartystreets/assertions v0.0.0-20190116191733-b6c0e53d7304 h1:Jpy1PXuP99tXNrhbq2BaPz9B+jNAvH1JPQQpG/9GCXY= github.com/smartystreets/assertions v0.0.0-20190116191733-b6c0e53d7304/go.mod h1:OnSkiWE9lh6wB0YB77sQom3nweQdgAjqCqsofrRNTgc= @@ -378,6 +381,7 @@ google.golang.org/grpc v1.21.1 h1:j6XxA85m/6txkUCHvzlV5f+HBNl/1r5cZ2A/3IEFOO8= google.golang.org/grpc v1.21.1/go.mod h1:oYelfM1adQP15Ek0mdvEgi9Df8B9CZIaU1084ijfRaM= google.golang.org/grpc v1.24.0 h1:vb/1TCsVn3DcJlQ0Gs1yB1pKI6Do2/QNwxdKqmc/b0s= google.golang.org/grpc v1.24.0/go.mod h1:XDChyiUovWa60DnaeDeZmSW86xtLtjtZbwvSiRnRtcA= +gopkg.in/alecthomas/kingpin.v2 v2.2.6 h1:jMFz6MfLP0/4fUyZle81rXUoxOBFi19VUFKVDOQfozc= gopkg.in/alecthomas/kingpin.v2 v2.2.6/go.mod h1:FMv+mEhP44yOT+4EoQTLFTRgOQ1FBLkstjWtayDeSgw= gopkg.in/asn1-ber.v1 v1.0.0-20150924051756-4e86f4367175 h1:nn6Zav2sOQHCFJHEspya8KqxhFwKci30UxHy3HXPTyQ= gopkg.in/asn1-ber.v1 v1.0.0-20150924051756-4e86f4367175/go.mod h1:cuepJuh7vyXfUyUwEgHQXw849cJrilpS5NeIjOWESAw= diff --git a/go/mysql/client.go b/go/mysql/client.go index d679de6d733..1c30044197d 100644 --- a/go/mysql/client.go +++ b/go/mysql/client.go @@ -380,7 +380,7 @@ func (c *Conn) parseInitialHandshakePacket(data []byte) (uint32, []byte, error) errorCode, pos, _ := readUint16(data, pos) // Normally there would be a 1-byte sql_state_marker field and a 5-byte // sql_state field here, but docs say these will not be present in this case. - errorMsg, pos, _ := readEOFString(data, pos) + errorMsg, _, _ := readEOFString(data, pos) return 0, nil, NewSQLError(CRServerHandshakeErr, SSUnknownSQLState, "immediate error from server errorCode=%v errorMsg=%v", errorCode, errorMsg) } diff --git a/go/mysql/flavor_filepos.go b/go/mysql/flavor_filepos.go index fe1f0e54a83..d457efa92f4 100644 --- a/go/mysql/flavor_filepos.go +++ b/go/mysql/flavor_filepos.go @@ -21,6 +21,7 @@ import ( "fmt" "io" "strconv" + "strings" "time" "golang.org/x/net/context" @@ -143,12 +144,19 @@ func (flv *filePosFlavor) readBinlogEvent(c *Conn) (BinlogEvent, error) { // No need to transmit. Just update the internal position for the next event. continue } - case eXIDEvent, eQueryEvent, eTableMapEvent, + case eXIDEvent, eTableMapEvent, eWriteRowsEventV0, eWriteRowsEventV1, eWriteRowsEventV2, eDeleteRowsEventV0, eDeleteRowsEventV1, eDeleteRowsEventV2, eUpdateRowsEventV0, eUpdateRowsEventV1, eUpdateRowsEventV2: flv.savedEvent = event return newFilePosGTIDEvent(flv.file, event.nextPosition(flv.format), event.Timestamp()), nil + case eQueryEvent: + q, err := event.Query(flv.format) + if err == nil && strings.HasPrefix(q.SQL, "#") { + continue + } + flv.savedEvent = event + return newFilePosGTIDEvent(flv.file, event.nextPosition(flv.format), event.Timestamp()), nil default: // For unrecognized events, send a fake "repair" event so that // the position gets transmitted. diff --git a/go/mysql/server_flaky_test.go b/go/mysql/server_test.go similarity index 97% rename from go/mysql/server_flaky_test.go rename to go/mysql/server_test.go index 29de345971c..53f6cde7b54 100644 --- a/go/mysql/server_flaky_test.go +++ b/go/mysql/server_test.go @@ -719,11 +719,6 @@ func TestServer(t *testing.T) { // TestClearTextServer creates a Server that needs clear text // passwords from the client. func TestClearTextServer(t *testing.T) { - // If the database we're using is MariaDB, the client - // is also the MariaDB client, that does support - // clear text by default. - isMariaDB := os.Getenv("MYSQL_FLAVOR") == "MariaDB" - th := &testHandler{} authServer := NewAuthServerStatic() @@ -741,6 +736,9 @@ func TestClearTextServer(t *testing.T) { host, port := getHostPort(t, l.Addr()) + version, _ := runMysql(t, nil, "--version") + isMariaDB := strings.Contains(version, "MariaDB") + // Setup the right parameters. params := &ConnParams{ Host: host, @@ -1170,30 +1168,34 @@ func runMysql(t *testing.T, params *ConnParams, command string) (string, bool) { command = command[len(enableCleartextPluginPrefix):] args = append(args, "--enable-cleartext-plugin") } - args = append(args, "-e", command) - if params.UnixSocket != "" { - args = append(args, "-S", params.UnixSocket) + if command == "--version" { + args = append(args, command) } else { - args = append(args, - "-h", params.Host, - "-P", fmt.Sprintf("%v", params.Port)) - } - if params.Uname != "" { - args = append(args, "-u", params.Uname) - } - if params.Pass != "" { - args = append(args, "-p"+params.Pass) - } - if params.DbName != "" { - args = append(args, "-D", params.DbName) - } - if params.Flags&CapabilityClientSSL > 0 { - args = append(args, - "--ssl", - "--ssl-ca", params.SslCa, - "--ssl-cert", params.SslCert, - "--ssl-key", params.SslKey, - "--ssl-verify-server-cert") + args = append(args, "-e", command) + if params.UnixSocket != "" { + args = append(args, "-S", params.UnixSocket) + } else { + args = append(args, + "-h", params.Host, + "-P", fmt.Sprintf("%v", params.Port)) + } + if params.Uname != "" { + args = append(args, "-u", params.Uname) + } + if params.Pass != "" { + args = append(args, "-p"+params.Pass) + } + if params.DbName != "" { + args = append(args, "-D", params.DbName) + } + if params.Flags&CapabilityClientSSL > 0 { + args = append(args, + "--ssl", + "--ssl-ca", params.SslCa, + "--ssl-cert", params.SslCert, + "--ssl-key", params.SslKey, + "--ssl-verify-server-cert") + } } env := []string{ "LD_LIBRARY_PATH=" + path.Join(dir, "lib/mysql"), diff --git a/go/vt/vttablet/tabletmanager/vreplication/vstreamer_client_test.go b/go/vt/vttablet/tabletmanager/vreplication/vstreamer_client_test.go index e63bad6e668..1afe8e1a906 100644 --- a/go/vt/vttablet/tabletmanager/vreplication/vstreamer_client_test.go +++ b/go/vt/vttablet/tabletmanager/vreplication/vstreamer_client_test.go @@ -184,6 +184,7 @@ func TestTabletVStreamerClientVStream(t *testing.T) { "drop table t1", fmt.Sprintf("drop table %s.t1", vrepldb), }) + env.SchemaEngine.Reload(context.Background()) ctx := context.Background() err := vsClient.Open(ctx) @@ -241,6 +242,7 @@ func TestTabletVStreamerClientVStreamRows(t *testing.T) { "drop table t1", fmt.Sprintf("drop table %s.t1", vrepldb), }) + env.SchemaEngine.Reload(context.Background()) qr, err := env.Mysqld.FetchSuperQuery(context.Background(), "select now()") if err != nil { @@ -443,6 +445,7 @@ func TestMySQLVStreamerClientVStream(t *testing.T) { "drop table t1", fmt.Sprintf("drop table %s.t1", vrepldb), }) + env.SchemaEngine.Reload(context.Background()) ctx := context.Background() err := vsClient.Open(ctx) @@ -497,6 +500,7 @@ func TestMySQLVStreamerClientVStreamRows(t *testing.T) { "drop table t1", fmt.Sprintf("drop table %s.t1", vrepldb), }) + env.SchemaEngine.Reload(context.Background()) qr, err := env.Mysqld.FetchSuperQuery(context.Background(), "select now()") if err != nil { diff --git a/go/vt/vttablet/tabletserver/vstreamer/vstreamer_flaky_test.go b/go/vt/vttablet/tabletserver/vstreamer/vstreamer_test.go similarity index 95% rename from go/vt/vttablet/tabletserver/vstreamer/vstreamer_flaky_test.go rename to go/vt/vttablet/tabletserver/vstreamer/vstreamer_test.go index a3eca41a71f..1a5b5d20911 100644 --- a/go/vt/vttablet/tabletserver/vstreamer/vstreamer_flaky_test.go +++ b/go/vt/vttablet/tabletserver/vstreamer/vstreamer_test.go @@ -55,8 +55,6 @@ func TestStatements(t *testing.T) { "update stream1 set val='bbb' where id = 1", "commit", }, - // MySQL issues GTID->BEGIN. - // MariaDB issues BEGIN->GTID. output: [][]string{{ `begin`, `type:FIELD field_event: fields: > `, @@ -111,34 +109,6 @@ func TestStatements(t *testing.T) { `gtid`, `type:DDL ddl:"truncate table stream2" `, }}, - }, { - // repair, optimize and analyze show up in binlog stream, but ignored by vitess. - input: "repair table stream2", - output: [][]string{{ - `gtid`, - `type:OTHER `, - }}, - }, { - input: "optimize table stream2", - output: [][]string{{ - `gtid`, - `type:OTHER `, - }}, - }, { - input: "analyze table stream2", - output: [][]string{{ - `gtid`, - `type:OTHER `, - }}, - }, { - // select, set, show and describe don't get logged. - input: "select * from stream1", - }, { - input: "set @val=1", - }, { - input: "show tables", - }, { - input: "describe stream1", }} runCases(t, nil, testcases, "") @@ -148,6 +118,70 @@ func TestStatements(t *testing.T) { runCases(t, nil, testcases, "") } +// TestOther tests "other" statements. These statements produce +// very different events depending on the version of mysql or mariadb +// So, we just show that vreplication transmits "OTHER" events +// if the binlog is affected by the statement. +func TestOther(t *testing.T) { + if testing.Short() { + t.Skip() + } + + execStatements(t, []string{ + "create table stream1(id int, val varbinary(128), primary key(id))", + "create table stream2(id int, val varbinary(128), primary key(id))", + }) + defer execStatements(t, []string{ + "drop table stream1", + "drop table stream2", + }) + engine.se.Reload(context.Background()) + + testcases := []string{ + "repair table stream2", + "optimize table stream2", + "analyze table stream2", + "select * from stream1", + "set @val=1", + "show tables", + "describe stream1", + } + + // customRun is a modified version of runCases. + customRun := func(mode string) { + t.Logf("Run mode: %v", mode) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + ch := startStream(ctx, t, nil, "") + + want := [][]string{{ + `gtid`, + `type:OTHER `, + }} + + for _, stmt := range testcases { + startPosition := masterPosition(t) + execStatement(t, stmt) + endPosition := masterPosition(t) + if startPosition == endPosition { + t.Logf("statement %s did not affect binlog", stmt) + continue + } + expectLog(ctx, t, stmt, ch, want) + } + cancel() + if evs, ok := <-ch; ok { + t.Fatalf("unexpected evs: %v", evs) + } + } + customRun("gtid") + + // Test FilePos flavor + engine.cp.Flavor = "FilePos" + defer func() { engine.cp.Flavor = "" }() + customRun("filePos") +} + func TestRegexp(t *testing.T) { if testing.Short() { t.Skip() @@ -424,8 +458,6 @@ func TestSelectFilter(t *testing.T) { "insert into t1 values (2, 4, 'aaa')", "commit", }, - // MySQL issues GTID->BEGIN. - // MariaDB issues BEGIN->GTID. output: [][]string{{ `begin`, `type:FIELD field_event: fields: > `, diff --git a/test.go b/test.go index 846cb0c8755..6022c739dae 100755 --- a/test.go +++ b/test.go @@ -106,7 +106,7 @@ const ( configFileName = "test/config.json" // List of flavors for which a bootstrap Docker image is available. - flavors = "mariadb,mysql56,mysql57,percona,percona57" + flavors = "mysql56,mysql57,mysql80,mariadb,mariadb103,percona,percona57,percona80" ) // Config is the overall object serialized in test/config.json. From c60368a845bffe414937c5d636e1966909e2400c Mon Sep 17 00:00:00 2001 From: Sugu Sougoumarane Date: Sun, 22 Dec 2019 13:24:42 -0800 Subject: [PATCH 2/4] tests: deflake filelogger_test.go Signed-off-by: Sugu Sougoumarane --- go/vt/vttablet/filelogger/filelogger_test.go | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/go/vt/vttablet/filelogger/filelogger_test.go b/go/vt/vttablet/filelogger/filelogger_test.go index b952496cc3b..25009700a5e 100644 --- a/go/vt/vttablet/filelogger/filelogger_test.go +++ b/go/vt/vttablet/filelogger/filelogger_test.go @@ -60,13 +60,19 @@ func TestFileLog(t *testing.T) { tabletenv.StatsLogger.Send(log2) // Allow time for propagation - time.Sleep(10 * time.Millisecond) - - want := "\t\t\t''\t''\t0001-01-01 00:00:00.000000\t0001-01-01 00:00:00.000000\t0.000000\t\t\"test 1\"\tmap[]\t1\t\"test 1 PII\"\tmysql\t0.000000\t0.000000\t0\t0\t\"\"\t\n\t\t\t''\t''\t0001-01-01 00:00:00.000000\t0001-01-01 00:00:00.000000\t0.000000\t\t\"test 2\"\tmap[]\t1\t\"test 2 PII\"\tmysql\t0.000000\t0.000000\t0\t0\t\"\"\t\n" - contents, _ := ioutil.ReadFile(logPath) - got := string(contents) - if want != string(got) { - t.Errorf("streamlog file: want %q got %q", want, got) + for i := 0; i < 10; i++ { + time.Sleep(10 * time.Millisecond) + + want := "\t\t\t''\t''\t0001-01-01 00:00:00.000000\t0001-01-01 00:00:00.000000\t0.000000\t\t\"test 1\"\tmap[]\t1\t\"test 1 PII\"\tmysql\t0.000000\t0.000000\t0\t0\t\"\"\t\n\t\t\t''\t''\t0001-01-01 00:00:00.000000\t0001-01-01 00:00:00.000000\t0.000000\t\t\"test 2\"\tmap[]\t1\t\"test 2 PII\"\tmysql\t0.000000\t0.000000\t0\t0\t\"\"\t\n" + contents, _ := ioutil.ReadFile(logPath) + got := string(contents) + if want == got { + return + } + // Last iteration. + if i == 9 { + t.Errorf("streamlog file: want %q got %q", want, got) + } } } From 1c3d2fff62ffa4a23cc2ad41990071e29b8437ba Mon Sep 17 00:00:00 2001 From: Morgan Tocker Date: Sun, 22 Dec 2019 15:43:26 -0700 Subject: [PATCH 3/4] Add additional flavors to matrix for testing Signed-off-by: Morgan Tocker --- .github/workflows/unit.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/unit.yml b/.github/workflows/unit.yml index c029d9882be..ad69fb7dd09 100644 --- a/.github/workflows/unit.yml +++ b/.github/workflows/unit.yml @@ -6,7 +6,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - name: [mysql57, mysql80] + name: [percona56, mysql57, mysql80, mariadb101, mariadb102, mariadb103] steps: - name: Set up Go From 72952374b4c3bd570948c0e836dab0789ab1bea5 Mon Sep 17 00:00:00 2001 From: Sugu Sougoumarane Date: Sun, 22 Dec 2019 17:58:42 -0800 Subject: [PATCH 4/4] tests: HeartbeatTime=10s to reduce flakyness Do this for all tests by changing this in TestMain. Signed-off-by: Sugu Sougoumarane --- go/vt/vttablet/tabletserver/vstreamer/main_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/go/vt/vttablet/tabletserver/vstreamer/main_test.go b/go/vt/vttablet/tabletserver/vstreamer/main_test.go index cda8ad22251..f9a37b8dbc5 100644 --- a/go/vt/vttablet/tabletserver/vstreamer/main_test.go +++ b/go/vt/vttablet/tabletserver/vstreamer/main_test.go @@ -21,6 +21,7 @@ import ( "fmt" "os" "testing" + "time" "vitess.io/vitess/go/vt/vttablet/tabletserver/vstreamer/testenv" ) @@ -53,6 +54,12 @@ func TestMain(m *testing.M) { engine.Open(env.KeyspaceName, env.Cells[0]) defer engine.Close() + // GH actions machines sometimes exhibit multi-second delays on replication. + // So, give a generously high value for heartbeat time for all tests. + saveHeartbeat := HeartbeatTime + defer func() { HeartbeatTime = saveHeartbeat }() + HeartbeatTime = 10 * time.Second + return m.Run() }() os.Exit(exitCode)