vtctl,vindexes: logs warnings and export stat for unknown vindex params

changelog/18.0/18.0.0/summary.md

@@ -11,6 +11,8 @@
   - **[Deprecations and Deletions](#deprecations-and-deletions)**
     - [Deleted `k8stopo`](#deleted-k8stopo)
     - [Deleted `vtgr`](#deleted-vtgr)
+  - **[New stats](#new-stats)**
+    - [VTGate Vindex unknown parameters](#vtgate-vindex-unknown-parameters)
 
 ## <a id="major-changes"/>Major Changes
 
@@ -38,4 +40,10 @@ The `k8stopo` has been deprecated in Vitess 17, also see https://github.com/vite
 
 #### <a id="deleted-vtgr"/>Deleted `vtgr`
 
-The `vtgr` has been deprecated in Vitess 17, also see https://github.com/vitessio/vitess/issues/13300. With Vitess 18 `vtgr` has been removed.
+The `vtgr` has been deprecated in Vitess 17, also see https://github.com/vitessio/vitess/issues/13300. With Vitess 18 `vtgr` has been removed.
+
+### <a id="new-stats"/>New stats
+
+#### <a id="vtgate-vindex-unknown-parameters"/>VTGate Vindex unknown parameters
+
+The VTGate stat `VindexUnknownParameters` gauges unknown Vindex parameters found in the latest VSchema pulled from the topology.

go/vt/topo/vschema.go

@@ -33,7 +33,7 @@ import (
 // SaveVSchema first validates the VSchema, then saves it.
 // If the VSchema is empty, just remove it.
 func (ts *Server) SaveVSchema(ctx context.Context, keyspace string, vschema *vschemapb.Keyspace) error {
-	err := vindexes.ValidateKeyspace(vschema)
+	_, err := vindexes.BuildKeyspace(vschema)
 	if err != nil {
 		return err
 	}

go/vt/vtctl/testdata/unknown-params-logged-dry-run-vschema.json

@@ -0,0 +1,18 @@
+{
+  "sharded": true,
+  "vindexes": {
+    "hash_vdx" : {
+      "type": "hash",
+      "params": {
+        "foo": "bar",
+        "hello": "world"
+      }
+    },
+    "binary_vdx": {
+      "type": "binary",
+      "params": {
+        "hello": "world"
+      }
+    }
+  }
+}

go/vt/vtctl/testdata/unknown-params-logged-vschema.json

@@ -0,0 +1,18 @@
+{
+  "sharded": true,
+  "vindexes": {
+    "binary_vdx": {
+      "type": "binary",
+      "params": {
+        "hello": "world"
+      }
+    },
+    "hash_vdx": {
+      "type": "hash",
+      "params": {
+        "foo": "bar",
+        "hello": "world"
+      }
+    }
+  }
+}

go/vt/vtctl/vtctl.go

@@ -123,6 +123,7 @@ import (
 	"vitess.io/vitess/go/vt/topotools"
 	"vitess.io/vitess/go/vt/vtctl/workflow"
 	"vitess.io/vitess/go/vt/vterrors"
+	"vitess.io/vitess/go/vt/vtgate/vindexes"
 	"vitess.io/vitess/go/vt/wrangler"
 )
 
@@ -3335,6 +3336,27 @@ func commandApplyVSchema(ctx context.Context, wr *wrangler.Wrangler, subFlags *p
 		wr.Logger().Printf("New VSchema object:\n%s\nIf this is not what you expected, check the input data (as JSON parsing will skip unexpected fields).\n", b)
 	}
 
+	// Validate the VSchema.
+	ksVs, err := vindexes.BuildKeyspace(vs)
+	if err != nil {
+		return err
+	}
+
+	// Log unknown Vindex params as warnings.
+	var vdxNames []string
+	for name := range ksVs.Vindexes {
+		vdxNames = append(vdxNames, name)
+	}
+	sort.Strings(vdxNames)
+	for _, name := range vdxNames {
+		vdx := ksVs.Vindexes[name]
+		if val, ok := vdx.(vindexes.ParamValidating); ok {
+			for _, param := range val.UnknownParams() {
+				wr.Logger().Warningf("Unknown param in vindex %s: %s", name, param)
+			}
+		}
+	}
+
 	if *dryRun {
 		wr.Logger().Printf("Dry run: Skipping update of VSchema\n")
 		return nil

go/vt/vtctl/vtctl_test.go

@@ -18,7 +18,9 @@ package vtctl
 
 import (
 	"context"
+	_ "embed"
 	"fmt"
+	"regexp"
 	"strings"
 	"testing"
 	"time"
@@ -32,6 +34,109 @@ import (
 	"vitess.io/vitess/go/vt/wrangler"
 )
 
+var (
+	//go:embed testdata/unknown-params-logged-vschema.json
+	unknownParamsLoggedVSchema string
+
+	//go:embed testdata/unknown-params-logged-dry-run-vschema.json
+	unknownParamsLoggedDryRunVSchema string
+)
+
+// TestApplyVSchema tests the the MoveTables client command
+// via the commandVRApplyVSchema() cmd handler.
+func TestApplyVSchema(t *testing.T) {
+	shard := "0"
+	ks := "ks"
+	ctx := context.Background()
+	env := newTestVTCtlEnv()
+	defer env.close()
+	_ = env.addTablet(100, ks, shard, &topodatapb.KeyRange{}, topodatapb.TabletType_PRIMARY)
+
+	tests := []struct {
+		name          string
+		args          []string
+		expectResults func()
+		want          string
+	}{
+		{
+			name: "EmptyVSchema",
+			args: []string{"--vschema", "{}", ks},
+			want: "New VSchema object:\n{}\nIf this is not what you expected, check the input data (as JSON parsing will skip unexpected fields).\n\n",
+		},
+		{
+			name: "UnknownParamsLogged",
+			args: []string{"--vschema", unknownParamsLoggedVSchema, ks},
+			want: `/New VSchema object:
+{
+  "sharded": true,
+  "vindexes": {
+    "binary_vdx": {
+      "type": "binary",
+      "params": {
+        "hello": "world"
+      }
+    },
+    "hash_vdx": {
+      "type": "hash",
+      "params": {
+        "foo": "bar",
+        "hello": "world"
+      }
+    }
+  }
+}
+If this is not what you expected, check the input data \(as JSON parsing will skip unexpected fields\)\.
+
+.*W.* .* vtctl.go:.* Unknown param in vindex binary_vdx: hello
+W.* .* vtctl.go:.* Unknown param in vindex hash_vdx: foo
+W.* .* vtctl.go:.* Unknown param in vindex hash_vdx: hello`,
+		},
+		{
+			name: "UnknownParamsLoggedWithDryRun",
+			args: []string{"--vschema", unknownParamsLoggedDryRunVSchema, "--dry-run", ks},
+			want: `/New VSchema object:
+{
+  "sharded": true,
+  "vindexes": {
+    "binary_vdx": {
+      "type": "binary",
+      "params": {
+        "hello": "world"
+      }
+    },
+    "hash_vdx": {
+      "type": "hash",
+      "params": {
+        "foo": "bar",
+        "hello": "world"
+      }
+    }
+  }
+}
+If this is not what you expected, check the input data \(as JSON parsing will skip unexpected fields\)\.
+
+.*W.* .* vtctl.go:.* Unknown param in vindex binary_vdx: hello
+W.* .* vtctl.go:.* Unknown param in vindex hash_vdx: foo
+W.* .* vtctl.go:.* Unknown param in vindex hash_vdx: hello
+Dry run: Skipping update of VSchema`,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			subFlags := pflag.NewFlagSet("test", pflag.ContinueOnError)
+			err := commandApplyVSchema(ctx, env.wr, subFlags, tt.args)
+			require.NoError(t, err)
+			if strings.HasPrefix(tt.want, "/") {
+				require.Regexp(t, regexp.MustCompile(tt.want[1:]), env.cmdlog.String())
+			} else {
+				require.Equal(t, tt.want, env.cmdlog.String())
+			}
+			env.cmdlog.Clear()
+			env.tmc.clearResults()
+		})
+	}
+}
+
 // TestMoveTables tests the the MoveTables client command
 // via the commandVRWorkflow() cmd handler.
 // This currently only tests the Progress action (which is

go/vt/vtgate/executor.go

@@ -947,6 +947,13 @@ func (e *Executor) SaveVSchema(vschema *vindexes.VSchema, stats *VSchemaStats) {
 		vschemaCounters.Add("Reload", 1)
 	}
 
+	if vindexUnknownParams != nil {
+		var unknownParams int
+		for _, ks := range stats.Keyspaces {
+			unknownParams += ks.VindexUnknownParamsCount
+		}
+		vindexUnknownParams.Set(int64(unknownParams))
+	}
 }
 
 // ParseDestinationTarget parses destination target string and sets default keyspace if possible.

go/vt/vtgate/vindexes/binarymd5.go

@@ -33,15 +33,15 @@ var (
 
 // BinaryMD5 is a vindex that hashes binary bits to a keyspace id.
 type BinaryMD5 struct {
-	name   string
-	params map[string]string
+	name          string
+	unknownParams []string
 }
 
 // newBinaryMD5 creates a new BinaryMD5.
 func newBinaryMD5(name string, params map[string]string) (Vindex, error) {
 	return &BinaryMD5{
-		name:   name,
-		params: params,
+		name:          name,
+		unknownParams: FindUnknownParams(params, nil),
 	}, nil
 }
 
@@ -101,7 +101,7 @@ func (vind *BinaryMD5) Hash(id sqltypes.Value) ([]byte, error) {
 
 // UnknownParams implements the ParamValidating interface.
 func (vind *BinaryMD5) UnknownParams() []string {
-	return FindUnknownParams(vind.params, nil)
+	return vind.unknownParams
 }
 
 func vMD5Hash(source []byte) []byte {

go/vt/vtgate/vindexes/vschema.go

@@ -284,11 +284,10 @@ func BuildKeyspaceSchema(input *vschemapb.Keyspace, keyspace string) (*KeyspaceS
 	return vschema.Keyspaces[keyspace], err
 }
 
-// ValidateKeyspace ensures that the keyspace vschema is valid.
+// BuildKeyspace ensures that the keyspace vschema is valid.
 // External references (like sequence) are not validated.
-func ValidateKeyspace(input *vschemapb.Keyspace) error {
-	_, err := BuildKeyspaceSchema(input, "")
-	return err
+func BuildKeyspace(input *vschemapb.Keyspace) (*KeyspaceSchema, error) {
+	return BuildKeyspaceSchema(input, "")
 }
 
 func buildKeyspaces(source *vschemapb.SrvVSchema, vschema *VSchema) {

go/vt/vtgate/vindexes/vschema_test.go

@@ -234,7 +234,7 @@ func init() {
 }
 
 func TestUnshardedVSchemaValid(t *testing.T) {
-	err := ValidateKeyspace(&vschemapb.Keyspace{
+	_, err := BuildKeyspace(&vschemapb.Keyspace{
 		Sharded:  false,
 		Vindexes: make(map[string]*vschemapb.Vindex),
 		Tables:   make(map[string]*vschemapb.Table),
@@ -2432,7 +2432,7 @@ func TestValidate(t *testing.T) {
 			"t2": {},
 		},
 	}
-	err := ValidateKeyspace(good)
+	_, err := BuildKeyspace(good)
 	require.NoError(t, err)
 	bad := &vschemapb.Keyspace{
 		Sharded: true,
@@ -2445,7 +2445,7 @@ func TestValidate(t *testing.T) {
 			"t2": {},
 		},
 	}
-	err = ValidateKeyspace(bad)
+	_, err = BuildKeyspace(bad)
 	want := `vindexType "absent" not found`
 	if err == nil || !strings.HasPrefix(err.Error(), want) {
 		t.Errorf("Validate: %v, must start with %s", err, want)

go/vt/vtgate/vschema_stats.go

@@ -31,11 +31,12 @@ type VSchemaStats struct {
 // VSchemaKeyspaceStats contains a rollup of the VSchema stats for a keyspace.
 // It is used to display a table with the information in the status page.
 type VSchemaKeyspaceStats struct {
-	Keyspace    string
-	Sharded     bool
-	TableCount  int
-	VindexCount int
-	Error       string
+	Keyspace                 string
+	Sharded                  bool
+	TableCount               int
+	VindexCount              int
+	VindexUnknownParamsCount int
+	Error                    string
 }
 
 // NewVSchemaStats returns a new VSchemaStats from a VSchema.
@@ -54,6 +55,11 @@ func NewVSchemaStats(vschema *vindexes.VSchema, errorMessage string) *VSchemaSta
 			for _, t := range k.Tables {
 				s.VindexCount += len(t.ColumnVindexes) + len(t.Ordered) + len(t.Owned)
 			}
+			for _, vdx := range k.Vindexes {
+				if pv, ok := vdx.(vindexes.ParamValidating); ok {
+					s.VindexUnknownParamsCount += len(pv.UnknownParams())
+				}
+			}
 		}
 		if k.Error != nil {
 			s.Error = k.Error.Error()
@@ -95,13 +101,15 @@ const (
     <th>Sharded</th>
     <th>Table Count</th>
     <th>Vindex Count</th>
+    <th>Vindex Unknown Parameters Count</th>
     <th>Error</th>
   </tr>
 {{range $i, $ks := .Keyspaces}}  <tr>
     <td>{{$ks.Keyspace}}</td>
     <td>{{if $ks.Sharded}}Yes{{else}}No{{end}}</td>
     <td>{{$ks.TableCount}}</td>
     <td>{{$ks.VindexCount}}</td>
+    <td>{{$ks.VindexUnknownParamsCount}}</td>
     <td style="color:red">{{$ks.Error}}</td>
   </tr>{{end}}
 </table>