From 0a82dec379beedc21920d8c614026d3ac15d689c Mon Sep 17 00:00:00 2001 From: Nick Van Wiggeren Date: Sat, 25 Oct 2025 17:16:47 +0200 Subject: [PATCH 01/14] initial commit: pick a tablet Signed-off-by: Nick Van Wiggeren --- go/test/vschemawrapper/vschema_wrapper.go | 8 ++-- go/vt/topo/topoproto/destination.go | 39 +++++++++++++++----- go/vt/topo/topoproto/destination_test.go | 8 ++-- go/vt/vtgate/executorcontext/safe_session.go | 21 +++++++++++ go/vt/vtgate/executorcontext/vcursor_impl.go | 19 +++++++++- go/vt/vtgate/scatter_conn.go | 11 ++++++ go/vtbench/client.go | 2 +- 7 files changed, 88 insertions(+), 20 deletions(-) diff --git a/go/test/vschemawrapper/vschema_wrapper.go b/go/test/vschemawrapper/vschema_wrapper.go index 4a526b434a6..df8bb0b87b4 100644 --- a/go/test/vschemawrapper/vschema_wrapper.go +++ b/go/test/vschemawrapper/vschema_wrapper.go @@ -236,7 +236,7 @@ func (vw *VSchemaWrapper) ShardDestination() key.ShardDestination { } func (vw *VSchemaWrapper) FindTable(tab sqlparser.TableName) (*vindexes.BaseTable, string, topodatapb.TabletType, key.ShardDestination, error) { - destKeyspace, destTabletType, destTarget, err := topoproto.ParseDestination(tab.Qualifier.String(), topodatapb.TabletType_PRIMARY) + destKeyspace, destTabletType, destTarget, _, err := topoproto.ParseDestination(tab.Qualifier.String(), topodatapb.TabletType_PRIMARY) if err != nil { return nil, destKeyspace, destTabletType, destTarget, err } @@ -248,7 +248,7 @@ func (vw *VSchemaWrapper) FindTable(tab sqlparser.TableName) (*vindexes.BaseTabl } func (vw *VSchemaWrapper) FindView(tab sqlparser.TableName) sqlparser.TableStatement { - destKeyspace, _, _, err := topoproto.ParseDestination(tab.Qualifier.String(), topodatapb.TabletType_PRIMARY) + destKeyspace, _, _, _, err := topoproto.ParseDestination(tab.Qualifier.String(), topodatapb.TabletType_PRIMARY) if err != nil { return nil } @@ -256,7 +256,7 @@ func (vw *VSchemaWrapper) FindView(tab sqlparser.TableName) sqlparser.TableState } func (vw *VSchemaWrapper) FindViewTarget(name sqlparser.TableName) (*vindexes.Keyspace, error) { - destKeyspace, _, _, err := topoproto.ParseDestination(name.Qualifier.String(), topodatapb.TabletType_PRIMARY) + destKeyspace, _, _, _, err := topoproto.ParseDestination(name.Qualifier.String(), topodatapb.TabletType_PRIMARY) if err != nil { return nil, err } @@ -336,7 +336,7 @@ func (vw *VSchemaWrapper) IsViewsEnabled() bool { // FindMirrorRule finds the mirror rule for the requested keyspace, table // name, and the tablet type in the VSchema. func (vw *VSchemaWrapper) FindMirrorRule(tab sqlparser.TableName) (*vindexes.MirrorRule, error) { - destKeyspace, destTabletType, _, err := topoproto.ParseDestination(tab.Qualifier.String(), topodatapb.TabletType_PRIMARY) + destKeyspace, destTabletType, _, _, err := topoproto.ParseDestination(tab.Qualifier.String(), topodatapb.TabletType_PRIMARY) if err != nil { return nil, err } diff --git a/go/vt/topo/topoproto/destination.go b/go/vt/topo/topoproto/destination.go index 19a4965e62b..34ffe03497c 100644 --- a/go/vt/topo/topoproto/destination.go +++ b/go/vt/topo/topoproto/destination.go @@ -29,16 +29,37 @@ import ( // ParseDestination parses the string representation of a ShardDestination // of the form keyspace:shard@tablet_type. You can use a / instead of a :. -func ParseDestination(targetString string, defaultTabletType topodatapb.TabletType) (string, topodatapb.TabletType, key.ShardDestination, error) { +// It also supports tablet-specific routing with keyspace@tablet-alias where +// tablet-alias is in the format cell-uid (e.g., zone1-0000000100). +func ParseDestination(targetString string, defaultTabletType topodatapb.TabletType) (string, topodatapb.TabletType, key.ShardDestination, *topodatapb.TabletAlias, error) { var dest key.ShardDestination var keyspace string + var tabletAlias *topodatapb.TabletAlias tabletType := defaultTabletType last := strings.LastIndexAny(targetString, "@") if last != -1 { - // No need to check the error. UNKNOWN will be returned on - // error and it will fail downstream. - tabletType, _ = ParseTabletType(targetString[last+1:]) + afterAt := targetString[last+1:] + // Try parsing as tablet type first (backward compatible) + parsedTabletType, err := ParseTabletType(afterAt) + // If tablet type parsing fails or returns UNKNOWN, try parsing as tablet alias + if err != nil || parsedTabletType == topodatapb.TabletType_UNKNOWN { + // Check if it looks like a tablet alias (contains a dash) + if strings.Contains(afterAt, "-") { + alias, aliasErr := ParseTabletAlias(afterAt) + if aliasErr == nil { + tabletAlias = alias + // Keep tabletType as defaultTabletType when using tablet alias + } else { + // If both tablet type and tablet alias parsing fail, keep the UNKNOWN tablet type + // which will cause appropriate error handling downstream + tabletType = topodatapb.TabletType_UNKNOWN + } + } + } else { + // Successfully parsed as tablet type + tabletType = parsedTabletType + } targetString = targetString[:last] } last = strings.LastIndexAny(targetString, "/:") @@ -51,29 +72,29 @@ func ParseDestination(targetString string, defaultTabletType topodatapb.TabletTy if last != -1 { rangeEnd := strings.LastIndexAny(targetString, "]") if rangeEnd == -1 { - return keyspace, tabletType, dest, vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "invalid key range provided. Couldn't find range end ']'") + return keyspace, tabletType, dest, nil, vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "invalid key range provided. Couldn't find range end ']'") } rangeString := targetString[last+1 : rangeEnd] if strings.Contains(rangeString, "-") { // Parse as range keyRange, err := key.ParseShardingSpec(rangeString) if err != nil { - return keyspace, tabletType, dest, err + return keyspace, tabletType, dest, nil, err } if len(keyRange) != 1 { - return keyspace, tabletType, dest, vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "single keyrange expected in %s", rangeString) + return keyspace, tabletType, dest, nil, vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "single keyrange expected in %s", rangeString) } dest = key.DestinationExactKeyRange{KeyRange: keyRange[0]} } else { // Parse as keyspace id destBytes, err := hex.DecodeString(rangeString) if err != nil { - return keyspace, tabletType, dest, vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "expected valid hex in keyspace id %s", rangeString) + return keyspace, tabletType, dest, nil, vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "expected valid hex in keyspace id %s", rangeString) } dest = key.DestinationKeyspaceID(destBytes) } targetString = targetString[:last] } keyspace = targetString - return keyspace, tabletType, dest, nil + return keyspace, tabletType, dest, tabletAlias, nil } diff --git a/go/vt/topo/topoproto/destination_test.go b/go/vt/topo/topoproto/destination_test.go index 553f749021b..06a6c834253 100644 --- a/go/vt/topo/topoproto/destination_test.go +++ b/go/vt/topo/topoproto/destination_test.go @@ -90,7 +90,7 @@ func TestParseDestination(t *testing.T) { }} for _, tcase := range testcases { - if targetKeyspace, targetTabletType, targetDest, _ := ParseDestination(tcase.targetString, topodatapb.TabletType_PRIMARY); !reflect.DeepEqual(targetDest, tcase.dest) || targetKeyspace != tcase.keyspace || targetTabletType != tcase.tabletType { + if targetKeyspace, targetTabletType, targetDest, _, _ := ParseDestination(tcase.targetString, topodatapb.TabletType_PRIMARY); !reflect.DeepEqual(targetDest, tcase.dest) || targetKeyspace != tcase.keyspace || targetTabletType != tcase.tabletType { t.Errorf("ParseDestination(%s) - got: (%v, %v, %v), want (%v, %v, %v)", tcase.targetString, targetDest, @@ -103,19 +103,19 @@ func TestParseDestination(t *testing.T) { } } - _, _, _, err := ParseDestination("ks[20-40-60]", topodatapb.TabletType_PRIMARY) + _, _, _, _, err := ParseDestination("ks[20-40-60]", topodatapb.TabletType_PRIMARY) want := "single keyrange expected in 20-40-60" if err == nil || err.Error() != want { t.Errorf("executorExec error: %v, want %s", err, want) } - _, _, _, err = ParseDestination("ks[--60]", topodatapb.TabletType_PRIMARY) + _, _, _, _, err = ParseDestination("ks[--60]", topodatapb.TabletType_PRIMARY) want = "malformed spec: MinKey/MaxKey cannot be in the middle of the spec: \"--60\"" if err == nil || err.Error() != want { t.Errorf("executorExec error: %v, want %s", err, want) } - _, _, _, err = ParseDestination("ks[qrnqorrs]@primary", topodatapb.TabletType_PRIMARY) + _, _, _, _, err = ParseDestination("ks[qrnqorrs]@primary", topodatapb.TabletType_PRIMARY) want = "expected valid hex in keyspace id qrnqorrs" if err == nil || err.Error() != want { t.Errorf("executorExec error: %v, want %s", err, want) diff --git a/go/vt/vtgate/executorcontext/safe_session.go b/go/vt/vtgate/executorcontext/safe_session.go index 13b46dd0240..6042ce04ce9 100644 --- a/go/vt/vtgate/executorcontext/safe_session.go +++ b/go/vt/vtgate/executorcontext/safe_session.go @@ -65,6 +65,11 @@ type ( logging *ExecuteLogger + // targetTabletAlias is set when using tablet-specific routing via USE keyspace@tablet-alias. + // This causes all queries to route to the specified tablet until cleared. + // Note: This is stored in the Go wrapper, not in the protobuf Session. + targetTabletAlias *topodatapb.TabletAlias + *vtgatepb.Session } @@ -1143,3 +1148,19 @@ func (l *ExecuteLogger) GetLogs() []engine.ExecuteEntry { copy(result, l.entries) return result } + +// SetTargetTabletAlias sets the tablet alias for tablet-specific routing. +// When set, all queries will route to the specified tablet until cleared. +func (session *SafeSession) SetTargetTabletAlias(alias *topodatapb.TabletAlias) { + session.mu.Lock() + defer session.mu.Unlock() + session.targetTabletAlias = alias +} + +// GetTargetTabletAlias returns the current tablet alias for tablet-specific routing, +// or nil if not set. +func (session *SafeSession) GetTargetTabletAlias() *topodatapb.TabletAlias { + session.mu.Lock() + defer session.mu.Unlock() + return session.targetTabletAlias +} diff --git a/go/vt/vtgate/executorcontext/vcursor_impl.go b/go/vt/vtgate/executorcontext/vcursor_impl.go index fc98e1f37c0..928cf7eb37a 100644 --- a/go/vt/vtgate/executorcontext/vcursor_impl.go +++ b/go/vt/vtgate/executorcontext/vcursor_impl.go @@ -538,7 +538,8 @@ func (vc *VCursorImpl) parseDestinationTarget(targetString string) (string, topo // ParseDestinationTarget parses destination target string and provides a keyspace if possible. func ParseDestinationTarget(targetString string, tablet topodatapb.TabletType, vschema *vindexes.VSchema) (string, topodatapb.TabletType, key.ShardDestination, error) { - destKeyspace, destTabletType, dest, err := topoprotopb.ParseDestination(targetString, tablet) + destKeyspace, destTabletType, dest, _, err := topoprotopb.ParseDestination(targetString, tablet) + // Note: We ignore the tablet alias here as it's handled separately in SetTarget // If the keyspace is not specified, and there is only one keyspace in the VSchema, use that. if destKeyspace == "" && len(vschema.Keyspaces) == 1 { for k := range vschema.Keyspaces { @@ -1027,10 +1028,24 @@ func (vc *VCursorImpl) Session() engine.SessionActions { } func (vc *VCursorImpl) SetTarget(target string) error { - keyspace, tabletType, _, err := topoprotopb.ParseDestination(target, vc.config.DefaultTabletType) + keyspace, tabletType, _, tabletAlias, err := topoprotopb.ParseDestination(target, vc.config.DefaultTabletType) if err != nil { return err } + + // Clear any existing tablet-specific routing if not specified + if tabletAlias == nil { + vc.SafeSession.SetTargetTabletAlias(nil) + } else { + // Store tablet alias in session for routing + // Note: We don't validate the tablet exists here - if it doesn't exist or is + // unreachable, the query will fail during execution with a clear error message + vc.SafeSession.SetTargetTabletAlias(tabletAlias) + + // Keep tabletType as determined by ParseDestination (defaultTabletType) + // The actual routing uses the tablet alias, so the type is only for VCursor state + } + if _, ok := vc.vschema.Keyspaces[keyspace]; !ignoreKeyspace(keyspace) && !ok { return vterrors.VT05003(keyspace) } diff --git a/go/vt/vtgate/scatter_conn.go b/go/vt/vtgate/scatter_conn.go index 96fc5f066e4..8a11091d4e0 100644 --- a/go/vt/vtgate/scatter_conn.go +++ b/go/vt/vtgate/scatter_conn.go @@ -859,6 +859,13 @@ func requireNewQS(err error, target *querypb.Target) bool { // actionInfo looks at the current session, and returns information about what needs to be done for this tablet func actionInfo(ctx context.Context, target *querypb.Target, session *econtext.SafeSession, autocommit bool, txMode vtgatepb.TransactionMode) (*shardActionInfo, *vtgatepb.Session_ShardSession, error) { if !(session.InTransaction() || session.InReservedConn()) { + // Check for tablet-specific routing for non-transactional queries + if alias := session.GetTargetTabletAlias(); alias != nil { + return &shardActionInfo{ + actionNeeded: nothing, + alias: alias, + }, nil, nil + } return &shardActionInfo{}, nil, nil } ignoreSession := ctx.Value(engine.IgnoreReserveTxn) @@ -896,6 +903,10 @@ func actionInfo(ctx context.Context, target *querypb.Target, session *econtext.S info.alias = shardSession.TabletAlias info.rowsAffected = shardSession.RowsAffected } + // Override alias if tablet-specific routing is set + if targetAlias := session.GetTargetTabletAlias(); targetAlias != nil { + info.alias = targetAlias + } return info, shardSession, nil } diff --git a/go/vtbench/client.go b/go/vtbench/client.go index 70c9615b321..996ddc430c1 100644 --- a/go/vtbench/client.go +++ b/go/vtbench/client.go @@ -125,7 +125,7 @@ func (c *grpcVttabletConn) connect(ctx context.Context, cp ConnParams) error { }) // parse the "db" into the keyspace/shard target - keyspace, tabletType, dest, err := topoproto.ParseDestination(cp.DB, topodatapb.TabletType_PRIMARY) + keyspace, tabletType, dest, _, err := topoproto.ParseDestination(cp.DB, topodatapb.TabletType_PRIMARY) if err != nil { return err } From de35ba8676b2705f8ebe232f324c258ccce5a795 Mon Sep 17 00:00:00 2001 From: Nick Van Wiggeren Date: Sat, 25 Oct 2025 17:26:52 +0200 Subject: [PATCH 02/14] add tests Signed-off-by: Nick Van Wiggeren --- go/test/endtoend/vtgate/misc_test.go | 67 +++++++++++++++++++ go/vt/topo/topoproto/destination_test.go | 27 ++++++-- .../executorcontext/safe_session_test.go | 34 ++++++++++ go/vt/vtgate/scatter_conn_test.go | 65 ++++++++++++++++++ 4 files changed, 189 insertions(+), 4 deletions(-) diff --git a/go/test/endtoend/vtgate/misc_test.go b/go/test/endtoend/vtgate/misc_test.go index 30a26765c62..7cc5dae2a09 100644 --- a/go/test/endtoend/vtgate/misc_test.go +++ b/go/test/endtoend/vtgate/misc_test.go @@ -840,6 +840,73 @@ func TestDDLTargeted(t *testing.T) { utils.AssertMatches(t, conn, `select id from ddl_targeted`, `[[INT64(1)]]`) } +// TestTabletTargeting tests tablet-specific routing with USE keyspace@tablet-alias syntax. +// In a sharded keyspace, this validates that tablet-specific routing overrides normal hash-based routing. +func TestTabletTargeting(t *testing.T) { + ctx := context.Background() + conn, err := mysql.Connect(ctx, &vtParams) + require.NoError(t, err) + defer conn.Close() + + // Get tablet aliases from show vitess_tablets + qr := utils.Exec(t, conn, "show vitess_tablets") + require.Greater(t, len(qr.Rows), 0, "no tablets found") + + // Find PRIMARY tablets for both shards (-80 and 80-) + var primaryShard80Minus string // We'll target this shard + var primaryShard80Plus string // We'll verify this shard gets no writes + for _, row := range qr.Rows { + shard := row[2].ToString() + tabletType := row[3].ToString() + if tabletType == "PRIMARY" { + switch shard { + case "-80": + primaryShard80Minus = row[0].ToString() + case "80-": + primaryShard80Plus = row[0].ToString() + } + } + } + require.NotEmpty(t, primaryShard80Minus, "no PRIMARY tablet found for -80 shard") + require.NotEmpty(t, primaryShard80Plus, "no PRIMARY tablet found for 80- shard") + + // Test: Target the -80 shard's PRIMARY and insert rows that normally hash to -80 + // id1=1 and id1=2 both hash to -80 shard based on the hash vindex + utils.Exec(t, conn, fmt.Sprintf("USE ks@%s", primaryShard80Minus)) + utils.Exec(t, conn, "insert into t1(id1, id2) values(1, 100), (2, 200)") + utils.AssertMatches(t, conn, "select id1 from t1 where id1 in (1, 2) order by id1", "[[INT64(1)] [INT64(2)]]") + + // Test: Verify the other shard (80-) did not receive these writes + utils.Exec(t, conn, fmt.Sprintf("USE ks@%s", primaryShard80Plus)) + utils.AssertIsEmpty(t, conn, "select id1 from t1 where id1 in (1, 2)") + + // Test: Transaction with tablet-specific routing + utils.Exec(t, conn, fmt.Sprintf("USE ks@%s", primaryShard80Minus)) + utils.Exec(t, conn, "begin") + utils.Exec(t, conn, "insert into t1(id1, id2) values(10, 300)") + utils.Exec(t, conn, "commit") + utils.AssertMatches(t, conn, "select id1 from t1 where id1=10", "[[INT64(10)]]") + + // Test: Verify the other shard still has no data for our test rows + utils.Exec(t, conn, fmt.Sprintf("USE ks@%s", primaryShard80Plus)) + utils.AssertIsEmpty(t, conn, "select id1 from t1 where id1 in (1, 2, 10)") + + // Test: Rollback with tablet-specific routing + utils.Exec(t, conn, fmt.Sprintf("USE ks@%s", primaryShard80Minus)) + utils.Exec(t, conn, "begin") + utils.Exec(t, conn, "insert into t1(id1, id2) values(20, 400)") + utils.Exec(t, conn, "rollback") + // 20 should not exist + utils.AssertIsEmpty(t, conn, "select id1 from t1 where id1=20") + + // Test: Clear tablet targeting returns to normal routing + utils.Exec(t, conn, "USE ks") + utils.AssertMatches(t, conn, "select id1 from t1 where id1 in (1, 2, 10) order by id1", "[[INT64(1)] [INT64(2)] [INT64(10)]]") + + // Cleanup + utils.Exec(t, conn, "delete from t1 where id1 in (1, 2, 10)") +} + // TestDynamicConfig tests the dynamic configurations. func TestDynamicConfig(t *testing.T) { t.Run("DiscoveryLowReplicationLag", func(t *testing.T) { diff --git a/go/vt/topo/topoproto/destination_test.go b/go/vt/topo/topoproto/destination_test.go index 06a6c834253..aa425e99b23 100644 --- a/go/vt/topo/topoproto/destination_test.go +++ b/go/vt/topo/topoproto/destination_test.go @@ -35,6 +35,7 @@ func TestParseDestination(t *testing.T) { dest key.ShardDestination keyspace string tabletType topodatapb.TabletType + tabletAlias *topodatapb.TabletAlias }{{ targetString: "ks[10-20]@primary", keyspace: "ks", @@ -87,18 +88,36 @@ func TestParseDestination(t *testing.T) { keyspace: "ks", dest: key.DestinationShard("-80"), tabletType: topodatapb.TabletType_PRIMARY, + }, { + targetString: "ks@zone1-0000000100", + keyspace: "ks", + tabletType: topodatapb.TabletType_PRIMARY, + tabletAlias: &topodatapb.TabletAlias{Cell: "zone1", Uid: 100}, + }, { + targetString: "ks:-80@zone1-0000000100", + keyspace: "ks", + dest: key.DestinationShard("-80"), + tabletType: topodatapb.TabletType_PRIMARY, + tabletAlias: &topodatapb.TabletAlias{Cell: "zone1", Uid: 100}, + }, { + targetString: "@zone1-0000000200", + keyspace: "", + tabletType: topodatapb.TabletType_PRIMARY, + tabletAlias: &topodatapb.TabletAlias{Cell: "zone1", Uid: 200}, }} for _, tcase := range testcases { - if targetKeyspace, targetTabletType, targetDest, _, _ := ParseDestination(tcase.targetString, topodatapb.TabletType_PRIMARY); !reflect.DeepEqual(targetDest, tcase.dest) || targetKeyspace != tcase.keyspace || targetTabletType != tcase.tabletType { - t.Errorf("ParseDestination(%s) - got: (%v, %v, %v), want (%v, %v, %v)", + if targetKeyspace, targetTabletType, targetDest, targetAlias, _ := ParseDestination(tcase.targetString, topodatapb.TabletType_PRIMARY); !reflect.DeepEqual(targetDest, tcase.dest) || targetKeyspace != tcase.keyspace || targetTabletType != tcase.tabletType || !reflect.DeepEqual(targetAlias, tcase.tabletAlias) { + t.Errorf("ParseDestination(%s) - got: (%v, %v, %v, %v), want (%v, %v, %v, %v)", tcase.targetString, - targetDest, targetKeyspace, targetTabletType, - tcase.dest, + targetDest, + targetAlias, tcase.keyspace, tcase.tabletType, + tcase.dest, + tcase.tabletAlias, ) } } diff --git a/go/vt/vtgate/executorcontext/safe_session_test.go b/go/vt/vtgate/executorcontext/safe_session_test.go index 2226575e31b..4430f1b68d4 100644 --- a/go/vt/vtgate/executorcontext/safe_session_test.go +++ b/go/vt/vtgate/executorcontext/safe_session_test.go @@ -205,3 +205,37 @@ func TestTimeZone(t *testing.T) { }) } } + +// TestTargetTabletAlias tests the SetTargetTabletAlias and GetTargetTabletAlias methods. +func TestTargetTabletAlias(t *testing.T) { + session := NewSafeSession(&vtgatepb.Session{}) + + // Test: initially nil + assert.Nil(t, session.GetTargetTabletAlias()) + + // Test: Set and get + alias := &topodatapb.TabletAlias{Cell: "zone1", Uid: 100} + session.SetTargetTabletAlias(alias) + got := session.GetTargetTabletAlias() + assert.Equal(t, alias, got) + + // Test: Clear (set to nil) + session.SetTargetTabletAlias(nil) + assert.Nil(t, session.GetTargetTabletAlias()) + + // Test: Thread safety - concurrent access + done := make(chan bool) + for i := 0; i < 100; i++ { + go func(uid uint32) { + testAlias := &topodatapb.TabletAlias{Cell: "cell", Uid: uid} + session.SetTargetTabletAlias(testAlias) + _ = session.GetTargetTabletAlias() + done <- true + }(uint32(i)) + } + for i := 0; i < 100; i++ { + <-done + } + // Just verify we didn't panic - the final value is non-deterministic + assert.NotNil(t, session.GetTargetTabletAlias()) +} diff --git a/go/vt/vtgate/scatter_conn_test.go b/go/vt/vtgate/scatter_conn_test.go index c09aa84b09d..f096a159e99 100644 --- a/go/vt/vtgate/scatter_conn_test.go +++ b/go/vt/vtgate/scatter_conn_test.go @@ -632,3 +632,68 @@ func TestIsConnClosed(t *testing.T) { }) } } + +// TestActionInfoWithTabletAlias tests the actionInfo function with tablet-specific routing. +func TestActionInfoWithTabletAlias(t *testing.T) { + ctx := utils.LeakCheckContext(t) + target := &querypb.Target{ + Keyspace: "ks", + Shard: "-80", + TabletType: topodatapb.TabletType_PRIMARY, + } + tabletAlias := &topodatapb.TabletAlias{Cell: "zone1", Uid: 100} + + t.Run("non-transactional with tablet alias", func(t *testing.T) { + session := econtext.NewSafeSession(&vtgatepb.Session{}) + session.SetTargetTabletAlias(tabletAlias) + + info, shardSession, err := actionInfo(ctx, target, session, false, vtgatepb.TransactionMode_MULTI) + require.NoError(t, err) + assert.Nil(t, shardSession) + assert.Equal(t, nothing, info.actionNeeded) + assert.Equal(t, tabletAlias, info.alias) + }) + + t.Run("transaction begin with tablet alias", func(t *testing.T) { + session := econtext.NewSafeSession(&vtgatepb.Session{ + InTransaction: true, + }) + session.SetTargetTabletAlias(tabletAlias) + + info, shardSession, err := actionInfo(ctx, target, session, false, vtgatepb.TransactionMode_MULTI) + require.NoError(t, err) + assert.Nil(t, shardSession) + assert.Equal(t, begin, info.actionNeeded) + assert.Equal(t, tabletAlias, info.alias) + }) + + t.Run("existing transaction with tablet alias", func(t *testing.T) { + session := econtext.NewSafeSession(&vtgatepb.Session{ + InTransaction: true, + ShardSessions: []*vtgatepb.Session_ShardSession{{ + Target: target, + TransactionId: 12345, + TabletAlias: &topodatapb.TabletAlias{Cell: "zone1", Uid: 50}, + }}, + }) + session.SetTargetTabletAlias(tabletAlias) + + info, shardSession, err := actionInfo(ctx, target, session, false, vtgatepb.TransactionMode_MULTI) + require.NoError(t, err) + assert.NotNil(t, shardSession) + assert.Equal(t, int64(12345), info.transactionID) + assert.Equal(t, nothing, info.actionNeeded) + // Tablet alias should be overridden + assert.Equal(t, tabletAlias, info.alias) + }) + + t.Run("no tablet alias - existing behavior", func(t *testing.T) { + session := econtext.NewSafeSession(&vtgatepb.Session{}) + + info, shardSession, err := actionInfo(ctx, target, session, false, vtgatepb.TransactionMode_MULTI) + require.NoError(t, err) + assert.Nil(t, shardSession) + assert.Equal(t, nothing, info.actionNeeded) + assert.Nil(t, info.alias) + }) +} From b5b4284b2975c5361363dc643f217940f5df543f Mon Sep 17 00:00:00 2001 From: Nick Van Wiggeren Date: Sun, 26 Oct 2025 17:13:12 +0100 Subject: [PATCH 03/14] wip: set destination in the right places Signed-off-by: Nick Van Wiggeren --- go/test/endtoend/vtgate/misc_test.go | 39 +++++++++++-------- go/vt/vtgate/executor.go | 2 +- go/vt/vtgate/executor_test.go | 6 +-- go/vt/vtgate/executorcontext/vcursor_impl.go | 19 +++++---- .../executorcontext/vcursor_impl_test.go | 2 +- go/vt/vtgate/vtgate.go | 8 ++-- 6 files changed, 43 insertions(+), 33 deletions(-) diff --git a/go/test/endtoend/vtgate/misc_test.go b/go/test/endtoend/vtgate/misc_test.go index 7cc5dae2a09..6e1473fa692 100644 --- a/go/test/endtoend/vtgate/misc_test.go +++ b/go/test/endtoend/vtgate/misc_test.go @@ -840,8 +840,8 @@ func TestDDLTargeted(t *testing.T) { utils.AssertMatches(t, conn, `select id from ddl_targeted`, `[[INT64(1)]]`) } -// TestTabletTargeting tests tablet-specific routing with USE keyspace@tablet-alias syntax. -// In a sharded keyspace, this validates that tablet-specific routing overrides normal hash-based routing. +// TestTabletTargeting tests tablet-specific routing with USE keyspace:shard@tablet-alias syntax. +// When shard is specified, tablet-specific routing bypasses vindex-based shard resolution. func TestTabletTargeting(t *testing.T) { ctx := context.Background() conn, err := mysql.Connect(ctx, &vtParams) @@ -870,41 +870,46 @@ func TestTabletTargeting(t *testing.T) { require.NotEmpty(t, primaryShard80Minus, "no PRIMARY tablet found for -80 shard") require.NotEmpty(t, primaryShard80Plus, "no PRIMARY tablet found for 80- shard") - // Test: Target the -80 shard's PRIMARY and insert rows that normally hash to -80 - // id1=1 and id1=2 both hash to -80 shard based on the hash vindex - utils.Exec(t, conn, fmt.Sprintf("USE ks@%s", primaryShard80Minus)) + // Test: Target a specific tablet in -80 shard and insert rows + // id1=1 and id1=2 both hash to -80 shard - this validates normal behavior + utils.Exec(t, conn, fmt.Sprintf("USE ks:-80@%s", primaryShard80Minus)) utils.Exec(t, conn, "insert into t1(id1, id2) values(1, 100), (2, 200)") utils.AssertMatches(t, conn, "select id1 from t1 where id1 in (1, 2) order by id1", "[[INT64(1)] [INT64(2)]]") - // Test: Verify the other shard (80-) did not receive these writes - utils.Exec(t, conn, fmt.Sprintf("USE ks@%s", primaryShard80Plus)) - utils.AssertIsEmpty(t, conn, "select id1 from t1 where id1 in (1, 2)") + // Test: Insert data that would normally hash to 80- shard, but goes to -80 because of shard targeting + // id1=4 hashes to 80-, but we're targeting -80 shard explicitly + utils.Exec(t, conn, fmt.Sprintf("USE ks:-80@%s", primaryShard80Minus)) + utils.Exec(t, conn, "insert into t1(id1, id2) values(4, 400)") + + // Verify the data went to -80 shard (not where vindex would have put it) + utils.Exec(t, conn, "USE ks:-80") + utils.AssertMatches(t, conn, "select id1 from t1 where id1=4", "[[INT64(4)]]") + + // Verify the data did NOT go to 80- shard (where vindex says it should be) + utils.Exec(t, conn, "USE ks:80-") + utils.AssertIsEmpty(t, conn, "select id1 from t1 where id1=4") // Test: Transaction with tablet-specific routing - utils.Exec(t, conn, fmt.Sprintf("USE ks@%s", primaryShard80Minus)) + utils.Exec(t, conn, fmt.Sprintf("USE ks:-80@%s", primaryShard80Minus)) utils.Exec(t, conn, "begin") utils.Exec(t, conn, "insert into t1(id1, id2) values(10, 300)") utils.Exec(t, conn, "commit") utils.AssertMatches(t, conn, "select id1 from t1 where id1=10", "[[INT64(10)]]") - // Test: Verify the other shard still has no data for our test rows - utils.Exec(t, conn, fmt.Sprintf("USE ks@%s", primaryShard80Plus)) - utils.AssertIsEmpty(t, conn, "select id1 from t1 where id1 in (1, 2, 10)") - // Test: Rollback with tablet-specific routing - utils.Exec(t, conn, fmt.Sprintf("USE ks@%s", primaryShard80Minus)) + utils.Exec(t, conn, fmt.Sprintf("USE ks:-80@%s", primaryShard80Minus)) utils.Exec(t, conn, "begin") - utils.Exec(t, conn, "insert into t1(id1, id2) values(20, 400)") + utils.Exec(t, conn, "insert into t1(id1, id2) values(20, 500)") utils.Exec(t, conn, "rollback") // 20 should not exist utils.AssertIsEmpty(t, conn, "select id1 from t1 where id1=20") // Test: Clear tablet targeting returns to normal routing utils.Exec(t, conn, "USE ks") - utils.AssertMatches(t, conn, "select id1 from t1 where id1 in (1, 2, 10) order by id1", "[[INT64(1)] [INT64(2)] [INT64(10)]]") + utils.AssertMatches(t, conn, "select id1 from t1 where id1 in (1, 2, 4, 10) order by id1", "[[INT64(1)] [INT64(2)] [INT64(4)] [INT64(10)]]") // Cleanup - utils.Exec(t, conn, "delete from t1 where id1 in (1, 2, 10)") + utils.Exec(t, conn, "delete from t1 where id1 in (1, 2, 4, 10)") } // TestDynamicConfig tests the dynamic configurations. diff --git a/go/vt/vtgate/executor.go b/go/vt/vtgate/executor.go index 4c1fbedb447..d8201d41d0e 100644 --- a/go/vt/vtgate/executor.go +++ b/go/vt/vtgate/executor.go @@ -1105,7 +1105,7 @@ func (e *Executor) SaveVSchema(vschema *vindexes.VSchema, stats *VSchemaStats) { } // ParseDestinationTarget parses destination target string and sets default keyspace if possible. -func (e *Executor) ParseDestinationTarget(targetString string) (string, topodatapb.TabletType, key.ShardDestination, error) { +func (e *Executor) ParseDestinationTarget(targetString string) (string, topodatapb.TabletType, key.ShardDestination, *topodatapb.TabletAlias, error) { return econtext.ParseDestinationTarget(targetString, defaultTabletType, e.VSchema()) } diff --git a/go/vt/vtgate/executor_test.go b/go/vt/vtgate/executor_test.go index b76e6cf74ca..5e9fac052c8 100644 --- a/go/vt/vtgate/executor_test.go +++ b/go/vt/vtgate/executor_test.go @@ -1920,7 +1920,7 @@ func TestParseEmptyTargetSingleKeyspace(t *testing.T) { } r.vschema = altVSchema - destKeyspace, destTabletType, _, _ := r.ParseDestinationTarget("") + destKeyspace, destTabletType, _, _, _ := r.ParseDestinationTarget("") if destKeyspace != KsTestUnsharded || destTabletType != topodatapb.TabletType_PRIMARY { t.Errorf( "parseDestinationTarget(%s): got (%v, %v), want (%v, %v)", @@ -1944,7 +1944,7 @@ func TestParseEmptyTargetMultiKeyspace(t *testing.T) { } r.vschema = altVSchema - destKeyspace, destTabletType, _, _ := r.ParseDestinationTarget("") + destKeyspace, destTabletType, _, _, _ := r.ParseDestinationTarget("") if destKeyspace != "" || destTabletType != topodatapb.TabletType_PRIMARY { t.Errorf( "parseDestinationTarget(%s): got (%v, %v), want (%v, %v)", @@ -1967,7 +1967,7 @@ func TestParseTargetSingleKeyspace(t *testing.T) { } r.vschema = altVSchema - destKeyspace, destTabletType, _, _ := r.ParseDestinationTarget("@replica") + destKeyspace, destTabletType, _, _, _ := r.ParseDestinationTarget("@replica") if destKeyspace != KsTestUnsharded || destTabletType != topodatapb.TabletType_REPLICA { t.Errorf( "parseDestinationTarget(%s): got (%v, %v), want (%v, %v)", diff --git a/go/vt/vtgate/executorcontext/vcursor_impl.go b/go/vt/vtgate/executorcontext/vcursor_impl.go index 928cf7eb37a..6c3b5750429 100644 --- a/go/vt/vtgate/executorcontext/vcursor_impl.go +++ b/go/vt/vtgate/executorcontext/vcursor_impl.go @@ -208,11 +208,15 @@ func NewVCursorImpl( cfg VCursorConfig, metrics Metrics, ) (*VCursorImpl, error) { - keyspace, tabletType, destination, err := ParseDestinationTarget(safeSession.TargetString, cfg.DefaultTabletType, vschema) + keyspace, tabletType, destination, tabletAlias, err := ParseDestinationTarget(safeSession.TargetString, cfg.DefaultTabletType, vschema) if err != nil { return nil, err } + // Store tablet alias from target string into session + // This ensures tablet-specific routing persists across queries + safeSession.SetTargetTabletAlias(tabletAlias) + var ts *topo.Server // We don't have access to the underlying TopoServer if this vtgate is // filtering keyspaces because we don't have an accurate view of the topo. @@ -533,20 +537,20 @@ func (vc *VCursorImpl) FindViewTarget(name sqlparser.TableName) (*vindexes.Keysp } func (vc *VCursorImpl) parseDestinationTarget(targetString string) (string, topodatapb.TabletType, key.ShardDestination, error) { - return ParseDestinationTarget(targetString, vc.tabletType, vc.vschema) + keyspace, tabletType, dest, _, err := ParseDestinationTarget(targetString, vc.tabletType, vc.vschema) + return keyspace, tabletType, dest, err } // ParseDestinationTarget parses destination target string and provides a keyspace if possible. -func ParseDestinationTarget(targetString string, tablet topodatapb.TabletType, vschema *vindexes.VSchema) (string, topodatapb.TabletType, key.ShardDestination, error) { - destKeyspace, destTabletType, dest, _, err := topoprotopb.ParseDestination(targetString, tablet) - // Note: We ignore the tablet alias here as it's handled separately in SetTarget +func ParseDestinationTarget(targetString string, tablet topodatapb.TabletType, vschema *vindexes.VSchema) (string, topodatapb.TabletType, key.ShardDestination, *topodatapb.TabletAlias, error) { + destKeyspace, destTabletType, dest, tabletAlias, err := topoprotopb.ParseDestination(targetString, tablet) // If the keyspace is not specified, and there is only one keyspace in the VSchema, use that. if destKeyspace == "" && len(vschema.Keyspaces) == 1 { for k := range vschema.Keyspaces { destKeyspace = k } } - return destKeyspace, destTabletType, dest, err + return destKeyspace, destTabletType, dest, tabletAlias, err } func (vc *VCursorImpl) getDualTable() (*vindexes.BaseTable, vindexes.Vindex, string, topodatapb.TabletType, key.ShardDestination, error) { @@ -1028,7 +1032,7 @@ func (vc *VCursorImpl) Session() engine.SessionActions { } func (vc *VCursorImpl) SetTarget(target string) error { - keyspace, tabletType, _, tabletAlias, err := topoprotopb.ParseDestination(target, vc.config.DefaultTabletType) + keyspace, tabletType, destination, tabletAlias, err := topoprotopb.ParseDestination(target, vc.config.DefaultTabletType) if err != nil { return err } @@ -1055,6 +1059,7 @@ func (vc *VCursorImpl) SetTarget(target string) error { } vc.SafeSession.SetTargetString(target) vc.keyspace = keyspace + vc.destination = destination vc.tabletType = tabletType return nil } diff --git a/go/vt/vtgate/executorcontext/vcursor_impl_test.go b/go/vt/vtgate/executorcontext/vcursor_impl_test.go index ed7654a58d4..a5eb7d4c394 100644 --- a/go/vt/vtgate/executorcontext/vcursor_impl_test.go +++ b/go/vt/vtgate/executorcontext/vcursor_impl_test.go @@ -393,7 +393,7 @@ func (f fakeExecutor) SetVitessMetadata(ctx context.Context, name, value string) panic("implement me") } -func (f fakeExecutor) ParseDestinationTarget(targetString string) (string, topodatapb.TabletType, key.ShardDestination, error) { +func (f fakeExecutor) ParseDestinationTarget(targetString string) (string, topodatapb.TabletType, key.ShardDestination, *topodatapb.TabletAlias, error) { // TODO implement me panic("implement me") } diff --git a/go/vt/vtgate/vtgate.go b/go/vt/vtgate/vtgate.go index 3fe35044b31..3a3179588a9 100644 --- a/go/vt/vtgate/vtgate.go +++ b/go/vt/vtgate/vtgate.go @@ -558,7 +558,7 @@ func (vtg *VTGate) Execute( prepared bool, ) (newSession *vtgatepb.Session, qr *sqltypes.Result, err error) { // In this context, we don't care if we can't fully parse destination - destKeyspace, destTabletType, _, _ := vtg.executor.ParseDestinationTarget(session.TargetString) + destKeyspace, destTabletType, _, _, _ := vtg.executor.ParseDestinationTarget(session.TargetString) statsKey := []string{"Execute", destKeyspace, topoproto.TabletTypeLString(destTabletType)} defer vtg.timings.Record(statsKey, time.Now()) @@ -620,7 +620,7 @@ func (vtg *VTGate) ExecuteMulti( // ExecuteBatch executes a batch of queries. func (vtg *VTGate) ExecuteBatch(ctx context.Context, session *vtgatepb.Session, sqlList []string, bindVariablesList []map[string]*querypb.BindVariable) (*vtgatepb.Session, []sqltypes.QueryResponse, error) { // In this context, we don't care if we can't fully parse destination - destKeyspace, destTabletType, _, _ := vtg.executor.ParseDestinationTarget(session.TargetString) + destKeyspace, destTabletType, _, _, _ := vtg.executor.ParseDestinationTarget(session.TargetString) statsKey := []string{"ExecuteBatch", destKeyspace, topoproto.TabletTypeLString(destTabletType)} defer vtg.timings.Record(statsKey, time.Now()) @@ -649,7 +649,7 @@ func (vtg *VTGate) ExecuteBatch(ctx context.Context, session *vtgatepb.Session, // Note we guarantee the callback will not be called concurrently by multiple go routines. func (vtg *VTGate) StreamExecute(ctx context.Context, mysqlCtx vtgateservice.MySQLConnection, session *vtgatepb.Session, sql string, bindVariables map[string]*querypb.BindVariable, callback func(*sqltypes.Result) error) (*vtgatepb.Session, error) { // In this context, we don't care if we can't fully parse destination - destKeyspace, destTabletType, _, _ := vtg.executor.ParseDestinationTarget(session.TargetString) + destKeyspace, destTabletType, _, _, _ := vtg.executor.ParseDestinationTarget(session.TargetString) statsKey := []string{"StreamExecute", destKeyspace, topoproto.TabletTypeLString(destTabletType)} defer vtg.timings.Record(statsKey, time.Now()) @@ -735,7 +735,7 @@ func (vtg *VTGate) CloseSession(ctx context.Context, session *vtgatepb.Session) // Prepare supports non-streaming prepare statement query with multi shards func (vtg *VTGate) Prepare(ctx context.Context, session *vtgatepb.Session, sql string) (newSession *vtgatepb.Session, fld []*querypb.Field, paramsCount uint16, err error) { // In this context, we don't care if we can't fully parse destination - destKeyspace, destTabletType, _, _ := vtg.executor.ParseDestinationTarget(session.TargetString) + destKeyspace, destTabletType, _, _, _ := vtg.executor.ParseDestinationTarget(session.TargetString) statsKey := []string{"Prepare", destKeyspace, topoproto.TabletTypeLString(destTabletType)} defer vtg.timings.Record(statsKey, time.Now()) From 355b03162ec5a8e81bd9b113c23b7ff0080423b1 Mon Sep 17 00:00:00 2001 From: Nick Van Wiggeren Date: Sun, 26 Oct 2025 17:24:22 +0100 Subject: [PATCH 04/14] update test to be more complete Signed-off-by: Nick Van Wiggeren --- go/test/endtoend/vtgate/misc_test.go | 56 ++++++++++++++-------------- 1 file changed, 27 insertions(+), 29 deletions(-) diff --git a/go/test/endtoend/vtgate/misc_test.go b/go/test/endtoend/vtgate/misc_test.go index 6e1473fa692..28c78f6cb28 100644 --- a/go/test/endtoend/vtgate/misc_test.go +++ b/go/test/endtoend/vtgate/misc_test.go @@ -852,63 +852,61 @@ func TestTabletTargeting(t *testing.T) { qr := utils.Exec(t, conn, "show vitess_tablets") require.Greater(t, len(qr.Rows), 0, "no tablets found") - // Find PRIMARY tablets for both shards (-80 and 80-) - var primaryShard80Minus string // We'll target this shard - var primaryShard80Plus string // We'll verify this shard gets no writes + // Find PRIMARY and REPLICA tablets for both shards (-80 and 80-) + var primaryShard80Minus string + var primaryShard80Plus string for _, row := range qr.Rows { shard := row[2].ToString() tabletType := row[3].ToString() - if tabletType == "PRIMARY" { - switch shard { - case "-80": - primaryShard80Minus = row[0].ToString() - case "80-": - primaryShard80Plus = row[0].ToString() - } + switch { + case tabletType == "PRIMARY" && shard == "-80": + primaryShard80Minus = row[0].ToString() + case tabletType == "PRIMARY" && shard == "80-": + primaryShard80Plus = row[0].ToString() } } require.NotEmpty(t, primaryShard80Minus, "no PRIMARY tablet found for -80 shard") require.NotEmpty(t, primaryShard80Plus, "no PRIMARY tablet found for 80- shard") - // Test: Target a specific tablet in -80 shard and insert rows - // id1=1 and id1=2 both hash to -80 shard - this validates normal behavior - utils.Exec(t, conn, fmt.Sprintf("USE ks:-80@%s", primaryShard80Minus)) + // Test 1: Shard targeting bypasses vindex resolution + // Insert data that would normally hash to 80- shard, but goes to -80 because of shard targeting + utils.Exec(t, conn, fmt.Sprintf("USE `ks:-80@%s`", primaryShard80Minus)) utils.Exec(t, conn, "insert into t1(id1, id2) values(1, 100), (2, 200)") - utils.AssertMatches(t, conn, "select id1 from t1 where id1 in (1, 2) order by id1", "[[INT64(1)] [INT64(2)]]") - - // Test: Insert data that would normally hash to 80- shard, but goes to -80 because of shard targeting // id1=4 hashes to 80-, but we're targeting -80 shard explicitly - utils.Exec(t, conn, fmt.Sprintf("USE ks:-80@%s", primaryShard80Minus)) utils.Exec(t, conn, "insert into t1(id1, id2) values(4, 400)") // Verify the data went to -80 shard (not where vindex would have put it) - utils.Exec(t, conn, "USE ks:-80") - utils.AssertMatches(t, conn, "select id1 from t1 where id1=4", "[[INT64(4)]]") + utils.Exec(t, conn, "USE `ks:-80`") + utils.AssertMatches(t, conn, "select id1 from t1 where id1 in (1, 2, 4) order by id1", "[[INT64(1)] [INT64(2)] [INT64(4)]]") - // Verify the data did NOT go to 80- shard (where vindex says it should be) - utils.Exec(t, conn, "USE ks:80-") + // Verify the data did NOT go to 80- shard (where vindex says id1=4 should be) + utils.Exec(t, conn, "USE `ks:80-`") utils.AssertIsEmpty(t, conn, "select id1 from t1 where id1=4") - // Test: Transaction with tablet-specific routing - utils.Exec(t, conn, fmt.Sprintf("USE ks:-80@%s", primaryShard80Minus)) + // Test 2: Transaction with tablet-specific routing maintains sticky connection + utils.Exec(t, conn, fmt.Sprintf("USE `ks:-80@%s`", primaryShard80Minus)) utils.Exec(t, conn, "begin") utils.Exec(t, conn, "insert into t1(id1, id2) values(10, 300)") + // Subsequent queries in transaction should go to same tablet + utils.AssertMatches(t, conn, "select id1 from t1 where id1=10", "[[INT64(10)]]") utils.Exec(t, conn, "commit") utils.AssertMatches(t, conn, "select id1 from t1 where id1=10", "[[INT64(10)]]") - // Test: Rollback with tablet-specific routing - utils.Exec(t, conn, fmt.Sprintf("USE ks:-80@%s", primaryShard80Minus)) + // Test 3: Rollback with tablet-specific routing + utils.Exec(t, conn, fmt.Sprintf("USE `ks:-80@%s`", primaryShard80Minus)) utils.Exec(t, conn, "begin") utils.Exec(t, conn, "insert into t1(id1, id2) values(20, 500)") utils.Exec(t, conn, "rollback") - // 20 should not exist utils.AssertIsEmpty(t, conn, "select id1 from t1 where id1=20") - // Test: Clear tablet targeting returns to normal routing + // Test 4: Clear tablet targeting returns to normal routing + // With normal routing, the query for id1=4 will be sent to the wrong shard (80-), so it won't be found. + // This is expected and demonstrates that vindex routing is back in effect. utils.Exec(t, conn, "USE ks") - utils.AssertMatches(t, conn, "select id1 from t1 where id1 in (1, 2, 4, 10) order by id1", "[[INT64(1)] [INT64(2)] [INT64(4)] [INT64(10)]]") + utils.AssertMatches(t, conn, "select id1 from t1 where id1 in (1, 2, 4, 10) order by id1", "[[INT64(1)] [INT64(2)] [INT64(10)]]") - // Cleanup + // Cleanup: must target the specific shard to delete the mis-routed row for id1=4 + utils.Exec(t, conn, "USE `ks:-80`") utils.Exec(t, conn, "delete from t1 where id1 in (1, 2, 4, 10)") } From e872db54c2e1fb86355cf18aae7f20cc70cfadbf Mon Sep 17 00:00:00 2001 From: Nick Van Wiggeren Date: Mon, 27 Oct 2025 10:39:54 +0100 Subject: [PATCH 05/14] require that a shard be passed in when doing tablet targeting Signed-off-by: Nick Van Wiggeren --- go/test/endtoend/vtgate/main_test.go | 2 +- go/test/endtoend/vtgate/misc_test.go | 123 +++++++++++++++++++++-- go/vt/topo/topoproto/destination.go | 45 +++++---- go/vt/topo/topoproto/destination_test.go | 59 +++++++++-- 4 files changed, 190 insertions(+), 39 deletions(-) diff --git a/go/test/endtoend/vtgate/main_test.go b/go/test/endtoend/vtgate/main_test.go index 80c81231ced..934df0970ed 100644 --- a/go/test/endtoend/vtgate/main_test.go +++ b/go/test/endtoend/vtgate/main_test.go @@ -74,7 +74,7 @@ func TestMain(m *testing.M) { clusterInstance.VtTabletExtraArgs = append(clusterInstance.VtTabletExtraArgs, "--queryserver-config-max-result-size", "100", "--queryserver-config-terse-errors") - err = clusterInstance.StartKeyspace(*keyspace, []string{"-80", "80-"}, 0, false) + err = clusterInstance.StartKeyspace(*keyspace, []string{"-80", "80-"}, 2, false) if err != nil { return 1 } diff --git a/go/test/endtoend/vtgate/misc_test.go b/go/test/endtoend/vtgate/misc_test.go index 28c78f6cb28..b526ad2b0c4 100644 --- a/go/test/endtoend/vtgate/misc_test.go +++ b/go/test/endtoend/vtgate/misc_test.go @@ -849,29 +849,43 @@ func TestTabletTargeting(t *testing.T) { defer conn.Close() // Get tablet aliases from show vitess_tablets + // Note: In partial keyspace mode, there may be multiple keyspaces (e.g. ks and ks_routed) qr := utils.Exec(t, conn, "show vitess_tablets") require.Greater(t, len(qr.Rows), 0, "no tablets found") - // Find PRIMARY and REPLICA tablets for both shards (-80 and 80-) + // Find PRIMARY and REPLICA tablets for both shards (-80 and 80-) in the test keyspace var primaryShard80Minus string var primaryShard80Plus string + var replicasShard80Minus []string + var replicasShard80Plus []string for _, row := range qr.Rows { + if row[1].ToString() != KeyspaceName { + continue + } shard := row[2].ToString() tabletType := row[3].ToString() + alias := row[5].ToString() switch { case tabletType == "PRIMARY" && shard == "-80": - primaryShard80Minus = row[0].ToString() + primaryShard80Minus = alias case tabletType == "PRIMARY" && shard == "80-": - primaryShard80Plus = row[0].ToString() + primaryShard80Plus = alias + case tabletType == "REPLICA" && shard == "-80": + replicasShard80Minus = append(replicasShard80Minus, alias) + case tabletType == "REPLICA" && shard == "80-": + replicasShard80Plus = append(replicasShard80Plus, alias) } } require.NotEmpty(t, primaryShard80Minus, "no PRIMARY tablet found for -80 shard") require.NotEmpty(t, primaryShard80Plus, "no PRIMARY tablet found for 80- shard") + require.NotEmpty(t, replicasShard80Minus, "no REPLICA tablets found for -80 shard") + require.NotEmpty(t, replicasShard80Plus, "no REPLICA tablets found for 80- shard") // Test 1: Shard targeting bypasses vindex resolution // Insert data that would normally hash to 80- shard, but goes to -80 because of shard targeting - utils.Exec(t, conn, fmt.Sprintf("USE `ks:-80@%s`", primaryShard80Minus)) - utils.Exec(t, conn, "insert into t1(id1, id2) values(1, 100), (2, 200)") + useStmt := fmt.Sprintf("USE `ks:-80@primary|%s`", primaryShard80Minus) + utils.Exec(t, conn, useStmt) + utils.Exec(t, conn, "INSERT into t1(id1, id2) values(1, 100), (2, 200)") // id1=4 hashes to 80-, but we're targeting -80 shard explicitly utils.Exec(t, conn, "insert into t1(id1, id2) values(4, 400)") @@ -884,7 +898,8 @@ func TestTabletTargeting(t *testing.T) { utils.AssertIsEmpty(t, conn, "select id1 from t1 where id1=4") // Test 2: Transaction with tablet-specific routing maintains sticky connection - utils.Exec(t, conn, fmt.Sprintf("USE `ks:-80@%s`", primaryShard80Minus)) + useStmt = fmt.Sprintf("USE `ks:-80@primary|%s`", primaryShard80Minus) + utils.Exec(t, conn, useStmt) utils.Exec(t, conn, "begin") utils.Exec(t, conn, "insert into t1(id1, id2) values(10, 300)") // Subsequent queries in transaction should go to same tablet @@ -893,21 +908,107 @@ func TestTabletTargeting(t *testing.T) { utils.AssertMatches(t, conn, "select id1 from t1 where id1=10", "[[INT64(10)]]") // Test 3: Rollback with tablet-specific routing - utils.Exec(t, conn, fmt.Sprintf("USE `ks:-80@%s`", primaryShard80Minus)) + useStmt = fmt.Sprintf("USE `ks:-80@primary|%s`", primaryShard80Minus) + utils.Exec(t, conn, useStmt) utils.Exec(t, conn, "begin") utils.Exec(t, conn, "insert into t1(id1, id2) values(20, 500)") utils.Exec(t, conn, "rollback") utils.AssertIsEmpty(t, conn, "select id1 from t1 where id1=20") - // Test 4: Clear tablet targeting returns to normal routing + // Test 4: Invalid tablet alias should fail + useStmt = "USE `ks:-80@primary|nonexistent-tablet`" + _, err = conn.ExecuteFetch(useStmt, 1, false) + require.Error(t, err, "query should fail on invalid tablet") + require.Contains(t, err.Error(), "invalid tablet alias in target") + + // Test 5: Tablet alias without shard should fail + useStmt = fmt.Sprintf("USE `ks@primary|%s`", primaryShard80Minus) + _, err = conn.ExecuteFetch(useStmt, 1, false) + require.Error(t, err, "tablet alias must be used with a shard") + + // Test 6: Clear tablet targeting returns to normal routing // With normal routing, the query for id1=4 will be sent to the wrong shard (80-), so it won't be found. // This is expected and demonstrates that vindex routing is back in effect. utils.Exec(t, conn, "USE ks") utils.AssertMatches(t, conn, "select id1 from t1 where id1 in (1, 2, 4, 10) order by id1", "[[INT64(1)] [INT64(2)] [INT64(10)]]") - // Cleanup: must target the specific shard to delete the mis-routed row for id1=4 - utils.Exec(t, conn, "USE `ks:-80`") - utils.Exec(t, conn, "delete from t1 where id1 in (1, 2, 4, 10)") + // Test 7: Targeting a specific REPLICA tablet allows reads but not writes + replicaAlias := replicasShard80Minus[0] + useStmt = fmt.Sprintf("USE `ks:-80@replica|%s`", replicaAlias) + utils.Exec(t, conn, useStmt) + + // Reads should work on replica + utils.AssertMatches(t, conn, "select id1 from t1 where id1 in (1, 2, 4, 10) order by id1", "[[INT64(1)] [INT64(2)] [INT64(4)] [INT64(10)]]") + + // Writes should fail on replica (replicas are read-only) + _, err = conn.ExecuteFetch("insert into t1(id1, id2) values(99, 999)", 1, false) + require.Error(t, err, "write should fail on replica tablet") + require.Contains(t, err.Error(), "1290") + + // Test 8: Targeting different REPLICA tablets in the same shard + secondReplicaAlias := replicasShard80Minus[1] + useStmt = fmt.Sprintf("USE `ks:-80@replica|%s`", secondReplicaAlias) + utils.Exec(t, conn, useStmt) + + // Should still be able to read from this different replica + utils.AssertMatches(t, conn, "select id1 from t1 where id1 in (1, 2, 4, 10) order by id1", "[[INT64(1)] [INT64(2)] [INT64(4)] [INT64(10)]]") + + // Writes should still fail + _, err = conn.ExecuteFetch("insert into t1(id1, id2) values(98, 998)", 1, false) + require.Error(t, err, "write should fail on replica tablet") + require.Contains(t, err.Error(), "1290") + + // Test 9: Write to primary, verify it replicates to replica + // This tests that tablet-specific routing doesn't break replication + useStmt = fmt.Sprintf("USE `ks:-80@primary|%s`", primaryShard80Minus) + utils.Exec(t, conn, useStmt) + utils.Exec(t, conn, "insert into t1(id1, id2) values(50, 5000)") + utils.AssertMatches(t, conn, "select id1 from t1 where id1=50", "[[INT64(50)]]") + + // Switch to replica and verify the data replicated + replicaAlias = replicasShard80Minus[0] + useStmt = fmt.Sprintf("USE `ks:-80@replica|%s`", replicaAlias) + utils.Exec(t, conn, useStmt) + // Give replication a moment to catch up + time.Sleep(100 * time.Millisecond) + utils.AssertMatches(t, conn, "select id1 from t1 where id1=50", "[[INT64(50)]]") + + // Test 10: Query different replicas and verify different server UUIDs + // This proves we're actually hitting different physical tablets + for range 10 { + // Get server UUID from first replica + useStmt = fmt.Sprintf("USE `ks:-80@replica|%s`", replicasShard80Minus[0]) + utils.Exec(t, conn, useStmt) + var uuid1 string + for i := range 5 { + result1 := utils.Exec(t, conn, "SELECT @@server_uuid") + require.NotNil(t, result1) + require.Greater(t, len(result1.Rows), 0) + if i > 0 { + // UUID should be the same across multiple queries to same tablet + require.Equal(t, uuid1, result1.Rows[0][0].ToString()) + } + uuid1 = result1.Rows[0][0].ToString() + } + + // Get server UUID from second replica + useStmt = fmt.Sprintf("USE `ks:-80@replica|%s`", replicasShard80Minus[1]) + utils.Exec(t, conn, useStmt) + var uuid2 string + for i := range 5 { + result2 := utils.Exec(t, conn, "SELECT @@server_uuid") + require.NotNil(t, result2) + require.Greater(t, len(result2.Rows), 0) + if i > 0 { + // UUID should be the same across multiple queries to same tablet + require.Equal(t, uuid2, result2.Rows[0][0].ToString()) + } + uuid2 = result2.Rows[0][0].ToString() + } + + // Server UUIDs should be different, proving we're targeting different tablets + require.NotEqual(t, uuid1, uuid2, "different replicas should have different server UUIDs") + } } // TestDynamicConfig tests the dynamic configurations. diff --git a/go/vt/topo/topoproto/destination.go b/go/vt/topo/topoproto/destination.go index 34ffe03497c..5a04cf67704 100644 --- a/go/vt/topo/topoproto/destination.go +++ b/go/vt/topo/topoproto/destination.go @@ -29,8 +29,10 @@ import ( // ParseDestination parses the string representation of a ShardDestination // of the form keyspace:shard@tablet_type. You can use a / instead of a :. -// It also supports tablet-specific routing with keyspace@tablet-alias where -// tablet-alias is in the format cell-uid (e.g., zone1-0000000100). +// It also supports tablet-specific routing with keyspace:shard@tablet_type|tablet-alias +// where tablet-alias is in the format cell-uid (e.g., zone1-0000000100). +// The tablet_type|tablet-alias syntax explicitly specifies both the expected tablet type +// and the specific tablet to route to (e.g., @replica|zone1-0000000100). func ParseDestination(targetString string, defaultTabletType topodatapb.TabletType) (string, topodatapb.TabletType, key.ShardDestination, *topodatapb.TabletAlias, error) { var dest key.ShardDestination var keyspace string @@ -40,25 +42,27 @@ func ParseDestination(targetString string, defaultTabletType topodatapb.TabletTy last := strings.LastIndexAny(targetString, "@") if last != -1 { afterAt := targetString[last+1:] - // Try parsing as tablet type first (backward compatible) - parsedTabletType, err := ParseTabletType(afterAt) - // If tablet type parsing fails or returns UNKNOWN, try parsing as tablet alias - if err != nil || parsedTabletType == topodatapb.TabletType_UNKNOWN { - // Check if it looks like a tablet alias (contains a dash) - if strings.Contains(afterAt, "-") { - alias, aliasErr := ParseTabletAlias(afterAt) - if aliasErr == nil { - tabletAlias = alias - // Keep tabletType as defaultTabletType when using tablet alias - } else { - // If both tablet type and tablet alias parsing fail, keep the UNKNOWN tablet type - // which will cause appropriate error handling downstream - tabletType = topodatapb.TabletType_UNKNOWN - } + // Check for explicit tablet_type|tablet_alias syntax (e.g., "replica|zone1-0000000100") + if pipeIdx := strings.Index(afterAt, "|"); pipeIdx != -1 { + typeStr := afterAt[:pipeIdx] + aliasStr := afterAt[pipeIdx+1:] + + // Parse the tablet type + parsedTabletType, err := ParseTabletType(typeStr) + if err != nil || parsedTabletType == topodatapb.TabletType_UNKNOWN { + return "", defaultTabletType, nil, nil, vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "invalid tablet type in target: %s", typeStr) } - } else { - // Successfully parsed as tablet type tabletType = parsedTabletType + + // Parse the tablet alias + alias, err := ParseTabletAlias(aliasStr) + if err != nil { + return "", defaultTabletType, nil, nil, vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "invalid tablet alias in target: %s", aliasStr) + } + tabletAlias = alias + } else { + // No pipe: just a tablet type (backward compatible - allow UNKNOWN) + tabletType, _ = ParseTabletType(afterAt) } targetString = targetString[:last] } @@ -96,5 +100,8 @@ func ParseDestination(targetString string, defaultTabletType topodatapb.TabletTy targetString = targetString[:last] } keyspace = targetString + if tabletAlias != nil && dest == nil { + return "", defaultTabletType, nil, nil, vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "tablet alias must be used with a shard") + } return keyspace, tabletType, dest, tabletAlias, nil } diff --git a/go/vt/topo/topoproto/destination_test.go b/go/vt/topo/topoproto/destination_test.go index aa425e99b23..85dbef3d7f8 100644 --- a/go/vt/topo/topoproto/destination_test.go +++ b/go/vt/topo/topoproto/destination_test.go @@ -89,21 +89,23 @@ func TestParseDestination(t *testing.T) { dest: key.DestinationShard("-80"), tabletType: topodatapb.TabletType_PRIMARY, }, { - targetString: "ks@zone1-0000000100", + targetString: "ks:-80@primary|zone1-0000000100", keyspace: "ks", + dest: key.DestinationShard("-80"), tabletType: topodatapb.TabletType_PRIMARY, tabletAlias: &topodatapb.TabletAlias{Cell: "zone1", Uid: 100}, }, { - targetString: "ks:-80@zone1-0000000100", + targetString: "ks:-80@replica|zone1-0000000101", keyspace: "ks", dest: key.DestinationShard("-80"), - tabletType: topodatapb.TabletType_PRIMARY, - tabletAlias: &topodatapb.TabletAlias{Cell: "zone1", Uid: 100}, + tabletType: topodatapb.TabletType_REPLICA, + tabletAlias: &topodatapb.TabletAlias{Cell: "zone1", Uid: 101}, }, { - targetString: "@zone1-0000000200", - keyspace: "", - tabletType: topodatapb.TabletType_PRIMARY, - tabletAlias: &topodatapb.TabletAlias{Cell: "zone1", Uid: 200}, + targetString: "ks:80-@rdonly|zone2-0000000200", + keyspace: "ks", + dest: key.DestinationShard("80-"), + tabletType: topodatapb.TabletType_RDONLY, + tabletAlias: &topodatapb.TabletAlias{Cell: "zone2", Uid: 200}, }} for _, tcase := range testcases { @@ -139,4 +141,45 @@ func TestParseDestination(t *testing.T) { if err == nil || err.Error() != want { t.Errorf("executorExec error: %v, want %s", err, want) } + + _, _, _, _, err = ParseDestination("ks@primary|zone1-0000000100", topodatapb.TabletType_PRIMARY) + want = "tablet alias must be used with a shard" + if err == nil || err.Error() != want { + t.Errorf("ParseDestination error: %v, want %s", err, want) + } + + // Test invalid tablet type in pipe syntax + _, _, _, _, err = ParseDestination("ks:-80@invalid|zone1-0000000100", topodatapb.TabletType_PRIMARY) + want = "invalid tablet type in target: invalid" + if err == nil || err.Error() != want { + t.Errorf("ParseDestination error: %v, want %s", err, want) + } + + // Test invalid tablet alias in pipe syntax + _, _, _, _, err = ParseDestination("ks:-80@primary|invalid-alias", topodatapb.TabletType_PRIMARY) + want = "invalid tablet alias in target: invalid-alias" + if err == nil || err.Error() != want { + t.Errorf("ParseDestination error: %v, want %s", err, want) + } + + // Test unknown tablet type in pipe syntax + _, _, _, _, err = ParseDestination("ks:-80@unknown|zone1-0000000100", topodatapb.TabletType_PRIMARY) + want = "invalid tablet type in target: unknown" + if err == nil || err.Error() != want { + t.Errorf("ParseDestination error: %v, want %s", err, want) + } + + // Test empty tablet type in pipe syntax + _, _, _, _, err = ParseDestination("ks:-80@|zone1-0000000100", topodatapb.TabletType_PRIMARY) + want = "invalid tablet type in target: " + if err == nil || err.Error() != want { + t.Errorf("ParseDestination error: %v, want %s", err, want) + } + + // Test empty tablet alias in pipe syntax + _, _, _, _, err = ParseDestination("ks:-80@primary|", topodatapb.TabletType_PRIMARY) + want = "invalid tablet alias in target: " + if err == nil || err.Error() != want { + t.Errorf("ParseDestination error: %v, want %s", err, want) + } } From b2ae3698e61fb92a59014f07373daf4e79b6fb91 Mon Sep 17 00:00:00 2001 From: Nick Van Wiggeren Date: Fri, 31 Oct 2025 12:04:10 -0700 Subject: [PATCH 06/14] clean up tests Signed-off-by: Nick Van Wiggeren --- go/test/endtoend/vtgate/misc_test.go | 63 ++++++++++------------------ 1 file changed, 23 insertions(+), 40 deletions(-) diff --git a/go/test/endtoend/vtgate/misc_test.go b/go/test/endtoend/vtgate/misc_test.go index b526ad2b0c4..381d54be0c7 100644 --- a/go/test/endtoend/vtgate/misc_test.go +++ b/go/test/endtoend/vtgate/misc_test.go @@ -848,42 +848,25 @@ func TestTabletTargeting(t *testing.T) { require.NoError(t, err) defer conn.Close() - // Get tablet aliases from show vitess_tablets - // Note: In partial keyspace mode, there may be multiple keyspaces (e.g. ks and ks_routed) - qr := utils.Exec(t, conn, "show vitess_tablets") - require.Greater(t, len(qr.Rows), 0, "no tablets found") - - // Find PRIMARY and REPLICA tablets for both shards (-80 and 80-) in the test keyspace - var primaryShard80Minus string - var primaryShard80Plus string - var replicasShard80Minus []string - var replicasShard80Plus []string - for _, row := range qr.Rows { - if row[1].ToString() != KeyspaceName { - continue - } - shard := row[2].ToString() - tabletType := row[3].ToString() - alias := row[5].ToString() - switch { - case tabletType == "PRIMARY" && shard == "-80": - primaryShard80Minus = alias - case tabletType == "PRIMARY" && shard == "80-": - primaryShard80Plus = alias - case tabletType == "REPLICA" && shard == "-80": - replicasShard80Minus = append(replicasShard80Minus, alias) - case tabletType == "REPLICA" && shard == "80-": - replicasShard80Plus = append(replicasShard80Plus, alias) + instances := make(map[string]map[string][]string) + + for _, ks := range clusterInstance.Keyspaces { + for _, shard := range ks.Shards { + instances[shard.Name] = make(map[string][]string) + for _, tablet := range shard.Vttablets { + instances[shard.Name][tablet.Type] = append(instances[shard.Name][tablet.Type], tablet.Alias) + } } } - require.NotEmpty(t, primaryShard80Minus, "no PRIMARY tablet found for -80 shard") - require.NotEmpty(t, primaryShard80Plus, "no PRIMARY tablet found for 80- shard") - require.NotEmpty(t, replicasShard80Minus, "no REPLICA tablets found for -80 shard") - require.NotEmpty(t, replicasShard80Plus, "no REPLICA tablets found for 80- shard") + + require.NotEmpty(t, instances["-80"]["primary"][0], "no PRIMARY tablet found for -80 shard") + require.NotEmpty(t, instances["80-"]["primary"][0], "no PRIMARY tablet found for 80- shard") + require.NotEmpty(t, instances["-80"]["replica"], "no REPLICA tablets found for -80 shard") + require.NotEmpty(t, instances["80-"]["replica"], "no REPLICA tablets found for 80- shard") // Test 1: Shard targeting bypasses vindex resolution // Insert data that would normally hash to 80- shard, but goes to -80 because of shard targeting - useStmt := fmt.Sprintf("USE `ks:-80@primary|%s`", primaryShard80Minus) + useStmt := fmt.Sprintf("USE `ks:-80@primary|%s`", instances["-80"]["primary"][0]) utils.Exec(t, conn, useStmt) utils.Exec(t, conn, "INSERT into t1(id1, id2) values(1, 100), (2, 200)") // id1=4 hashes to 80-, but we're targeting -80 shard explicitly @@ -898,7 +881,7 @@ func TestTabletTargeting(t *testing.T) { utils.AssertIsEmpty(t, conn, "select id1 from t1 where id1=4") // Test 2: Transaction with tablet-specific routing maintains sticky connection - useStmt = fmt.Sprintf("USE `ks:-80@primary|%s`", primaryShard80Minus) + useStmt = fmt.Sprintf("USE `ks:-80@primary|%s`", instances["-80"]["primary"][0]) utils.Exec(t, conn, useStmt) utils.Exec(t, conn, "begin") utils.Exec(t, conn, "insert into t1(id1, id2) values(10, 300)") @@ -908,7 +891,7 @@ func TestTabletTargeting(t *testing.T) { utils.AssertMatches(t, conn, "select id1 from t1 where id1=10", "[[INT64(10)]]") // Test 3: Rollback with tablet-specific routing - useStmt = fmt.Sprintf("USE `ks:-80@primary|%s`", primaryShard80Minus) + useStmt = fmt.Sprintf("USE `ks:-80@primary|%s`", instances["-80"]["primary"][0]) utils.Exec(t, conn, useStmt) utils.Exec(t, conn, "begin") utils.Exec(t, conn, "insert into t1(id1, id2) values(20, 500)") @@ -922,7 +905,7 @@ func TestTabletTargeting(t *testing.T) { require.Contains(t, err.Error(), "invalid tablet alias in target") // Test 5: Tablet alias without shard should fail - useStmt = fmt.Sprintf("USE `ks@primary|%s`", primaryShard80Minus) + useStmt = fmt.Sprintf("USE `ks@primary|%s`", instances["-80"]["primary"][0]) _, err = conn.ExecuteFetch(useStmt, 1, false) require.Error(t, err, "tablet alias must be used with a shard") @@ -933,7 +916,7 @@ func TestTabletTargeting(t *testing.T) { utils.AssertMatches(t, conn, "select id1 from t1 where id1 in (1, 2, 4, 10) order by id1", "[[INT64(1)] [INT64(2)] [INT64(10)]]") // Test 7: Targeting a specific REPLICA tablet allows reads but not writes - replicaAlias := replicasShard80Minus[0] + replicaAlias := instances["-80"]["replica"][0] useStmt = fmt.Sprintf("USE `ks:-80@replica|%s`", replicaAlias) utils.Exec(t, conn, useStmt) @@ -946,7 +929,7 @@ func TestTabletTargeting(t *testing.T) { require.Contains(t, err.Error(), "1290") // Test 8: Targeting different REPLICA tablets in the same shard - secondReplicaAlias := replicasShard80Minus[1] + secondReplicaAlias := instances["-80"]["replica"][1] useStmt = fmt.Sprintf("USE `ks:-80@replica|%s`", secondReplicaAlias) utils.Exec(t, conn, useStmt) @@ -960,13 +943,13 @@ func TestTabletTargeting(t *testing.T) { // Test 9: Write to primary, verify it replicates to replica // This tests that tablet-specific routing doesn't break replication - useStmt = fmt.Sprintf("USE `ks:-80@primary|%s`", primaryShard80Minus) + useStmt = fmt.Sprintf("USE `ks:-80@primary|%s`", instances["-80"]["primary"][0]) utils.Exec(t, conn, useStmt) utils.Exec(t, conn, "insert into t1(id1, id2) values(50, 5000)") utils.AssertMatches(t, conn, "select id1 from t1 where id1=50", "[[INT64(50)]]") // Switch to replica and verify the data replicated - replicaAlias = replicasShard80Minus[0] + replicaAlias = instances["-80"]["replica"][0] useStmt = fmt.Sprintf("USE `ks:-80@replica|%s`", replicaAlias) utils.Exec(t, conn, useStmt) // Give replication a moment to catch up @@ -977,7 +960,7 @@ func TestTabletTargeting(t *testing.T) { // This proves we're actually hitting different physical tablets for range 10 { // Get server UUID from first replica - useStmt = fmt.Sprintf("USE `ks:-80@replica|%s`", replicasShard80Minus[0]) + useStmt = fmt.Sprintf("USE `ks:-80@replica|%s`", instances["-80"]["replica"][0]) utils.Exec(t, conn, useStmt) var uuid1 string for i := range 5 { @@ -992,7 +975,7 @@ func TestTabletTargeting(t *testing.T) { } // Get server UUID from second replica - useStmt = fmt.Sprintf("USE `ks:-80@replica|%s`", replicasShard80Minus[1]) + useStmt = fmt.Sprintf("USE `ks:-80@replica|%s`", instances["-80"]["replica"][1]) utils.Exec(t, conn, useStmt) var uuid2 string for i := range 5 { From 1ec01a987aa3d933691b9f3a8361cb4f60a9134e Mon Sep 17 00:00:00 2001 From: Nick Van Wiggeren Date: Fri, 31 Oct 2025 15:12:40 -0700 Subject: [PATCH 07/14] fix tests with partial ks Signed-off-by: Nick Van Wiggeren --- go/test/endtoend/vtgate/misc_test.go | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/go/test/endtoend/vtgate/misc_test.go b/go/test/endtoend/vtgate/misc_test.go index 381d54be0c7..5222c3e284c 100644 --- a/go/test/endtoend/vtgate/misc_test.go +++ b/go/test/endtoend/vtgate/misc_test.go @@ -851,6 +851,9 @@ func TestTabletTargeting(t *testing.T) { instances := make(map[string]map[string][]string) for _, ks := range clusterInstance.Keyspaces { + if ks.Name != "ks" { + continue + } for _, shard := range ks.Shards { instances[shard.Name] = make(map[string][]string) for _, tablet := range shard.Vttablets { @@ -864,7 +867,6 @@ func TestTabletTargeting(t *testing.T) { require.NotEmpty(t, instances["-80"]["replica"], "no REPLICA tablets found for -80 shard") require.NotEmpty(t, instances["80-"]["replica"], "no REPLICA tablets found for 80- shard") - // Test 1: Shard targeting bypasses vindex resolution // Insert data that would normally hash to 80- shard, but goes to -80 because of shard targeting useStmt := fmt.Sprintf("USE `ks:-80@primary|%s`", instances["-80"]["primary"][0]) utils.Exec(t, conn, useStmt) @@ -880,7 +882,7 @@ func TestTabletTargeting(t *testing.T) { utils.Exec(t, conn, "USE `ks:80-`") utils.AssertIsEmpty(t, conn, "select id1 from t1 where id1=4") - // Test 2: Transaction with tablet-specific routing maintains sticky connection + // Transaction with tablet-specific routing maintains sticky connection useStmt = fmt.Sprintf("USE `ks:-80@primary|%s`", instances["-80"]["primary"][0]) utils.Exec(t, conn, useStmt) utils.Exec(t, conn, "begin") @@ -890,7 +892,7 @@ func TestTabletTargeting(t *testing.T) { utils.Exec(t, conn, "commit") utils.AssertMatches(t, conn, "select id1 from t1 where id1=10", "[[INT64(10)]]") - // Test 3: Rollback with tablet-specific routing + // Rollback with tablet-specific routing useStmt = fmt.Sprintf("USE `ks:-80@primary|%s`", instances["-80"]["primary"][0]) utils.Exec(t, conn, useStmt) utils.Exec(t, conn, "begin") @@ -898,24 +900,24 @@ func TestTabletTargeting(t *testing.T) { utils.Exec(t, conn, "rollback") utils.AssertIsEmpty(t, conn, "select id1 from t1 where id1=20") - // Test 4: Invalid tablet alias should fail + // Invalid tablet alias should fail useStmt = "USE `ks:-80@primary|nonexistent-tablet`" _, err = conn.ExecuteFetch(useStmt, 1, false) require.Error(t, err, "query should fail on invalid tablet") require.Contains(t, err.Error(), "invalid tablet alias in target") - // Test 5: Tablet alias without shard should fail + // Tablet alias without shard should fail useStmt = fmt.Sprintf("USE `ks@primary|%s`", instances["-80"]["primary"][0]) _, err = conn.ExecuteFetch(useStmt, 1, false) require.Error(t, err, "tablet alias must be used with a shard") - // Test 6: Clear tablet targeting returns to normal routing + // Clear tablet targeting returns to normal routing // With normal routing, the query for id1=4 will be sent to the wrong shard (80-), so it won't be found. // This is expected and demonstrates that vindex routing is back in effect. utils.Exec(t, conn, "USE ks") utils.AssertMatches(t, conn, "select id1 from t1 where id1 in (1, 2, 4, 10) order by id1", "[[INT64(1)] [INT64(2)] [INT64(10)]]") - // Test 7: Targeting a specific REPLICA tablet allows reads but not writes + // Targeting a specific REPLICA tablet allows reads but not writes replicaAlias := instances["-80"]["replica"][0] useStmt = fmt.Sprintf("USE `ks:-80@replica|%s`", replicaAlias) utils.Exec(t, conn, useStmt) @@ -928,7 +930,7 @@ func TestTabletTargeting(t *testing.T) { require.Error(t, err, "write should fail on replica tablet") require.Contains(t, err.Error(), "1290") - // Test 8: Targeting different REPLICA tablets in the same shard + // Targeting different REPLICA tablets in the same shard secondReplicaAlias := instances["-80"]["replica"][1] useStmt = fmt.Sprintf("USE `ks:-80@replica|%s`", secondReplicaAlias) utils.Exec(t, conn, useStmt) @@ -941,7 +943,7 @@ func TestTabletTargeting(t *testing.T) { require.Error(t, err, "write should fail on replica tablet") require.Contains(t, err.Error(), "1290") - // Test 9: Write to primary, verify it replicates to replica + // Write to primary, verify it replicates to replica // This tests that tablet-specific routing doesn't break replication useStmt = fmt.Sprintf("USE `ks:-80@primary|%s`", instances["-80"]["primary"][0]) utils.Exec(t, conn, useStmt) @@ -956,7 +958,7 @@ func TestTabletTargeting(t *testing.T) { time.Sleep(100 * time.Millisecond) utils.AssertMatches(t, conn, "select id1 from t1 where id1=50", "[[INT64(50)]]") - // Test 10: Query different replicas and verify different server UUIDs + // Query different replicas and verify different server UUIDs // This proves we're actually hitting different physical tablets for range 10 { // Get server UUID from first replica From 5fe677e7bbe4b2a3824de9c653b02a1bbc4f1b8f Mon Sep 17 00:00:00 2001 From: Nick Van Wiggeren Date: Fri, 5 Dec 2025 19:43:55 +0200 Subject: [PATCH 08/14] code review feedback: test improvements & do not set target alias where it is not valid to set Signed-off-by: Nick Van Wiggeren --- go/test/endtoend/vtgate/misc_test.go | 59 +++++++++---------- .../executorcontext/safe_session_test.go | 16 ----- go/vt/vtgate/executorcontext/vcursor_impl.go | 11 +++- go/vt/vtgate/scatter_conn.go | 10 +++- 4 files changed, 45 insertions(+), 51 deletions(-) diff --git a/go/test/endtoend/vtgate/misc_test.go b/go/test/endtoend/vtgate/misc_test.go index 5222c3e284c..f216e0723b1 100644 --- a/go/test/endtoend/vtgate/misc_test.go +++ b/go/test/endtoend/vtgate/misc_test.go @@ -960,40 +960,39 @@ func TestTabletTargeting(t *testing.T) { // Query different replicas and verify different server UUIDs // This proves we're actually hitting different physical tablets - for range 10 { - // Get server UUID from first replica - useStmt = fmt.Sprintf("USE `ks:-80@replica|%s`", instances["-80"]["replica"][0]) - utils.Exec(t, conn, useStmt) - var uuid1 string - for i := range 5 { - result1 := utils.Exec(t, conn, "SELECT @@server_uuid") - require.NotNil(t, result1) - require.Greater(t, len(result1.Rows), 0) - if i > 0 { - // UUID should be the same across multiple queries to same tablet - require.Equal(t, uuid1, result1.Rows[0][0].ToString()) - } - uuid1 = result1.Rows[0][0].ToString() - } - // Get server UUID from second replica - useStmt = fmt.Sprintf("USE `ks:-80@replica|%s`", instances["-80"]["replica"][1]) - utils.Exec(t, conn, useStmt) - var uuid2 string - for i := range 5 { - result2 := utils.Exec(t, conn, "SELECT @@server_uuid") - require.NotNil(t, result2) - require.Greater(t, len(result2.Rows), 0) - if i > 0 { - // UUID should be the same across multiple queries to same tablet - require.Equal(t, uuid2, result2.Rows[0][0].ToString()) - } - uuid2 = result2.Rows[0][0].ToString() + // Get server UUID from first replica + useStmt = fmt.Sprintf("USE `ks:-80@replica|%s`", instances["-80"]["replica"][0]) + utils.Exec(t, conn, useStmt) + var uuid1 string + for i := range 5 { + result1 := utils.Exec(t, conn, "SELECT @@server_uuid") + require.NotNil(t, result1) + require.Greater(t, len(result1.Rows), 0) + if i > 0 { + // UUID should be the same across multiple queries to same tablet + require.Equal(t, uuid1, result1.Rows[0][0].ToString()) } + uuid1 = result1.Rows[0][0].ToString() + } - // Server UUIDs should be different, proving we're targeting different tablets - require.NotEqual(t, uuid1, uuid2, "different replicas should have different server UUIDs") + // Get server UUID from second replica + useStmt = fmt.Sprintf("USE `ks:-80@replica|%s`", instances["-80"]["replica"][1]) + utils.Exec(t, conn, useStmt) + var uuid2 string + for i := range 5 { + result2 := utils.Exec(t, conn, "SELECT @@server_uuid") + require.NotNil(t, result2) + require.Greater(t, len(result2.Rows), 0) + if i > 0 { + // UUID should be the same across multiple queries to same tablet + require.Equal(t, uuid2, result2.Rows[0][0].ToString()) + } + uuid2 = result2.Rows[0][0].ToString() } + + // Server UUIDs should be different, proving we're targeting different tablets + require.NotEqual(t, uuid1, uuid2, "different replicas should have different server UUIDs") } // TestDynamicConfig tests the dynamic configurations. diff --git a/go/vt/vtgate/executorcontext/safe_session_test.go b/go/vt/vtgate/executorcontext/safe_session_test.go index 4430f1b68d4..c3144344586 100644 --- a/go/vt/vtgate/executorcontext/safe_session_test.go +++ b/go/vt/vtgate/executorcontext/safe_session_test.go @@ -222,20 +222,4 @@ func TestTargetTabletAlias(t *testing.T) { // Test: Clear (set to nil) session.SetTargetTabletAlias(nil) assert.Nil(t, session.GetTargetTabletAlias()) - - // Test: Thread safety - concurrent access - done := make(chan bool) - for i := 0; i < 100; i++ { - go func(uid uint32) { - testAlias := &topodatapb.TabletAlias{Cell: "cell", Uid: uid} - session.SetTargetTabletAlias(testAlias) - _ = session.GetTargetTabletAlias() - done <- true - }(uint32(i)) - } - for i := 0; i < 100; i++ { - <-done - } - // Just verify we didn't panic - the final value is non-deterministic - assert.NotNil(t, session.GetTargetTabletAlias()) } diff --git a/go/vt/vtgate/executorcontext/vcursor_impl.go b/go/vt/vtgate/executorcontext/vcursor_impl.go index 6c3b5750429..c8cb85fe422 100644 --- a/go/vt/vtgate/executorcontext/vcursor_impl.go +++ b/go/vt/vtgate/executorcontext/vcursor_impl.go @@ -1041,13 +1041,18 @@ func (vc *VCursorImpl) SetTarget(target string) error { if tabletAlias == nil { vc.SafeSession.SetTargetTabletAlias(nil) } else { + // Tablet targeting must be set before starting a transaction, not during. + // The feature is designed to pick a specific tablet and do work there, + // not to switch tablets mid-transaction. + if vc.SafeSession.InTransaction() { + return vterrors.Errorf(vtrpcpb.Code_FAILED_PRECONDITION, + "cannot set tablet target while in a transaction") + } + // Store tablet alias in session for routing // Note: We don't validate the tablet exists here - if it doesn't exist or is // unreachable, the query will fail during execution with a clear error message vc.SafeSession.SetTargetTabletAlias(tabletAlias) - - // Keep tabletType as determined by ParseDestination (defaultTabletType) - // The actual routing uses the tablet alias, so the type is only for VCursor state } if _, ok := vc.vschema.Keyspaces[keyspace]; !ignoreKeyspace(keyspace) && !ok { diff --git a/go/vt/vtgate/scatter_conn.go b/go/vt/vtgate/scatter_conn.go index 8a11091d4e0..893c6533e08 100644 --- a/go/vt/vtgate/scatter_conn.go +++ b/go/vt/vtgate/scatter_conn.go @@ -903,9 +903,15 @@ func actionInfo(ctx context.Context, target *querypb.Target, session *econtext.S info.alias = shardSession.TabletAlias info.rowsAffected = shardSession.RowsAffected } - // Override alias if tablet-specific routing is set + // Set tablet alias for routing if tablet-specific targeting is active if targetAlias := session.GetTargetTabletAlias(); targetAlias != nil { - info.alias = targetAlias + if info.alias == nil { + info.alias = targetAlias + } else if !proto.Equal(info.alias, targetAlias) { + return nil, nil, vterrors.Errorf(vtrpcpb.Code_FAILED_PRECONDITION, + "cannot change tablet target mid-transaction: session has %v, target is %v", + topoproto.TabletAliasString(info.alias), topoproto.TabletAliasString(targetAlias)) + } } return info, shardSession, nil } From f638a6249d61e421702746dd70c518709c1acac7 Mon Sep 17 00:00:00 2001 From: Nick Van Wiggeren Date: Sun, 7 Dec 2025 08:45:33 +0200 Subject: [PATCH 09/14] cleanup handling SetTarget Signed-off-by: Nick Van Wiggeren --- go/vt/vtgate/executorcontext/vcursor_impl.go | 20 ++++---------------- 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/go/vt/vtgate/executorcontext/vcursor_impl.go b/go/vt/vtgate/executorcontext/vcursor_impl.go index c8cb85fe422..8705eb7ffd2 100644 --- a/go/vt/vtgate/executorcontext/vcursor_impl.go +++ b/go/vt/vtgate/executorcontext/vcursor_impl.go @@ -1037,22 +1037,10 @@ func (vc *VCursorImpl) SetTarget(target string) error { return err } - // Clear any existing tablet-specific routing if not specified - if tabletAlias == nil { - vc.SafeSession.SetTargetTabletAlias(nil) - } else { - // Tablet targeting must be set before starting a transaction, not during. - // The feature is designed to pick a specific tablet and do work there, - // not to switch tablets mid-transaction. - if vc.SafeSession.InTransaction() { - return vterrors.Errorf(vtrpcpb.Code_FAILED_PRECONDITION, - "cannot set tablet target while in a transaction") - } - - // Store tablet alias in session for routing - // Note: We don't validate the tablet exists here - if it doesn't exist or is - // unreachable, the query will fail during execution with a clear error message - vc.SafeSession.SetTargetTabletAlias(tabletAlias) + // Tablet targeting must be set before starting a transaction, not during. + if tabletAlias != nil && vc.SafeSession.InTransaction() { + return vterrors.Errorf(vtrpcpb.Code_FAILED_PRECONDITION, + "cannot set tablet target while in a transaction") } if _, ok := vc.vschema.Keyspaces[keyspace]; !ignoreKeyspace(keyspace) && !ok { From 25a0e9c07abb18927fcbe4eb28aff658f3c96f57 Mon Sep 17 00:00:00 2001 From: Nick Van Wiggeren Date: Sun, 7 Dec 2025 08:53:27 +0200 Subject: [PATCH 10/14] update test to properly fail while in txn Signed-off-by: Nick Van Wiggeren --- go/vt/vtgate/scatter_conn_test.go | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/go/vt/vtgate/scatter_conn_test.go b/go/vt/vtgate/scatter_conn_test.go index f096a159e99..141de64b0f9 100644 --- a/go/vt/vtgate/scatter_conn_test.go +++ b/go/vt/vtgate/scatter_conn_test.go @@ -667,7 +667,7 @@ func TestActionInfoWithTabletAlias(t *testing.T) { assert.Equal(t, tabletAlias, info.alias) }) - t.Run("existing transaction with tablet alias", func(t *testing.T) { + t.Run("existing transaction with different tablet alias errors", func(t *testing.T) { session := econtext.NewSafeSession(&vtgatepb.Session{ InTransaction: true, ShardSessions: []*vtgatepb.Session_ShardSession{{ @@ -676,15 +676,11 @@ func TestActionInfoWithTabletAlias(t *testing.T) { TabletAlias: &topodatapb.TabletAlias{Cell: "zone1", Uid: 50}, }}, }) - session.SetTargetTabletAlias(tabletAlias) + session.SetTargetTabletAlias(tabletAlias) // zone1-100, different from zone1-50 - info, shardSession, err := actionInfo(ctx, target, session, false, vtgatepb.TransactionMode_MULTI) - require.NoError(t, err) - assert.NotNil(t, shardSession) - assert.Equal(t, int64(12345), info.transactionID) - assert.Equal(t, nothing, info.actionNeeded) - // Tablet alias should be overridden - assert.Equal(t, tabletAlias, info.alias) + _, _, err := actionInfo(ctx, target, session, false, vtgatepb.TransactionMode_MULTI) + require.Error(t, err) + require.Contains(t, err.Error(), "cannot change tablet target mid-transaction") }) t.Run("no tablet alias - existing behavior", func(t *testing.T) { From 5c0bc2dfdd7e70d58d7671163404f1f4cb7c4c1d Mon Sep 17 00:00:00 2001 From: Nick Van Wiggeren Date: Mon, 8 Dec 2025 08:48:15 +0200 Subject: [PATCH 11/14] move test to using require.Eventually and not time.Sleep Signed-off-by: Nick Van Wiggeren --- go/test/endtoend/vtgate/misc_test.go | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/go/test/endtoend/vtgate/misc_test.go b/go/test/endtoend/vtgate/misc_test.go index f216e0723b1..e802920a22b 100644 --- a/go/test/endtoend/vtgate/misc_test.go +++ b/go/test/endtoend/vtgate/misc_test.go @@ -922,8 +922,11 @@ func TestTabletTargeting(t *testing.T) { useStmt = fmt.Sprintf("USE `ks:-80@replica|%s`", replicaAlias) utils.Exec(t, conn, useStmt) - // Reads should work on replica - utils.AssertMatches(t, conn, "select id1 from t1 where id1 in (1, 2, 4, 10) order by id1", "[[INT64(1)] [INT64(2)] [INT64(4)] [INT64(10)]]") + // Reads should work on replica (wait for replication) + require.Eventually(t, func() bool { + result, err := conn.ExecuteFetch("select id1 from t1 where id1 in (1, 2, 4, 10) order by id1", 10, false) + return err == nil && len(result.Rows) == 4 + }, 15*time.Second, 100*time.Millisecond, "replication did not catch up for first replica read") // Writes should fail on replica (replicas are read-only) _, err = conn.ExecuteFetch("insert into t1(id1, id2) values(99, 999)", 1, false) @@ -935,8 +938,11 @@ func TestTabletTargeting(t *testing.T) { useStmt = fmt.Sprintf("USE `ks:-80@replica|%s`", secondReplicaAlias) utils.Exec(t, conn, useStmt) - // Should still be able to read from this different replica - utils.AssertMatches(t, conn, "select id1 from t1 where id1 in (1, 2, 4, 10) order by id1", "[[INT64(1)] [INT64(2)] [INT64(4)] [INT64(10)]]") + // Should still be able to read from this different replica (wait for replication) + require.Eventually(t, func() bool { + result, err := conn.ExecuteFetch("select id1 from t1 where id1 in (1, 2, 4, 10) order by id1", 10, false) + return err == nil && len(result.Rows) == 4 + }, 15*time.Second, 100*time.Millisecond, "replication did not catch up for second replica read") // Writes should still fail _, err = conn.ExecuteFetch("insert into t1(id1, id2) values(98, 998)", 1, false) @@ -954,9 +960,11 @@ func TestTabletTargeting(t *testing.T) { replicaAlias = instances["-80"]["replica"][0] useStmt = fmt.Sprintf("USE `ks:-80@replica|%s`", replicaAlias) utils.Exec(t, conn, useStmt) - // Give replication a moment to catch up - time.Sleep(100 * time.Millisecond) - utils.AssertMatches(t, conn, "select id1 from t1 where id1=50", "[[INT64(50)]]") + // Wait for replication to catch up + require.Eventually(t, func() bool { + result, err := conn.ExecuteFetch("select id1 from t1 where id1=50", 1, false) + return err == nil && len(result.Rows) == 1 + }, 15*time.Second, 100*time.Millisecond, "replication did not catch up") // Query different replicas and verify different server UUIDs // This proves we're actually hitting different physical tablets From a637fc9ecb5308ad6b725ed7be0180221bcc09c9 Mon Sep 17 00:00:00 2001 From: Nick Van Wiggeren Date: Mon, 8 Dec 2025 10:02:11 +0200 Subject: [PATCH 12/14] add changelog information for tablet targeting Signed-off-by: Nick Van Wiggeren --- changelog/24.0/24.0.0/summary.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/changelog/24.0/24.0.0/summary.md b/changelog/24.0/24.0.0/summary.md index 696da1ef99c..f44aa575ef2 100644 --- a/changelog/24.0/24.0.0/summary.md +++ b/changelog/24.0/24.0.0/summary.md @@ -6,6 +6,7 @@ - **[Major Changes](#major-changes)** - **[New Support](#new-support)** - [Window function pushdown for sharded keyspaces](#window-function-pushdown) + - [Tablet targeting via USE statement](#tablet-targeting) - **[Minor Changes](#minor-changes)** - **[VTGate](#minor-changes-vtgate)** - [New default for `--legacy-replication-lag-algorithm` flag](#vtgate-new-default-legacy-replication-lag-algorithm) @@ -24,6 +25,22 @@ Previously, all window function queries required single-shard routing, which lim For examples and more details, see the [documentation](https://vitess.io/docs/24.0/reference/compatibility/mysql-compatibility/#window-functions). +#### Tablet targeting via USE statement + +VTGate now supports routing queries to a specific tablet by alias using an extended `USE` statement syntax: + +```sql +USE keyspace:shard@tablet_type|tablet_alias; +``` + +For example, to target a specific primary tablet: + +```sql +USE commerce:-80@primary|zone1-0000000100; +``` + +Once set, all subsequent queries in the session route to the specified tablet until cleared with a standard `USE keyspace` or `USE keyspace@tablet_type` statement. This is useful for debugging, per-tablet monitoring, cache warming, and other operational tasks where targeting a specific tablet is required. + ## Minor Changes ### VTGate From 87727323a73b4642e5c97f9f9a23d84acb394225 Mon Sep 17 00:00:00 2001 From: Nick Van Wiggeren Date: Fri, 16 Jan 2026 18:57:55 +0000 Subject: [PATCH 13/14] Address review comments from mattlord - Fix comment in safe_session.go to use correct syntax - Add bounds check for empty string after @ in ParseDestination - Use require.EqualError instead of manual error checking in tests - Change %v to %s for string formatting in error message - Use require.ErrorContains instead of require.Error + require.Contains Signed-off-by: Nick Van Wiggeren --- go/vt/topo/topoproto/destination.go | 3 ++ go/vt/topo/topoproto/destination_test.go | 51 ++++++-------------- go/vt/vtgate/executorcontext/safe_session.go | 2 +- go/vt/vtgate/scatter_conn.go | 2 +- go/vt/vtgate/scatter_conn_test.go | 3 +- 5 files changed, 21 insertions(+), 40 deletions(-) diff --git a/go/vt/topo/topoproto/destination.go b/go/vt/topo/topoproto/destination.go index 5a04cf67704..1aa00d36404 100644 --- a/go/vt/topo/topoproto/destination.go +++ b/go/vt/topo/topoproto/destination.go @@ -42,6 +42,9 @@ func ParseDestination(targetString string, defaultTabletType topodatapb.TabletTy last := strings.LastIndexAny(targetString, "@") if last != -1 { afterAt := targetString[last+1:] + if afterAt == "" { + return "", defaultTabletType, nil, nil, vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "empty tablet type after @") + } // Check for explicit tablet_type|tablet_alias syntax (e.g., "replica|zone1-0000000100") if pipeIdx := strings.Index(afterAt, "|"); pipeIdx != -1 { typeStr := afterAt[:pipeIdx] diff --git a/go/vt/topo/topoproto/destination_test.go b/go/vt/topo/topoproto/destination_test.go index 85dbef3d7f8..22737877f53 100644 --- a/go/vt/topo/topoproto/destination_test.go +++ b/go/vt/topo/topoproto/destination_test.go @@ -21,6 +21,8 @@ import ( "reflect" "testing" + "github.com/stretchr/testify/require" + "vitess.io/vitess/go/vt/key" topodatapb "vitess.io/vitess/go/vt/proto/topodata" @@ -125,61 +127,38 @@ func TestParseDestination(t *testing.T) { } _, _, _, _, err := ParseDestination("ks[20-40-60]", topodatapb.TabletType_PRIMARY) - want := "single keyrange expected in 20-40-60" - if err == nil || err.Error() != want { - t.Errorf("executorExec error: %v, want %s", err, want) - } + require.EqualError(t, err, "single keyrange expected in 20-40-60") _, _, _, _, err = ParseDestination("ks[--60]", topodatapb.TabletType_PRIMARY) - want = "malformed spec: MinKey/MaxKey cannot be in the middle of the spec: \"--60\"" - if err == nil || err.Error() != want { - t.Errorf("executorExec error: %v, want %s", err, want) - } + require.EqualError(t, err, "malformed spec: MinKey/MaxKey cannot be in the middle of the spec: \"--60\"") _, _, _, _, err = ParseDestination("ks[qrnqorrs]@primary", topodatapb.TabletType_PRIMARY) - want = "expected valid hex in keyspace id qrnqorrs" - if err == nil || err.Error() != want { - t.Errorf("executorExec error: %v, want %s", err, want) - } + require.EqualError(t, err, "expected valid hex in keyspace id qrnqorrs") _, _, _, _, err = ParseDestination("ks@primary|zone1-0000000100", topodatapb.TabletType_PRIMARY) - want = "tablet alias must be used with a shard" - if err == nil || err.Error() != want { - t.Errorf("ParseDestination error: %v, want %s", err, want) - } + require.EqualError(t, err, "tablet alias must be used with a shard") // Test invalid tablet type in pipe syntax _, _, _, _, err = ParseDestination("ks:-80@invalid|zone1-0000000100", topodatapb.TabletType_PRIMARY) - want = "invalid tablet type in target: invalid" - if err == nil || err.Error() != want { - t.Errorf("ParseDestination error: %v, want %s", err, want) - } + require.EqualError(t, err, "invalid tablet type in target: invalid") // Test invalid tablet alias in pipe syntax _, _, _, _, err = ParseDestination("ks:-80@primary|invalid-alias", topodatapb.TabletType_PRIMARY) - want = "invalid tablet alias in target: invalid-alias" - if err == nil || err.Error() != want { - t.Errorf("ParseDestination error: %v, want %s", err, want) - } + require.EqualError(t, err, "invalid tablet alias in target: invalid-alias") // Test unknown tablet type in pipe syntax _, _, _, _, err = ParseDestination("ks:-80@unknown|zone1-0000000100", topodatapb.TabletType_PRIMARY) - want = "invalid tablet type in target: unknown" - if err == nil || err.Error() != want { - t.Errorf("ParseDestination error: %v, want %s", err, want) - } + require.EqualError(t, err, "invalid tablet type in target: unknown") // Test empty tablet type in pipe syntax _, _, _, _, err = ParseDestination("ks:-80@|zone1-0000000100", topodatapb.TabletType_PRIMARY) - want = "invalid tablet type in target: " - if err == nil || err.Error() != want { - t.Errorf("ParseDestination error: %v, want %s", err, want) - } + require.EqualError(t, err, "invalid tablet type in target: ") // Test empty tablet alias in pipe syntax _, _, _, _, err = ParseDestination("ks:-80@primary|", topodatapb.TabletType_PRIMARY) - want = "invalid tablet alias in target: " - if err == nil || err.Error() != want { - t.Errorf("ParseDestination error: %v, want %s", err, want) - } + require.EqualError(t, err, "invalid tablet alias in target: ") + + // Test empty string after @ + _, _, _, _, err = ParseDestination("ks@", topodatapb.TabletType_PRIMARY) + require.EqualError(t, err, "empty tablet type after @") } diff --git a/go/vt/vtgate/executorcontext/safe_session.go b/go/vt/vtgate/executorcontext/safe_session.go index 6042ce04ce9..2d3116ec21e 100644 --- a/go/vt/vtgate/executorcontext/safe_session.go +++ b/go/vt/vtgate/executorcontext/safe_session.go @@ -65,7 +65,7 @@ type ( logging *ExecuteLogger - // targetTabletAlias is set when using tablet-specific routing via USE keyspace@tablet-alias. + // targetTabletAlias is set when using tablet-specific routing via USE keyspace:shard@tablet_type|tablet-alias. // This causes all queries to route to the specified tablet until cleared. // Note: This is stored in the Go wrapper, not in the protobuf Session. targetTabletAlias *topodatapb.TabletAlias diff --git a/go/vt/vtgate/scatter_conn.go b/go/vt/vtgate/scatter_conn.go index 893c6533e08..f928f4c275f 100644 --- a/go/vt/vtgate/scatter_conn.go +++ b/go/vt/vtgate/scatter_conn.go @@ -909,7 +909,7 @@ func actionInfo(ctx context.Context, target *querypb.Target, session *econtext.S info.alias = targetAlias } else if !proto.Equal(info.alias, targetAlias) { return nil, nil, vterrors.Errorf(vtrpcpb.Code_FAILED_PRECONDITION, - "cannot change tablet target mid-transaction: session has %v, target is %v", + "cannot change tablet target mid-transaction: session has %s, target is %s", topoproto.TabletAliasString(info.alias), topoproto.TabletAliasString(targetAlias)) } } diff --git a/go/vt/vtgate/scatter_conn_test.go b/go/vt/vtgate/scatter_conn_test.go index 141de64b0f9..d9488f4a34f 100644 --- a/go/vt/vtgate/scatter_conn_test.go +++ b/go/vt/vtgate/scatter_conn_test.go @@ -679,8 +679,7 @@ func TestActionInfoWithTabletAlias(t *testing.T) { session.SetTargetTabletAlias(tabletAlias) // zone1-100, different from zone1-50 _, _, err := actionInfo(ctx, target, session, false, vtgatepb.TransactionMode_MULTI) - require.Error(t, err) - require.Contains(t, err.Error(), "cannot change tablet target mid-transaction") + require.ErrorContains(t, err, "cannot change tablet target mid-transaction") }) t.Run("no tablet alias - existing behavior", func(t *testing.T) { From c8edb0b8887c385023b313ead2acb0f4eae8a092 Mon Sep 17 00:00:00 2001 From: Nick Van Wiggeren Date: Fri, 16 Jan 2026 19:47:22 +0000 Subject: [PATCH 14/14] Update changelog: use replica example and add shard requirement note Signed-off-by: Nick Van Wiggeren --- changelog/24.0/24.0.0/summary.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/changelog/24.0/24.0.0/summary.md b/changelog/24.0/24.0.0/summary.md index 4c216823853..dbfbdcee695 100644 --- a/changelog/24.0/24.0.0/summary.md +++ b/changelog/24.0/24.0.0/summary.md @@ -43,14 +43,16 @@ VTGate now supports routing queries to a specific tablet by alias using an exten USE keyspace:shard@tablet_type|tablet_alias; ``` -For example, to target a specific primary tablet: +For example, to target a specific replica tablet: ```sql -USE commerce:-80@primary|zone1-0000000100; +USE commerce:-80@replica|zone1-0000000100; ``` Once set, all subsequent queries in the session route to the specified tablet until cleared with a standard `USE keyspace` or `USE keyspace@tablet_type` statement. This is useful for debugging, per-tablet monitoring, cache warming, and other operational tasks where targeting a specific tablet is required. +Note: A shard must be specified when using tablet targeting. Like shard targeting, this bypasses vindex-based routing, so use with care. + ## Minor Changes ### VTGate