vttablet: Cleanup unused db version string (#13102)

* vttablet: Cleanup unused db version string We're reading the database version string from the `mysqld` binary, but we subsequently never use it anywhere. In case Vitess is used with containers but with a separate MySQL container, this check is also not correct as it would report whatever is bundled with Vitess, not the MySQL version that actually runs. Since we don't need to have it and because of the above reason it's better that we remove it here. Signed-off-by: Dirkjan Bussink <[email protected]> * vtorc: Handle unknown fields correctly If there's fields being dropped, we want to ignore those and not error on them as that's how usually protobufs work. Signed-off-by: Dirkjan Bussink <[email protected]> --------- Signed-off-by: Dirkjan Bussink <[email protected]>
vitessio · May 18, 2023 · 2df867c · 2df867c
1 parent 2dd3853
commit 2df867c
Show file tree

Hide file tree

Showing 14 changed files with 238 additions and 328 deletions.
diff --git a/go/cmd/vttablet/vttablet.go b/go/cmd/vttablet/vttablet.go
@@ -109,7 +109,7 @@ func main() {
 	if servenv.GRPCPort() != 0 {
 		gRPCPort = int32(servenv.GRPCPort())
 	}
-	tablet, err := tabletmanager.BuildTabletFromInput(tabletAlias, int32(servenv.Port()), gRPCPort, mysqld.GetVersionString(), config.DB)
+	tablet, err := tabletmanager.BuildTabletFromInput(tabletAlias, int32(servenv.Port()), gRPCPort, config.DB)
 	if err != nil {
 		log.Exitf("failed to parse --tablet-path: %v", err)
 	}

diff --git a/go/json2/unmarshal.go b/go/json2/unmarshal.go
@@ -33,7 +33,8 @@ var carriageReturn = []byte("\n")
 // efficient and should not be used for high QPS operations.
 func Unmarshal(data []byte, v any) error {
 	if pb, ok := v.(proto.Message); ok {
-		return annotate(data, protojson.Unmarshal(data, pb))
+		opts := protojson.UnmarshalOptions{DiscardUnknown: true}
+		return annotate(data, opts.Unmarshal(data, pb))
 	}
 	return annotate(data, json.Unmarshal(data, v))
 }

diff --git a/go/test/endtoend/vtgr/vtgr_test.go b/go/test/endtoend/vtgr/vtgr_test.go
@@ -305,10 +305,10 @@ func TestVTGRFailover(t *testing.T) {
 func getTablet(t *testing.T, cluster *cluster.LocalProcessCluster, alias string) *topodatapb.Tablet {
 	result, err := cluster.VtctlclientProcess.ExecuteCommandWithOutput("GetTablet", alias)
 	require.NoError(t, err)
-	var tabletInfo *topodatapb.Tablet
+	var tabletInfo topodatapb.Tablet
 	err = json2.Unmarshal([]byte(result), &tabletInfo)
 	require.NoError(t, err)
-	return tabletInfo
+	return &tabletInfo
 }
 
 func findTabletByAlias(tablets []*cluster.Vttablet, alias *topodatapb.TabletAlias) *cluster.Vttablet {

diff --git a/go/vt/mysqlctl/mysqld.go b/go/vt/mysqlctl/mysqld.go
@@ -1176,7 +1176,7 @@ func buildLdPaths() ([]string, error) {
 	return ldPaths, nil
 }
 
-// GetVersionString is part of the MysqlDeamon interface.
+// GetVersionString is part of the MysqlDaemon interface.
 func (mysqld *Mysqld) GetVersionString() string {
 	return fmt.Sprintf("%d.%d.%d", mysqld.capabilities.version.Major, mysqld.capabilities.version.Minor, mysqld.capabilities.version.Patch)
 }

diff --git a/go/vt/proto/topodata/topodata.pb.go b/go/vt/proto/topodata/topodata.pb.go
diff --git a/go/vt/proto/topodata/topodata_vtproto.pb.go b/go/vt/proto/topodata/topodata_vtproto.pb.go
diff --git a/go/vt/vtorc/inst/analysis_dao.go b/go/vt/vtorc/inst/analysis_dao.go
@@ -346,14 +346,15 @@ func GetReplicationAnalysis(keyspace string, shard string, hints *ReplicationAna
 		}
 
 		tablet := &topodatapb.Tablet{}
-		if err := prototext.Unmarshal([]byte(m.GetString("tablet_info")), tablet); err != nil {
+		opts := prototext.UnmarshalOptions{DiscardUnknown: true}
+		if err := opts.Unmarshal([]byte(m.GetString("tablet_info")), tablet); err != nil {
 			log.Errorf("could not read tablet %v: %v", m.GetString("tablet_info"), err)
 			return nil
 		}
 
 		primaryTablet := &topodatapb.Tablet{}
 		if str := m.GetString("primary_tablet_info"); str != "" {
-			if err := prototext.Unmarshal([]byte(str), primaryTablet); err != nil {
+			if err := opts.Unmarshal([]byte(str), primaryTablet); err != nil {
 				log.Errorf("could not read tablet %v: %v", str, err)
 				return nil
 			}

diff --git a/go/vt/vtorc/inst/tablet_dao.go b/go/vt/vtorc/inst/tablet_dao.go
@@ -159,8 +159,9 @@ func ReadTablet(instanceKey InstanceKey) (*topodatapb.Tablet, error) {
 		`
 	args := sqlutils.Args(instanceKey.Hostname, instanceKey.Port)
 	tablet := &topodatapb.Tablet{}
+	opts := prototext.UnmarshalOptions{DiscardUnknown: true}
 	err := db.QueryVTOrc(query, args, func(row sqlutils.RowMap) error {
-		return prototext.Unmarshal([]byte(row.GetString("info")), tablet)
+		return opts.Unmarshal([]byte(row.GetString("info")), tablet)
 	})
 	if err != nil {
 		return nil, err

diff --git a/go/vt/vtorc/logic/tablet_discovery.go b/go/vt/vtorc/logic/tablet_discovery.go
@@ -245,7 +245,8 @@ func refreshTablets(tablets map[string]*topo.TabletInfo, query string, args []an
 			Port:     row.GetInt("port"),
 		}
 		tablet := &topodatapb.Tablet{}
-		if err := prototext.Unmarshal([]byte(row.GetString("info")), tablet); err != nil {
+		opts := prototext.UnmarshalOptions{DiscardUnknown: true}
+		if err := opts.Unmarshal([]byte(row.GetString("info")), tablet); err != nil {
 			log.Error(err)
 			return nil
 		}
@@ -338,7 +339,8 @@ func shardPrimary(keyspace string, shard string) (primary *topodatapb.Tablet, er
 	err = db.Db.QueryVTOrc(query, sqlutils.Args(keyspace, shard, topodatapb.TabletType_PRIMARY), func(m sqlutils.RowMap) error {
 		if primary == nil {
 			primary = &topodatapb.Tablet{}
-			return prototext.Unmarshal([]byte(m.GetString("info")), primary)
+			opts := prototext.UnmarshalOptions{DiscardUnknown: true}
+			return opts.Unmarshal([]byte(m.GetString("info")), primary)
 		}
 		return nil
 	})

diff --git a/go/vt/vttablet/tabletmanager/tm_init.go b/go/vt/vttablet/tabletmanager/tm_init.go
@@ -201,7 +201,7 @@ type TabletManager struct {
 }
 
 // BuildTabletFromInput builds a tablet record from input parameters.
-func BuildTabletFromInput(alias *topodatapb.TabletAlias, port, grpcPort int32, dbServerVersion string, db *dbconfigs.DBConfigs) (*topodatapb.Tablet, error) {
+func BuildTabletFromInput(alias *topodatapb.TabletAlias, port, grpcPort int32, db *dbconfigs.DBConfigs) (*topodatapb.Tablet, error) {
 	hostname := tabletHostname
 	if hostname == "" {
 		var err error
@@ -262,7 +262,6 @@ func BuildTabletFromInput(alias *topodatapb.TabletAlias, port, grpcPort int32, d
 		Type:                 tabletType,
 		DbNameOverride:       initDbNameOverride,
 		Tags:                 mergeTags(buildTags, initTags),
-		DbServerVersion:      dbServerVersion,
 		DefaultConnCollation: uint32(charset),
 	}, nil
 }

diff --git a/go/vt/vttablet/tabletmanager/tm_init_test.go b/go/vt/vttablet/tabletmanager/tm_init_test.go
@@ -43,12 +43,6 @@ import (
 	vschemapb "vitess.io/vitess/go/vt/proto/vschema"
 )
 
-var (
-	dbServerVersion = "8.0.0"
-	charsetName     = "utf8mb4"
-	dbsvCollID      = collations.NewEnvironment(dbServerVersion).DefaultCollationForCharset(charsetName).ID()
-)
-
 func TestStartBuildTabletFromInput(t *testing.T) {
 	alias := &topodatapb.TabletAlias{
 		Cell: "cell",
@@ -76,17 +70,16 @@ func TestStartBuildTabletFromInput(t *testing.T) {
 		Type:                 topodatapb.TabletType_REPLICA,
 		Tags:                 map[string]string{},
 		DbNameOverride:       "aa",
-		DbServerVersion:      dbServerVersion,
-		DefaultConnCollation: uint32(dbsvCollID),
+		DefaultConnCollation: uint32(collations.Default()),
 	}
 
-	gotTablet, err := BuildTabletFromInput(alias, port, grpcport, dbServerVersion, nil)
+	gotTablet, err := BuildTabletFromInput(alias, port, grpcport, nil)
 	require.NoError(t, err)
 
 	// Hostname should be resolved.
 	assert.Equal(t, wantTablet, gotTablet)
 	tabletHostname = ""
-	gotTablet, err = BuildTabletFromInput(alias, port, grpcport, dbServerVersion, nil)
+	gotTablet, err = BuildTabletFromInput(alias, port, grpcport, nil)
 	require.NoError(t, err)
 	assert.NotEqual(t, "", gotTablet.Hostname)
 
@@ -98,7 +91,7 @@ func TestStartBuildTabletFromInput(t *testing.T) {
 		Start: []byte(""),
 		End:   []byte("\xc0"),
 	}
-	gotTablet, err = BuildTabletFromInput(alias, port, grpcport, dbServerVersion, nil)
+	gotTablet, err = BuildTabletFromInput(alias, port, grpcport, nil)
 	require.NoError(t, err)
 	// KeyRange check is explicit because the next comparison doesn't
 	// show the diff well enough.
@@ -108,25 +101,25 @@ func TestStartBuildTabletFromInput(t *testing.T) {
 	// Invalid inputs.
 	initKeyspace = ""
 	initShard = "0"
-	_, err = BuildTabletFromInput(alias, port, grpcport, dbServerVersion, nil)
+	_, err = BuildTabletFromInput(alias, port, grpcport, nil)
 	assert.Contains(t, err.Error(), "init_keyspace and init_shard must be specified")
 
 	initKeyspace = "test_keyspace"
 	initShard = ""
-	_, err = BuildTabletFromInput(alias, port, grpcport, dbServerVersion, nil)
+	_, err = BuildTabletFromInput(alias, port, grpcport, nil)
 	assert.Contains(t, err.Error(), "init_keyspace and init_shard must be specified")
 
 	initShard = "x-y"
-	_, err = BuildTabletFromInput(alias, port, grpcport, dbServerVersion, nil)
+	_, err = BuildTabletFromInput(alias, port, grpcport, nil)
 	assert.Contains(t, err.Error(), "cannot validate shard name")
 
 	initShard = "0"
 	initTabletType = "bad"
-	_, err = BuildTabletFromInput(alias, port, grpcport, dbServerVersion, nil)
+	_, err = BuildTabletFromInput(alias, port, grpcport, nil)
 	assert.Contains(t, err.Error(), "unknown TabletType bad")
 
 	initTabletType = "primary"
-	_, err = BuildTabletFromInput(alias, port, grpcport, dbServerVersion, nil)
+	_, err = BuildTabletFromInput(alias, port, grpcport, nil)
 	assert.Contains(t, err.Error(), "invalid init_tablet_type PRIMARY")
 }
 
@@ -159,11 +152,10 @@ func TestBuildTabletFromInputWithBuildTags(t *testing.T) {
 		Type:                 topodatapb.TabletType_REPLICA,
 		Tags:                 servenv.AppVersion.ToStringMap(),
 		DbNameOverride:       "aa",
-		DbServerVersion:      dbServerVersion,
-		DefaultConnCollation: uint32(dbsvCollID),
+		DefaultConnCollation: uint32(collations.Default()),
 	}
 
-	gotTablet, err := BuildTabletFromInput(alias, port, grpcport, dbServerVersion, nil)
+	gotTablet, err := BuildTabletFromInput(alias, port, grpcport, nil)
 	require.NoError(t, err)
 	assert.Equal(t, wantTablet, gotTablet)
 }

diff --git a/proto/topodata.proto b/proto/topodata.proto
@@ -157,16 +157,14 @@ message Tablet {
   //
   vttime.Time primary_term_start_time = 14;
 
-  // db_server_version represents the database version used by the tablet.
-  string db_server_version = 15;
-
   // default_conn_collation is the default connection collation used by this tablet.
   uint32 default_conn_collation = 16;
 
   // OBSOLETE: ip and tablet health information
   // string ip = 3;
   // map<string, string> health_map = 11;
-  reserved 3, 11;
+  // string db_server_version = 15;
+  reserved 3, 11, 15;
 }
 
 // A Shard contains data about a subset of the data whithin a keyspace.

diff --git a/web/vtadmin/src/proto/vtadmin.d.ts b/web/vtadmin/src/proto/vtadmin.d.ts
diff --git a/web/vtadmin/src/proto/vtadmin.js b/web/vtadmin/src/proto/vtadmin.js