added topo plan classification and added more granular classification…

… for Set and Show Execution commands Signed-off-by: Harshit Gangal <[email protected]>
vitessio · Feb 13, 2025 · a3fdcf7 · a3fdcf7
1 parent edc12c3
commit a3fdcf7
Show file tree

Hide file tree

Showing 7 changed files with 85 additions and 38 deletions.
diff --git a/go/test/endtoend/vtgate/main_test.go b/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 {

diff --git a/go/test/endtoend/vtgate/misc_test.go b/go/test/endtoend/vtgate/misc_test.go
@@ -955,11 +955,23 @@ func TestQueryProcessedMetric(t *testing.T) {
 	}, {
 		sql:    "savepoint a",
 		metric: "SAVEPOINT.Transaction.PRIMARY",
-		shards: 0,
 	}, {
 		sql:    "rollback",
 		metric: "ROLLBACK.Transaction.PRIMARY",
-		shards: 0,
+	}, {
+		sql:    "set @x=3",
+		metric: "SET.Local.PRIMARY",
+	}, {
+		sql:    "set sql_mode=''",
+		metric: "SET.MultiShard.PRIMARY",
+		shards: 1,
+	}, {
+		sql:    "set @@vitess_metadata.k1='v1'",
+		metric: "SET.Topology.PRIMARY",
+	}, {
+		sql:    "select 1 from t1 a, t1 b",
+		metric: "SELECT.Join.PRIMARY",
+		shards: 3,
 	}}
 
 	initialQP := getQPMetric(t, "QueryProcessed")
@@ -971,6 +983,7 @@ func TestQueryProcessedMetric(t *testing.T) {
 			updatedQR := getQPMetric(t, "QueryRouted")
 			assert.EqualValues(t, 1, getValue(updatedQP, tc.metric)-getValue(initialQP, tc.metric))
 			assert.EqualValues(t, tc.shards, getValue(updatedQR, tc.metric)-getValue(initialQR, tc.metric))
+			initialQP, initialQR = updatedQP, updatedQR
 		})
 	}
 }

diff --git a/go/vt/vtgate/engine/plan.go b/go/vt/vtgate/engine/plan.go
@@ -67,6 +67,7 @@ const (
 	PlanOnlineDDL
 	PlanDirectDDL
 	PlanTransaction
+	PlanTopoOp
 )
 
 func higher(a, b PlanType) PlanType {
@@ -156,6 +157,8 @@ func (p PlanType) String() string {
 		return "DirectDDL"
 	case PlanTransaction:
 		return "Transaction"
+	case PlanTopoOp:
+		return "Topology"
 	default:
 		return "Unknown"
 	}
@@ -165,7 +168,7 @@ func getPlanType(p Primitive) PlanType {
 	switch prim := p.(type) {
 	case *SessionPrimitive, *SingleRow, *UpdateTarget, *VindexFunc:
 		return PlanLocal
-	case *Lock, *ReplaceVariables, *RevertMigration, *Rows, *ShowExec:
+	case *Lock, *ReplaceVariables, *RevertMigration, *Rows:
 		return PlanPassthrough
 	case *Send:
 		return getPlanTypeFromTarget(prim)
@@ -203,6 +206,13 @@ func getPlanType(p Primitive) PlanType {
 			return PlanOnlineDDL
 		}
 		return PlanDirectDDL
+	case *ShowExec:
+		if prim.Command == sqlparser.VitessVariables {
+			return PlanTopoOp
+		}
+		return PlanLocal
+	case *Set:
+		return getPlanTypeForSet(prim)
 	case *VExplain:
 		return getPlanType(prim.Input)
 	case *RenameFields:
@@ -212,6 +222,28 @@ func getPlanType(p Primitive) PlanType {
 	}
 }
 
+func getPlanTypeForSet(prim *Set) (pt PlanType) {
+	for _, op := range prim.Ops {
+		pt = higher(pt, getPlanTypeForSetOp(op))
+	}
+	return pt
+}
+
+func getPlanTypeForSetOp(op SetOp) PlanType {
+	switch op.(type) {
+	case *UserDefinedVariable, *SysVarIgnore, *SysVarSetAware:
+		return PlanLocal
+	case *SysVarCheckAndIgnore:
+		return PlanPassthrough
+	case *SysVarReservedConn:
+		return PlanMultiShard
+	case *VitessMetadata:
+		return PlanTopoOp
+	default:
+		return PlanUnknown
+	}
+}
+
 func getPlanTypeFromTarget(prim *Send) PlanType {
 	switch prim.TargetDestination.(type) {
 	case key.DestinationNone:

diff --git a/go/vt/vtgate/planbuilder/testdata/onecase.json b/go/vt/vtgate/planbuilder/testdata/onecase.json
@@ -1,7 +1,7 @@
 [
   {
     "comment": "Add your test case here for debugging and run go test -run=One.",
-    "query": "",
+    "query": "select 1 from user a, user b",
     "plan": {
     }
   }

diff --git a/go/vt/vtgate/planbuilder/testdata/set_cases.json b/go/vt/vtgate/planbuilder/testdata/set_cases.json
@@ -3,7 +3,7 @@
     "comment": "set single user defined variable",
     "query": "set @foo = 42",
     "plan": {
-      "Type": "Complex",
+      "Type": "Local",
       "QueryType": "SET",
       "Original": "set @foo = 42",
       "Instructions": {
@@ -27,7 +27,7 @@
     "comment": "set multi user defined variable",
     "query": "set @foo = 42, @bar = @foo",
     "plan": {
-      "Type": "Complex",
+      "Type": "Local",
       "QueryType": "SET",
       "Original": "set @foo = 42, @bar = @foo",
       "Instructions": {
@@ -56,7 +56,7 @@
     "comment": "set multi user defined variable with complex expression",
     "query": "set @foo = 42, @bar = @foo + 1",
     "plan": {
-      "Type": "Complex",
+      "Type": "Local",
       "QueryType": "SET",
       "Original": "set @foo = 42, @bar = @foo + 1",
       "Instructions": {
@@ -85,7 +85,7 @@
     "comment": "set UDV to expression that can't be evaluated at vtgate",
     "query": "set @foo = SOUNDEX('Hello')",
     "plan": {
-      "Type": "Complex",
+      "Type": "Local",
       "QueryType": "SET",
       "Original": "set @foo = SOUNDEX('Hello')",
       "Instructions": {
@@ -116,7 +116,7 @@
     "comment": "single sysvar cases",
     "query": "SET sql_mode = 'STRICT_ALL_TABLES,NO_AUTO_VALUE_ON_ZERO'",
     "plan": {
-      "Type": "Complex",
+      "Type": "MultiShard",
       "QueryType": "SET",
       "Original": "SET sql_mode = 'STRICT_ALL_TABLES,NO_AUTO_VALUE_ON_ZERO'",
       "Instructions": {
@@ -145,7 +145,7 @@
     "comment": "multiple sysvar cases",
     "query": "SET @@SESSION.sql_mode = CONCAT(CONCAT(@@sql_mode, ',STRICT_ALL_TABLES'), ',NO_AUTO_VALUE_ON_ZERO'), @@SESSION.sql_safe_updates = 0",
     "plan": {
-      "Type": "Complex",
+      "Type": "MultiShard",
       "QueryType": "SET",
       "Original": "SET @@SESSION.sql_mode = CONCAT(CONCAT(@@sql_mode, ',STRICT_ALL_TABLES'), ',NO_AUTO_VALUE_ON_ZERO'), @@SESSION.sql_safe_updates = 0",
       "Instructions": {
@@ -184,7 +184,7 @@
     "comment": "autocommit case",
     "query": "SET autocommit = 1, autocommit = on, autocommit = 'on', autocommit = @myudv, autocommit = `on`, autocommit = `off`",
     "plan": {
-      "Type": "Complex",
+      "Type": "Local",
       "QueryType": "SET",
       "Original": "SET autocommit = 1, autocommit = on, autocommit = 'on', autocommit = @myudv, autocommit = `on`, autocommit = `off`",
       "Instructions": {
@@ -233,7 +233,7 @@
     "comment": "set ignore plan",
     "query": "set @@default_storage_engine = 'DONOTCHANGEME'",
     "plan": {
-      "Type": "Complex",
+      "Type": "Local",
       "QueryType": "SET",
       "Original": "set @@default_storage_engine = 'DONOTCHANGEME'",
       "Instructions": {
@@ -257,7 +257,7 @@
     "comment": "set check and ignore plan",
     "query": "set @@sql_mode = concat(@@sql_mode, ',NO_AUTO_CREATE_USER')",
     "plan": {
-      "Type": "Complex",
+      "Type": "MultiShard",
       "QueryType": "SET",
       "Original": "set @@sql_mode = concat(@@sql_mode, ',NO_AUTO_CREATE_USER')",
       "Instructions": {
@@ -286,7 +286,7 @@
     "comment": "set system settings",
     "query": "set @@sql_safe_updates = 1",
     "plan": {
-      "Type": "Complex",
+      "Type": "MultiShard",
       "QueryType": "SET",
       "Original": "set @@sql_safe_updates = 1",
       "Instructions": {
@@ -315,7 +315,7 @@
     "comment": "set plan building with ON/OFF enum",
     "query": "set @@innodb_strict_mode = OFF",
     "plan": {
-      "Type": "Complex",
+      "Type": "Local",
       "QueryType": "SET",
       "Original": "set @@innodb_strict_mode = OFF",
       "Instructions": {
@@ -339,7 +339,7 @@
     "comment": "set plan building with string literal",
     "query": "set @@innodb_strict_mode = 'OFF'",
     "plan": {
-      "Type": "Complex",
+      "Type": "Local",
       "QueryType": "SET",
       "Original": "set @@innodb_strict_mode = 'OFF'",
       "Instructions": {
@@ -363,7 +363,7 @@
     "comment": "set plan building with string literal",
     "query": "set @@innodb_tmpdir = 'OFF'",
     "plan": {
-      "Type": "Complex",
+      "Type": "Local",
       "QueryType": "SET",
       "Original": "set @@innodb_tmpdir = 'OFF'",
       "Instructions": {
@@ -392,7 +392,7 @@
     "comment": "set autocommit",
     "query": "set autocommit = 1",
     "plan": {
-      "Type": "Complex",
+      "Type": "Local",
       "QueryType": "SET",
       "Original": "set autocommit = 1",
       "Instructions": {
@@ -416,7 +416,7 @@
     "comment": "set autocommit false",
     "query": "set autocommit = 0",
     "plan": {
-      "Type": "Complex",
+      "Type": "Local",
       "QueryType": "SET",
       "Original": "set autocommit = 0",
       "Instructions": {
@@ -440,7 +440,7 @@
     "comment": "set autocommit with backticks",
     "query": "set @@session.`autocommit` = 0",
     "plan": {
-      "Type": "Complex",
+      "Type": "Local",
       "QueryType": "SET",
       "Original": "set @@session.`autocommit` = 0",
       "Instructions": {
@@ -464,7 +464,7 @@
     "comment": "more vitess aware settings",
     "query": "set client_found_rows = off, skip_query_plan_cache = ON, sql_select_limit=20",
     "plan": {
-      "Type": "Complex",
+      "Type": "Local",
       "QueryType": "SET",
       "Original": "set client_found_rows = off, skip_query_plan_cache = ON, sql_select_limit=20",
       "Instructions": {
@@ -498,7 +498,7 @@
     "comment": "set autocommit to default",
     "query": "set @@autocommit = default",
     "plan": {
-      "Type": "Complex",
+      "Type": "Local",
       "QueryType": "SET",
       "Original": "set @@autocommit = default",
       "Instructions": {
@@ -522,7 +522,7 @@
     "comment": "set global autocommit to default",
     "query": "set global autocommit = off",
     "plan": {
-      "Type": "Complex",
+      "Type": "Passthrough",
       "QueryType": "SET",
       "Original": "set global autocommit = off",
       "Instructions": {
@@ -556,7 +556,7 @@
     "comment": "set transaction read only",
     "query": "set session transaction read only",
     "plan": {
-      "Type": "Complex",
+      "Type": "Local",
       "QueryType": "SET",
       "Original": "set session transaction read only",
       "Instructions": {
@@ -580,7 +580,7 @@
     "comment": "set transaction isolation level",
     "query": "set transaction isolation level read committed",
     "plan": {
-      "Type": "Complex",
+      "Type": "MultiShard",
       "QueryType": "SET",
       "Original": "set transaction isolation level read committed",
       "Instructions": {
@@ -609,7 +609,7 @@
     "comment": "set vitess_metadata",
     "query": "set @@vitess_metadata.app_v1= '1'",
     "plan": {
-      "Type": "Complex",
+      "Type": "Topology",
       "QueryType": "SET",
       "Original": "set @@vitess_metadata.app_v1= '1'",
       "Instructions": {
@@ -632,7 +632,7 @@
     "comment": "set last_insert_id with agrument to user defined variable",
     "query": "set @foo = last_insert_id(1)",
     "plan": {
-      "Type": "Complex",
+      "Type": "Local",
       "QueryType": "SET",
       "Original": "set @foo = last_insert_id(1)",
       "Instructions": {

diff --git a/go/vt/vtgate/planbuilder/testdata/set_sysvar_disabled_cases.json b/go/vt/vtgate/planbuilder/testdata/set_sysvar_disabled_cases.json
@@ -3,7 +3,7 @@
     "comment": "set passthrough disabled - check and ignore plan",
     "query": "set @@sql_mode = concat(@@sql_mode, ',NO_AUTO_CREATE_USER'), @@sql_safe_updates = 1",
     "plan": {
-      "Type": "Complex",
+      "Type": "Passthrough",
       "QueryType": "SET",
       "Original": "set @@sql_mode = concat(@@sql_mode, ',NO_AUTO_CREATE_USER'), @@sql_safe_updates = 1",
       "Instructions": {