Miscellaneous code modifications based on observations made while doi…

…ng a code walkthrough (#12873) * Move to glog v1.1.0 Signed-off-by: Rohit Nayak <[email protected]> Miscellaneous fixes based on an internal code audit Signed-off-by: Rohit Nayak <[email protected]> More mods Signed-off-by: Rohit Nayak <[email protected]> Add modified zkctl.txt Signed-off-by: Rohit Nayak <[email protected]> fix imports Signed-off-by: Manan Gupta <[email protected]> * Address review comments Signed-off-by: Rohit Nayak <[email protected]> * Remove statsd.go which came due to an incorrect merge Signed-off-by: Rohit Nayak <[email protected]> * Address review comment Signed-off-by: Rohit Nayak <[email protected]> * Update flags help output Signed-off-by: Rohit Nayak <[email protected]> * Switch back to glog v1.0.0 since moving to glog v1.1.0 trips up zookeeper docker tests in Shard 25 Signed-off-by: Rohit Nayak <[email protected]> --------- Signed-off-by: Manan Gupta <[email protected]> Signed-off-by: Rohit Nayak <[email protected]>
vitessio · Jun 9, 2023 · 122b4e4 · 122b4e4
1 parent d4a4852
commit 122b4e4
Show file tree

Hide file tree

Showing 15 changed files with 46 additions and 36 deletions.
diff --git a/go/mysql/mysql56_gtid_set_test.go b/go/mysql/mysql56_gtid_set_test.go
@@ -123,7 +123,7 @@ func TestParseMysql56GTIDSetInvalid(t *testing.T) {
 
 	for _, input := range table {
 		_, err := ParseMysql56GTIDSet(input)
-		assert.Error(t, err, "parseMysql56GTIDSet(%#v) expected error, got none", err)
+		assert.Error(t, err, "ParseMysql56GTIDSet(%#v) expected error, got none", err)
 	}
 }
 

diff --git a/go/test/endtoend/cluster/vttablet_process.go b/go/test/endtoend/cluster/vttablet_process.go
@@ -42,6 +42,8 @@ import (
 	"vitess.io/vitess/go/vt/sqlparser"
 )
 
+const vttabletStateTimeout = 30 * time.Second
+
 // VttabletProcess is a generic handle for a running vttablet .
 // It can be spawned manually
 type VttabletProcess struct {
@@ -269,19 +271,19 @@ func (vttablet *VttabletProcess) GetTabletType() string {
 	return ""
 }
 
-// WaitForTabletStatus waits for 10 second till expected status is reached
+// WaitForTabletStatus waits for one of the expected statuses to be reached
 func (vttablet *VttabletProcess) WaitForTabletStatus(expectedStatus string) error {
-	return vttablet.WaitForTabletStatusesForTimeout([]string{expectedStatus}, 10*time.Second)
+	return vttablet.WaitForTabletStatusesForTimeout([]string{expectedStatus}, vttabletStateTimeout)
 }
 
-// WaitForTabletStatuses waits for 10 second till one of expected statuses is reached
+// WaitForTabletStatuses waits for one of expected statuses is reached
 func (vttablet *VttabletProcess) WaitForTabletStatuses(expectedStatuses []string) error {
-	return vttablet.WaitForTabletStatusesForTimeout(expectedStatuses, 10*time.Second)
+	return vttablet.WaitForTabletStatusesForTimeout(expectedStatuses, vttabletStateTimeout)
 }
 
-// WaitForTabletTypes waits for 10 second till one of expected statuses is reached
+// WaitForTabletTypes waits for one of expected statuses is reached
 func (vttablet *VttabletProcess) WaitForTabletTypes(expectedTypes []string) error {
-	return vttablet.WaitForTabletTypesForTimeout(expectedTypes, 10*time.Second)
+	return vttablet.WaitForTabletTypesForTimeout(expectedTypes, vttabletStateTimeout)
 }
 
 // WaitForTabletStatusesForTimeout waits till the tablet reaches to any of the provided statuses
@@ -335,7 +337,7 @@ func contains(arr []string, str string) bool {
 
 // WaitForBinLogPlayerCount waits till binlog player count var matches
 func (vttablet *VttabletProcess) WaitForBinLogPlayerCount(expectedCount int) error {
-	timeout := time.Now().Add(10 * time.Second)
+	timeout := time.Now().Add(vttabletStateTimeout)
 	for time.Now().Before(timeout) {
 		if vttablet.getVReplStreamCount() == fmt.Sprintf("%d", expectedCount) {
 			return nil
@@ -352,19 +354,23 @@ func (vttablet *VttabletProcess) WaitForBinLogPlayerCount(expectedCount int) err
 
 // WaitForBinlogServerState wait for the tablet's binlog server to be in the provided state.
 func (vttablet *VttabletProcess) WaitForBinlogServerState(expectedStatus string) error {
-	timeout := time.Now().Add(10 * time.Second)
-	for time.Now().Before(timeout) {
+	ctx, cancel := context.WithTimeout(context.Background(), vttabletStateTimeout)
+	defer cancel()
+	t := time.NewTicker(300 * time.Millisecond)
+	defer t.Stop()
+	for {
 		if vttablet.getVarValue("UpdateStreamState") == expectedStatus {
 			return nil
 		}
 		select {
 		case err := <-vttablet.exit:
 			return fmt.Errorf("process '%s' exited prematurely (err: %s)", vttablet.Name, err)
-		default:
-			time.Sleep(300 * time.Millisecond)
+		case <-ctx.Done():
+			return fmt.Errorf("vttablet %s, expected status of %s not reached before timeout of %v",
+				vttablet.TabletPath, expectedStatus, vttabletStateTimeout)
+		case <-t.C:
 		}
 	}
-	return fmt.Errorf("vttablet %s, expected status not reached", vttablet.TabletPath)
 }
 
 func (vttablet *VttabletProcess) getVReplStreamCount() string {
@@ -377,9 +383,9 @@ func (vttablet *VttabletProcess) getVarValue(keyname string) string {
 	return fmt.Sprintf("%v", object)
 }
 
-// TearDown shuts down the running vttablet service and fails after 10 seconds
+// TearDown shuts down the running vttablet service and fails after a timeout
 func (vttablet *VttabletProcess) TearDown() error {
-	return vttablet.TearDownWithTimeout(10 * time.Second)
+	return vttablet.TearDownWithTimeout(vttabletStateTimeout)
 }
 
 // Kill shuts down the running vttablet service immediately.

diff --git a/go/test/endtoend/vreplication/vreplication_test.go b/go/test/endtoend/vreplication/vreplication_test.go
@@ -609,6 +609,7 @@ func TestCellAliasVreplicationWorkflow(t *testing.T) {
 	verifyClusterHealth(t, vc)
 
 	insertInitialData(t)
+
 	t.Run("VStreamFrom", func(t *testing.T) {
 		testVStreamFrom(t, keyspace, 2)
 	})

diff --git a/go/test/endtoend/vtgate/schematracker/loadkeyspace/schema_load_keyspace_test.go b/go/test/endtoend/vtgate/schematracker/loadkeyspace/schema_load_keyspace_test.go
@@ -72,7 +72,7 @@ func TestLoadKeyspaceWithNoTablet(t *testing.T) {
 		Name:      keyspaceName,
 		SchemaSQL: sqlSchema,
 	}
-	clusterInstance.VtTabletExtraArgs = []string{"--queryserver-config-schema-change-signal"}
+	clusterInstance.VtTabletExtraArgs = append(clusterInstance.VtTabletExtraArgs, "--queryserver-config-schema-change-signal")
 	err = clusterInstance.StartUnshardedKeyspace(*keyspace, 0, false)
 	require.NoError(t, err)
 
@@ -86,7 +86,7 @@ func TestLoadKeyspaceWithNoTablet(t *testing.T) {
 	}
 
 	// Start vtgate with the schema_change_signal flag
-	clusterInstance.VtGateExtraArgs = []string{"--schema_change_signal"}
+	clusterInstance.VtGateExtraArgs = append(clusterInstance.VtGateExtraArgs, "--schema_change_signal")
 	err = clusterInstance.StartVtgate()
 	require.NoError(t, err)
 

diff --git a/go/test/endtoend/vtgate/schematracker/unauthorized/unauthorized_test.go b/go/test/endtoend/vtgate/schematracker/unauthorized/unauthorized_test.go
@@ -66,8 +66,13 @@ func TestMain(m *testing.M) {
 			SchemaSQL: SchemaSQL,
 			VSchema:   VSchema,
 		}
-		clusterInstance.VtGateExtraArgs = []string{"--schema_change_signal"}
-		clusterInstance.VtTabletExtraArgs = []string{"--queryserver-config-schema-change-signal", "--queryserver-config-schema-change-signal-interval", "0.1", "--queryserver-config-strict-table-acl", "--queryserver-config-acl-exempt-acl", "userData1", "--table-acl-config", "dummy.json"}
+		clusterInstance.VtGateExtraArgs = append(clusterInstance.VtGateExtraArgs, "--schema_change_signal")
+		clusterInstance.VtTabletExtraArgs = append(clusterInstance.VtTabletExtraArgs,
+			"--queryserver-config-schema-change-signal",
+			"--queryserver-config-schema-change-signal-interval", "0.1",
+			"--queryserver-config-strict-table-acl",
+			"--queryserver-config-acl-exempt-acl", "userData1",
+			"--table-acl-config", "dummy.json")
 		err = clusterInstance.StartKeyspace(*keyspace, []string{"-80", "80-"}, 0, false)
 		if err != nil {
 			return 1

diff --git a/go/vt/mysqlctl/binlogs_gtid_test.go b/go/vt/mysqlctl/binlogs_gtid_test.go
@@ -14,6 +14,10 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
+// Package mysqlctl_test is the blackbox tests for package mysqlctl.
+// Tests that need to use fakemysqldaemon must be written as blackbox tests;
+// since fakemysqldaemon imports mysqlctl, importing fakemysqldaemon in
+// a `package mysqlctl` test would cause a circular import.
 package mysqlctl
 
 import (

diff --git a/go/vt/mysqlctl/compression.go b/go/vt/mysqlctl/compression.go
@@ -206,9 +206,9 @@ func newBuiltinDecompressor(engine string, reader io.Reader, logger logutil.Logg
 			return nil, err
 		}
 		decompressor = d
-	case "lz4":
+	case Lz4Compressor:
 		decompressor = io.NopCloser(lz4.NewReader(reader))
-	case "zstd":
+	case ZstdCompressor:
 		d, err := zstd.NewReader(reader)
 		if err != nil {
 			return nil, err

diff --git a/go/vt/schemadiff/table.go b/go/vt/schemadiff/table.go
@@ -877,7 +877,6 @@ func (c *CreateTableEntity) TableDiff(other *CreateTableEntity, hints *DiffHints
 	}
 	if tableSpecHasChanged {
 		parentAlterTableEntityDiff = newAlterTableEntityDiff(alterTable)
-
 	}
 	for _, superfluousFulltextKey := range superfluousFulltextKeys {
 		alterTable := &sqlparser.AlterTable{

diff --git a/go/vt/servenv/buildinfo_test.go b/go/vt/servenv/buildinfo_test.go
@@ -33,17 +33,17 @@ func TestVersionString(t *testing.T) {
 		buildTimePretty: "time is now",
 		buildGitRev:     "d54b87ca0be09b678bb4490060e8f23f890ddb92",
 		buildGitBranch:  "gitBranch",
-		goVersion:       "1.19.3",
+		goVersion:       "1.20.2",
 		goOS:            "amiga",
 		goArch:          "amd64",
 		version:         "v1.2.3-SNAPSHOT",
 	}
 
-	assert.Equal(t, "Version: v1.2.3-SNAPSHOT (Git revision d54b87ca0be09b678bb4490060e8f23f890ddb92 branch 'gitBranch') built on time is now by user@host using 1.19.3 amiga/amd64", v.String())
+	assert.Equal(t, "Version: v1.2.3-SNAPSHOT (Git revision d54b87ca0be09b678bb4490060e8f23f890ddb92 branch 'gitBranch') built on time is now by user@host using 1.20.2 amiga/amd64", v.String())
 
 	v.jenkinsBuildNumber = 422
 
-	assert.Equal(t, "Version: v1.2.3-SNAPSHOT (Jenkins build 422) (Git revision d54b87ca0be09b678bb4490060e8f23f890ddb92 branch 'gitBranch') built on time is now by user@host using 1.19.3 amiga/amd64", v.String())
+	assert.Equal(t, "Version: v1.2.3-SNAPSHOT (Jenkins build 422) (Git revision d54b87ca0be09b678bb4490060e8f23f890ddb92 branch 'gitBranch') built on time is now by user@host using 1.20.2 amiga/amd64", v.String())
 
 	assert.Equal(t, "8.0.30-Vitess", v.MySQLVersion())
 }
diff --git a/go/vt/vtexplain/vtexplain_vttablet.go b/go/vt/vtexplain/vtexplain_vttablet.go
@@ -530,7 +530,6 @@ func (t *explainTablet) HandleQuery(c *mysql.Conn, query string, callback func(*
 	// return the pre-computed results for any schema introspection queries
 	tEnv := t.vte.getGlobalTabletEnv()
 	result := tEnv.getResult(query)
-
 	if result != nil {
 		return callback(result)
 	}

diff --git a/go/vt/vttablet/tabletmanager/tm_init.go b/go/vt/vttablet/tabletmanager/tm_init.go
@@ -744,8 +744,10 @@ func (tm *TabletManager) initTablet(ctx context.Context) error {
 		// instance of a startup timeout). Upon running this code
 		// again, we want to fix ShardReplication.
 		if updateErr := topo.UpdateTabletReplicationData(ctx, tm.TopoServer, tablet); updateErr != nil {
+			log.Errorf("UpdateTabletReplicationData failed for tablet %v: %v", topoproto.TabletAliasString(tablet.Alias), updateErr)
 			return vterrors.Wrap(updateErr, "UpdateTabletReplicationData failed")
 		}
+		log.Infof("Successfully updated tablet replication data for alias: %v", topoproto.TabletAliasString(tablet.Alias))
 
 		// Then overwrite everything, ignoring version mismatch.
 		if err := tm.TopoServer.UpdateTablet(ctx, topo.NewTabletInfo(tablet, nil)); err != nil {

diff --git a/go/vt/vttablet/tabletmanager/vreplication/vplayer_flaky_test.go b/go/vt/vttablet/tabletmanager/vreplication/vplayer_flaky_test.go
@@ -38,6 +38,7 @@ import (
 	"vitess.io/vitess/go/sqltypes"
 	"vitess.io/vitess/go/vt/binlog/binlogplayer"
 	"vitess.io/vitess/go/vt/log"
+
 	binlogdatapb "vitess.io/vitess/go/vt/proto/binlogdata"
 	qh "vitess.io/vitess/go/vt/vttablet/tabletmanager/vreplication/queryhistory"
 )

diff --git a/go/vt/wrangler/testlib/apply_schema_flaky_test.go b/go/vt/wrangler/testlib/apply_schema_flaky_test.go
@@ -17,16 +17,14 @@ limitations under the License.
 package testlib
 
 import (
+	"context"
 	"strings"
 	"testing"
 	"time"
 
-	"vitess.io/vitess/go/vt/discovery"
-
-	"context"
-
 	"vitess.io/vitess/go/mysql/fakesqldb"
 	"vitess.io/vitess/go/sqltypes"
+	"vitess.io/vitess/go/vt/discovery"
 	"vitess.io/vitess/go/vt/logutil"
 	"vitess.io/vitess/go/vt/mysqlctl/tmutils"
 	"vitess.io/vitess/go/vt/topo/memorytopo"

diff --git a/test.go b/test.go
@@ -188,11 +188,6 @@ func (t *Test) run(dir, dataDir string) ([]byte, error) {
 			testCmd = append(testCmd, "--partial-keyspace")
 		}
 		testCmd = append(testCmd, extraArgs...)
-		if *docker {
-			// Teardown is unnecessary since Docker kills everything.
-			// Go cluster doesn't recognize 'skip-teardown' flag so commenting it out for now.
-			// testCmd = append(testCmd, "--skip-teardown")
-		}
 	}
 
 	var cmd *exec.Cmd

diff --git a/tools/rowlog/rowlog.go b/tools/rowlog/rowlog.go
@@ -443,7 +443,7 @@ func processPositionResult(gtidset string) (string, string) {
 	subs := strings.Split(arr[1], "-")
 	id, err := strconv.Atoi(subs[0])
 	if err != nil {
-		fmt.Printf(err.Error())
+		fmt.Println(err.Error())
 		return "", ""
 	}
 	firstPos := arr[0] + ":" + strconv.Itoa(id) // subs[0]