Merge remote-tracking branch 'origin/main' into improved-errors-durin…

…g-prs

Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>

.github/CODEOWNERS

@@ -17,9 +17,9 @@ go.sum @deepthi @harshit-gangal @mattlord @rohit-nayak-ps @systay @frouioui
 /go/cache @vmg
 /go/cmd @deepthi @mattlord
 /go/cmd/vtadmin @notfelineit
-/go/cmd/vtctldclient @mattlord
+/go/cmd/vtctldclient @mattlord @notfelineit
 /go/cmd/vtctldclient/command/throttler.go @shlomi-noach @mattlord
-/go/cmd/vtctldclient/command/vreplication @mattlord @rohit-nayak-ps
+/go/cmd/vtctldclient/command/vreplication @mattlord @rohit-nayak-ps @shlomi-noach @notfelineit
 /go/internal/flag @rohit-nayak-ps
 /go/mysql @harshit-gangal @systay @mattlord
 /go/pools @deepthi @harshit-gangal
@@ -59,33 +59,33 @@ go.sum @deepthi @harshit-gangal @mattlord @rohit-nayak-ps @systay @frouioui
 /go/vt/vterrors @harshit-gangal @systay
 /go/vt/vtexplain @systay @harshit-gangal
 /go/vt/vtgate @harshit-gangal @systay @frouioui @GuptaManan100
-/go/vt/vtgate/endtoend/*vstream* @rohit-nayak-ps @mattlord
+/go/vt/vtgate/endtoend/*vstream* @rohit-nayak-ps @mattlord @shlomi-noach @notfelineit
 /go/vt/vtgate/planbuilder @harshit-gangal @systay @frouioui @GuptaManan100 @arthurschreiber
-/go/vt/vtgate/*vstream* @rohit-nayak-ps @mattlord
+/go/vt/vtgate/*vstream* @rohit-nayak-ps @mattlord @shlomi-noach @notfelineit
 /go/vt/vtgate/evalengine @dbussink @vmg @systay
 /go/vt/vtorc @deepthi @shlomi-noach @GuptaManan100 @timvaillancourt
 /go/vt/vttablet/*conn* @harshit-gangal @systay
 /go/vt/vttablet/endtoend @harshit-gangal @mattlord @rohit-nayak-ps @systay
 /go/vt/vttablet/grpc* @rohit-nayak-ps @shlomi-noach @harshit-gangal
-/go/vt/vttablet/onlineddl @mattlord @rohit-nayak-ps @shlomi-noach
+/go/vt/vttablet/onlineddl @mattlord @rohit-nayak-ps @shlomi-noach @notfelineit
 /go/vt/vttablet/queryservice @harshit-gangal @systay
 /go/vt/vttablet/tabletmanager @deepthi @GuptaManan100 @rohit-nayak-ps @shlomi-noach
 /go/vt/vttablet/tabletmanager/rpc_throttler.go @shlomi-noach @mattlord @timvaillancourt
 /go/vt/vttablet/tabletserver/throttle @shlomi-noach @mattlord @timvaillancourt
-/go/vt/vttablet/tabletmanager/vreplication @rohit-nayak-ps @mattlord
-/go/vt/vttablet/tabletmanager/vdiff @rohit-nayak-ps @mattlord
-/go/vt/vttablet/tabletmanager/vstreamer @rohit-nayak-ps @mattlord
+/go/vt/vttablet/tabletmanager/vreplication @rohit-nayak-ps @mattlord @shlomi-noach @notfelineit
+/go/vt/vttablet/tabletmanager/vdiff @rohit-nayak-ps @mattlord @shlomi-noach @notfelineit
+/go/vt/vttablet/tabletmanager/vstreamer @rohit-nayak-ps @mattlord @shlomi-noach @notfelineit
 /go/vt/vttablet/tabletserver* @harshit-gangal @systay @shlomi-noach @rohit-nayak-ps @timvaillancourt
 /go/vt/vttablet/tabletserver/messager @mattlord @rohit-nayak-ps @derekperkins
 /go/vt/vttablet/*tmclient* @GuptaManan100 @rohit-nayak-ps @shlomi-noach
 /go/vt/vttablet/vexec @mattlord @rohit-nayak-ps @shlomi-noach
 /go/vt/wrangler @deepthi @mattlord @rohit-nayak-ps
-/go/vt/vtctl/workflow @mattlord @rohit-nayak-ps
+/go/vt/vtctl/workflow @mattlord @rohit-nayak-ps @shlomi-noach @notfelineit
 /proto/ @deepthi @harshit-gangal
 /proto/vtadmin.proto @notfelineit @mattlord
 /proto/vtctldata.proto @notfelineit @mattlord
 /proto/vtctlservice.proto @notfelineit @mattlord
 /test/ @GuptaManan100 @frouioui @rohit-nayak-ps @deepthi @mattlord @harshit-gangal
 /tools/ @frouioui @rohit-nayak-ps
 /web/vtadmin @notfelineit
-/web/vtadmin/src/proto @deepthi @harshit-gangal @mattlord
+/web/vtadmin/src/proto @deepthi @harshit-gangal @mattlord @notfelineit

changelog/22.0/22.0.0/summary.md

@@ -4,9 +4,11 @@
 
 - **[Major Changes](#major-changes)**
   - **[Deprecations and Deletions](#deprecations-and-deletions)**
+    - [Deprecated VTGate Metrics](#vtgate-metrics)
     - [Deprecated VTTablet Flags](#vttablet-flags)
     - [Removing gh-ost and pt-osc Online DDL strategies](#ghost-ptosc)
   - **[RPC Changes](#rpc-changes)**
+  - **[VTGate Metrics]**(#vtgate-metrics)
   - **[Prefer not promoting a replica that is currently taking a backup](#reparents-prefer-not-backing-up)**
   - **[VTOrc Config File Changes](#vtorc-config-file-changes)**
   - **[VTGate Config File Changes](#vtgate-config-file-changes)**
@@ -55,6 +57,33 @@ $ vtctldclient ApplySchema --ddl-strategy="gh-ost" ...
 $ vtctldclient ApplySchema --ddl-strategy="pt-osc" ...
 ```
 
+### <a id="vtgate-metrics"/>VTGate Metrics
+
+#### New Metrics Added:
+Three new metrics have been introduced for queries:
+1.	`QueryExecutions` – Counts the number of queries executed. **Dimensions:** Query type, Plan type, Tablet type.
+2.	`QueryRoutes` – Counts the number of vttablets the query was executed on. **Dimensions:** Query type, Plan type, Tablet type.
+3.	`QueryExecutionsByTable` – Tracks queries executed at VTGate, with counts recorded per table. **Dimensions:** Query type, Table. 
+
+Example: 
+```
+Query: select t1.a, t2.b from t1 join t2 on t1.id = t2.id
+Shards: 2
+Sharding Key: id for both tables
+
+Metrics Published:
+1. QueryExecutions – {select, scatter, primary}, 1
+2. QueryRoutes – {select, scatter, primary}, 2
+3. QueryExecutionsByTable – {select, t1}, 1 and {select, t2}, 1
+```
+
+#### Deprecated Metrics:
+The following metrics have been deprecated:
+1.	`QueriesProcessed`
+2.	`QueriesRouted`
+3.	`QueriesProcessedByTable`
+4.	`QueriesRoutedByTable`
+
 ### <a id="reparents-prefer-not-backing-up"/>Prefer not promoting a replica that is currently taking a backup
 
 Emergency reparents now prefer not promoting replicas that are currently taking backups with a backup engine other than

go/flags/endtoend/vtorc.txt

@@ -21,6 +21,8 @@ Flags:
       --audit-purge-duration duration                               Duration for which audit logs are held before being purged. Should be in multiples of days (default 168h0m0s)
       --audit-to-backend                                            Whether to store the audit log in the VTOrc database
       --audit-to-syslog                                             Whether to store the audit log in the syslog
+      --backend-read-concurrency int                                Maximum concurrency for reads to the backend (default 32)
+      --backend-write-concurrency int                               Maximum concurrency for writes to the backend (default 24)
       --bind-address string                                         Bind address for the server. If empty, the server will listen on all available unicast and anycast IP addresses of the local system.
       --catch-sigpipe                                               catch and ignore SIGPIPE on stdout and stderr if specified
       --change-tablets-with-errant-gtid-to-drained                  Whether VTOrc should be changing the type of tablets with errant GTIDs to DRAINED

go/test/endtoend/vtgate/main_test.go

@@ -71,8 +71,9 @@ 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-max-result-size", "100", "--queryserver-config-terse-errors"}
+		clusterInstance.VtTabletExtraArgs = append(clusterInstance.VtTabletExtraArgs,
+			"--queryserver-config-max-result-size", "100",
+			"--queryserver-config-terse-errors")
 		err = clusterInstance.StartKeyspace(*keyspace, []string{"-80", "80-"}, 0, false)
 		if err != nil {
 			return 1
@@ -88,7 +89,8 @@ func TestMain(m *testing.M) {
 			return 1
 		}
 
-		clusterInstance.VtGateExtraArgs = append(clusterInstance.VtGateExtraArgs, "--enable_system_settings=true")
+		clusterInstance.VtGateExtraArgs = append(clusterInstance.VtGateExtraArgs,
+			"--vschema_ddl_authorized_users", "%")
 		// Start vtgate
 		err = clusterInstance.StartVtgate()
 		if err != nil {

go/test/endtoend/vtgate/misc_test.go

@@ -926,3 +926,116 @@ func getVar(t *testing.T, key string) interface{} {
 	}
 	return val
 }
+
+// TestQueryProcessedMetric verifies that query metrics are correctly published.
+func TestQueryProcessedMetric(t *testing.T) {
+	conn, closer := start(t)
+	defer closer()
+
+	tcases := []struct {
+		sql          string
+		queryMetric  string
+		tableMetrics []string
+		shards       int
+	}{{
+		sql:          "select id1, id2 from t1",
+		queryMetric:  "SELECT.Scatter.PRIMARY",
+		shards:       2,
+		tableMetrics: []string{"SELECT.ks_t1"},
+	}, {
+		sql:          "update t1 set id2 = 2 where id1 = 1",
+		queryMetric:  "UPDATE.MultiShard.PRIMARY",
+		shards:       2,
+		tableMetrics: []string{"UPDATE.ks_t1"},
+	}, {
+		sql:          "delete from t1 where id1 in (1)",
+		queryMetric:  "DELETE.MultiShard.PRIMARY",
+		shards:       2,
+		tableMetrics: []string{"DELETE.ks_t1"},
+	}, {
+		sql:         "show tables",
+		queryMetric: "SHOW.Passthrough.PRIMARY",
+		shards:      1,
+	}, {
+		sql:         "savepoint a",
+		queryMetric: "SAVEPOINT.Transaction.PRIMARY",
+	}, {
+		sql:         "rollback",
+		queryMetric: "ROLLBACK.Transaction.PRIMARY",
+	}, {
+		sql:         "set @x=3",
+		queryMetric: "SET.Local.PRIMARY",
+	}, {
+		sql:         "set sql_mode=''",
+		queryMetric: "SET.MultiShard.PRIMARY",
+		shards:      1,
+	}, {
+		sql:         "set @@vitess_metadata.k1='v1'",
+		queryMetric: "SET.Topology.PRIMARY",
+	}, {
+		sql:          "select 1 from t1 a, t1 b",
+		queryMetric:  "SELECT.Join.PRIMARY",
+		shards:       3,
+		tableMetrics: []string{"SELECT.ks_t1"},
+	}, {
+		sql:          "select count(*) from t1 a, t1 b",
+		queryMetric:  "SELECT.Complex.PRIMARY",
+		shards:       6,
+		tableMetrics: []string{"SELECT.ks_t1"},
+	}, {
+		sql:          "select 1 from t1, t2, t3, t4 where t1.id1 = t2.id3 and t2.id3 = t3.id6 and t3.id6 = t4.id1 and t3.id7 = 5",
+		queryMetric:  "SELECT.Lookup.PRIMARY",
+		shards:       2,
+		tableMetrics: []string{"SELECT.ks_t1", "SELECT.ks_t2", "SELECT.ks_t3", "SELECT.ks_t4"},
+	}}
+
+	initialQP := getQPMetric(t, "QueryExecutions")
+	initialQR := getQPMetric(t, "QueryRoutes")
+	initialQT := getQPMetric(t, "QueryExecutionsByTable")
+	for _, tc := range tcases {
+		t.Run(tc.sql, func(t *testing.T) {
+			utils.Exec(t, conn, tc.sql)
+			updatedQP := getQPMetric(t, "QueryExecutions")
+			updatedQR := getQPMetric(t, "QueryRoutes")
+			updatedQT := getQPMetric(t, "QueryExecutionsByTable")
+			assert.EqualValuesf(t, 1, getValue(updatedQP, tc.queryMetric)-getValue(initialQP, tc.queryMetric), "queryExecutions metric: %s", tc.queryMetric)
+			assert.EqualValuesf(t, tc.shards, getValue(updatedQR, tc.queryMetric)-getValue(initialQR, tc.queryMetric), "queryRoutes metric: %s", tc.queryMetric)
+			for _, metric := range tc.tableMetrics {
+				assert.EqualValuesf(t, 1, getValue(updatedQT, metric)-getValue(initialQT, metric), "queryExecutionsByTable metric: %s", metric)
+			}
+			initialQP, initialQR, initialQT = updatedQP, updatedQR, updatedQT
+		})
+	}
+}
+
+func getQPMetric(t *testing.T, metric string) map[string]any {
+	t.Helper()
+
+	vars := clusterInstance.VtgateProcess.GetVars()
+	require.NotNil(t, vars)
+
+	qpVars, exists := vars[metric]
+	if !exists {
+		return nil
+	}
+
+	qpMap, ok := qpVars.(map[string]any)
+	require.True(t, ok, "query queryMetric vars is not a map")
+
+	return qpMap
+}
+
+func getValue(m map[string]any, key string) float64 {
+	if m == nil {
+		return 0
+	}
+	val, exists := m[key]
+	if !exists {
+		return 0
+	}
+	f, ok := val.(float64)
+	if !ok {
+		return 0
+	}
+	return f
+}

go/tools/asthelpergen/asthelpergen.go

@@ -110,6 +110,10 @@ func findImplementations(scope *types.Scope, iff *types.Interface, impl func(typ
 					panic(fmt.Errorf("interface %s implemented by %s (%s as %T) without ptr", iff.String(), baseType, tt.String(), tt))
 				}
 			}
+			if types.TypeString(baseType, noQualifier) == visitableName {
+				// skip the visitable interface
+				continue
+			}
 			if err := impl(baseType); err != nil {
 				return err
 			}
@@ -211,8 +215,8 @@ func GenerateASTHelpers(options *Options) (map[string]*jen.File, error) {
 		newEqualsGen(pName, &options.Equals),
 		newCloneGen(pName, &options.Clone),
 		newPathGen(pName, ifaceName),
-		newVisitGen(pName),
-		newRewriterGen(pName, types.TypeString(nt, noQualifier)),
+		newVisitGen(pName, ifaceName),
+		newRewriterGen(pName, ifaceName),
 		newCOWGen(pName, nt),
 	)
 

go/tools/asthelpergen/copy_on_rewrite_gen.go

@@ -45,6 +45,7 @@ func (c *cowGen) addFunc(code *jen.Statement) {
 }
 
 func (c *cowGen) genFile(generatorSPI) (string, *jen.File) {
+	c.genVisitableMethod()
 	return "ast_copy_on_rewrite.go", c.file
 }
 
@@ -192,6 +193,9 @@ func (c *cowGen) interfaceMethod(t types.Type, iface *types.Interface, spi gener
 	})
 
 	cases = append(cases,
+		jen.Case(jen.Id(visitableName)).Block(
+			jen.Return(jen.Id("c."+cowName+visitableName).Call(jen.Id("n, parent"))),
+		),
 		jen.Default().Block(
 			jen.Comment("this should never happen"),
 			jen.Return(jen.Nil(), jen.False()),
@@ -368,6 +372,62 @@ func (c *cowGen) visitStruct(t types.Type, strct *types.Struct, spi generatorSPI
 	c.addFunc(funcDeclaration.Block(stmts...))
 }
 
+func (c *cowGen) genVisitableMethod() {
+	c.file.Func().
+		Params(jen.Id("c").Op("*").Id("cow")).
+		Id("copyOnRewriteVisitable").
+		Params(
+			jen.Id("n").Id("Visitable"),
+			jen.Id("parent").Id(c.baseType),
+		).
+		Params(
+			jen.Id("out").Id(c.baseType),
+			jen.Id("changed").Bool(),
+		).
+		Block(
+			// if c.cursor.stop { return n, false }
+			jen.If(jen.Id("c").Dot("cursor").Dot("stop")).Block(
+				jen.Return(jen.Id("n"), jen.False()),
+			),
+
+			// out = n
+			jen.Id("out").Op("=").Id("n"),
+
+			// if c.pre == nil || c.pre(n, parent) { ... }
+			jen.If(
+				jen.Id("c").Dot("pre").Op("==").Nil().Op("||").
+					Id("c").Dot("pre").Call(jen.Id("n"), jen.Id("parent")),
+			).Block(
+				// _inner, changedInner := c.copyOnRewriteAST(n.VisitThis(), n)
+				jen.List(jen.Id("_inner"), jen.Id("changedInner")).
+					Op(":=").
+					Id("c").Dot("copyOnRewrite"+c.baseType).
+					Call(
+						jen.Id("n").Dot("VisitThis").Call(),
+						jen.Id("n"),
+					),
+
+				// if changedInner { res := n.Clone(_inner); out = res; changed = true }
+				jen.If(jen.Id("changedInner")).Block(
+					jen.Id("res").Op(":=").Id("n").Dot("Clone").Call(jen.Id("_inner")),
+					jen.Id("out").Op("=").Id("res"),
+					jen.Id("changed").Op("=").True(),
+				),
+			),
+
+			// if c.post != nil { out, changed = c.postVisit(out, parent, changed) }
+			jen.If(jen.Id("c").Dot("post").Op("!=").Nil()).Block(
+				jen.List(jen.Id("out"), jen.Id("changed")).
+					Op("=").
+					Id("c").Dot("postVisit").
+					Call(jen.Id("out"), jen.Id("parent"), jen.Id("changed")),
+			),
+
+			// return
+			jen.Return(),
+		)
+}
+
 func ifPostNotNilVisit(out string) *jen.Statement {
 	return ifNotNil("c.post", jen.List(jen.Id(out), jen.Id("changed")).Op("=").Id("c").Dot("postVisit").Params(jen.Id(out), jen.Id("parent"), jen.Id("changed")))
 }