diff --git a/changelog/18.0/18.0.0/summary.md b/changelog/18.0/18.0.0/summary.md index 2171e02b31c..66202167d43 100644 --- a/changelog/18.0/18.0.0/summary.md +++ b/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) ## Major Changes @@ -38,4 +40,10 @@ The `k8stopo` has been deprecated in Vitess 17, also see https://github.com/vite #### 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. \ No newline at end of file +The `vtgr` has been deprecated in Vitess 17, also see https://github.com/vitessio/vitess/issues/13300. With Vitess 18 `vtgr` has been removed. + +### New stats + +#### VTGate Vindex unknown parameters + +The VTGate stat `VindexUnknownParameters` gauges unknown Vindex parameters found in the latest VSchema pulled from the topology. diff --git a/go/vt/topo/vschema.go b/go/vt/topo/vschema.go index a2503673deb..c167475a462 100644 --- a/go/vt/topo/vschema.go +++ b/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 } diff --git a/go/vt/vtctl/testdata/unknown-params-logged-dry-run-vschema.json b/go/vt/vtctl/testdata/unknown-params-logged-dry-run-vschema.json new file mode 100644 index 00000000000..aefcfb13ae7 --- /dev/null +++ b/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" + } + } + } +} diff --git a/go/vt/vtctl/testdata/unknown-params-logged-vschema.json b/go/vt/vtctl/testdata/unknown-params-logged-vschema.json new file mode 100644 index 00000000000..d3abc1c0e03 --- /dev/null +++ b/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" + } + } + } +} diff --git a/go/vt/vtctl/vtctl.go b/go/vt/vtctl/vtctl.go index 14412902ef5..70ae6c90832 100644 --- a/go/vt/vtctl/vtctl.go +++ b/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 diff --git a/go/vt/vtctl/vtctl_test.go b/go/vt/vtctl/vtctl_test.go index ab2d7786a4b..5424c124382 100644 --- a/go/vt/vtctl/vtctl_test.go +++ b/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 diff --git a/go/vt/vtgate/executor.go b/go/vt/vtgate/executor.go index 644b1977ed9..a59aa49c525 100644 --- a/go/vt/vtgate/executor.go +++ b/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. diff --git a/go/vt/vtgate/vindexes/binarymd5.go b/go/vt/vtgate/vindexes/binarymd5.go index 2dcaefb7a0d..d3495e28deb 100644 --- a/go/vt/vtgate/vindexes/binarymd5.go +++ b/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 { diff --git a/go/vt/vtgate/vindexes/cached_size.go b/go/vt/vtgate/vindexes/cached_size.go index 492180e2d09..c27c5f33485 100644 --- a/go/vt/vtgate/vindexes/cached_size.go +++ b/go/vt/vtgate/vindexes/cached_size.go @@ -62,31 +62,21 @@ func (cached *Binary) CachedSize(alloc bool) int64 { } return size } - -//go:nocheckptr func (cached *BinaryMD5) CachedSize(alloc bool) int64 { if cached == nil { return int64(0) } size := int64(0) if alloc { - size += int64(24) + size += int64(48) } // field name string size += hack.RuntimeAllocSize(int64(len(cached.name))) - // field params map[string]string - if cached.params != nil { - size += int64(48) - hmap := reflect.ValueOf(cached.params) - numBuckets := int(math.Pow(2, float64((*(*uint8)(unsafe.Pointer(hmap.Pointer() + uintptr(9))))))) - numOldBuckets := (*(*uint16)(unsafe.Pointer(hmap.Pointer() + uintptr(10)))) - size += hack.RuntimeAllocSize(int64(numOldBuckets * 272)) - if len(cached.params) > 0 || numBuckets > 1 { - size += hack.RuntimeAllocSize(int64(numBuckets * 272)) - } - for k, v := range cached.params { - size += hack.RuntimeAllocSize(int64(len(k))) - size += hack.RuntimeAllocSize(int64(len(v))) + // field unknownParams []string + { + size += hack.RuntimeAllocSize(int64(cap(cached.unknownParams)) * int64(16)) + for _, elem := range cached.unknownParams { + size += hack.RuntimeAllocSize(int64(len(elem))) } } return size diff --git a/go/vt/vtgate/vindexes/vschema.go b/go/vt/vtgate/vindexes/vschema.go index dce0ec81690..fd8f515e1c7 100644 --- a/go/vt/vtgate/vindexes/vschema.go +++ b/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) { diff --git a/go/vt/vtgate/vindexes/vschema_test.go b/go/vt/vtgate/vindexes/vschema_test.go index 335b42560cb..63d05767c82 100644 --- a/go/vt/vtgate/vindexes/vschema_test.go +++ b/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) diff --git a/go/vt/vtgate/vschema_stats.go b/go/vt/vtgate/vschema_stats.go index ce234fdba9a..d4920d7486f 100644 --- a/go/vt/vtgate/vschema_stats.go +++ b/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,6 +101,7 @@ const ( Sharded Table Count Vindex Count + Vindex Unknown Parameters Count Error {{range $i, $ks := .Keyspaces}} @@ -102,6 +109,7 @@ const ( {{if $ks.Sharded}}Yes{{else}}No{{end}} {{$ks.TableCount}} {{$ks.VindexCount}} + {{$ks.VindexUnknownParamsCount}} {{$ks.Error}} {{end}} diff --git a/go/vt/vtgate/vtgate.go b/go/vt/vtgate/vtgate.go index 02b4dc2ef26..8f716385ff5 100644 --- a/go/vt/vtgate/vtgate.go +++ b/go/vt/vtgate/vtgate.go @@ -186,6 +186,8 @@ var ( vstreamSkewDelayCount = stats.NewCounter("VStreamEventsDelayedBySkewAlignment", "Number of events that had to wait because the skew across shards was too high") + + vindexUnknownParams = stats.NewGauge("VindexUnknownParameters", "Number of parameterss unrecognized by Vindexes") ) // VTGate is the rpc interface to vtgate. Only one instance diff --git a/go/vt/vtgate/vtgate_test.go b/go/vt/vtgate/vtgate_test.go index 74fef53f8b3..94451706117 100644 --- a/go/vt/vtgate/vtgate_test.go +++ b/go/vt/vtgate/vtgate_test.go @@ -644,6 +644,37 @@ var shardedVSchema = ` } ` +var shardedVSchemaUnknownParams = ` +{ + "sharded": true, + "vindexes": { + "hash_index": { + "type": "hash", + "params": { + "hello": "world", + "goodbye": "world" + } + }, + "binary_index": { + "type": "binary", + "params": { + "foo": "bar" + } + } + }, + "tables": { + "sp_tbl": { + "column_vindexes": [ + { + "column": "user_id", + "name": "hash_index" + } + ] + } + } +} +` + func TestMultiInternalSavepointVtGate(t *testing.T) { s := createSandbox(KsTestSharded) s.ShardSpec = "-40-80-" @@ -736,3 +767,30 @@ func TestMultiInternalSavepointVtGate(t *testing.T) { testQueryLog(t, logChan, "Execute", "INSERT", "insert into sp_tbl(user_id) values (:vtg1 /* INT64 */), (:vtg2 /* INT64 */)", 2) testQueryLog(t, logChan, "Execute", "INSERT", "insert into sp_tbl(user_id) values (:vtg1 /* INT64 */)", 1) } + +func TestVSchemaVindexUnknownParams(t *testing.T) { + s := createSandbox(KsTestSharded) + s.ShardSpec = "-40-80-" + + s.VSchema = shardedVSchema + srvSchema := getSandboxSrvVSchema() + rpcVTGate.executor.vm.VSchemaUpdate(srvSchema, nil) + hcVTGateTest.Reset() + + unknownParams := vindexUnknownParams.Get() + require.Equal(t, int64(0), unknownParams) + + s.VSchema = shardedVSchemaUnknownParams + srvSchema = getSandboxSrvVSchema() + rpcVTGate.executor.vm.VSchemaUpdate(srvSchema, nil) + + unknownParams = vindexUnknownParams.Get() + require.Equal(t, int64(3), unknownParams) + + s.VSchema = shardedVSchema + srvSchema = getSandboxSrvVSchema() + rpcVTGate.executor.vm.VSchemaUpdate(srvSchema, nil) + + unknownParams = vindexUnknownParams.Get() + require.Equal(t, int64(0), unknownParams) +}