-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Implement "Tablet Targeting" RFC #18809
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
0a82dec
de35ba8
b5b4284
355b031
e872db5
b2ae369
1ec01a9
5fe677e
f638a62
25a0e9c
5c0bc2d
a637fc9
8772732
7259df6
c8edb0b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -29,16 +29,44 @@ 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: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 | ||
| 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:] | ||
nickvanw marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| 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] | ||
| 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) | ||
| } | ||
| 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] | ||
| } | ||
| last = strings.LastIndexAny(targetString, "/:") | ||
|
|
@@ -51,29 +79,32 @@ 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 | ||
| 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 | ||
|
Comment on lines
+106
to
+109
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If there is a destination then it will not go through the planner? The query will be shard targeted.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's the expectation, which I think makes sense - this is basically shard targeting++, with the ability to shard target + target a specific tablet. |
||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nickvanw let's say I use this feature to write rows to a
PRIMARYthat doesn't align with the VSchema, am I creating any data correctness/etc risks we should detail here?I'm wondering if that would cause query failures or potentially-surprising results. But at the same time, I guess you can only use this feature with intention 🤔
And on a similar track, could vReplication get upset about rows in the wrong shard? One dreamed-up scenario I can think of is merging shards, and the same primary key exists in both, etc
None of what I'm thinking means changing the code here I think (it looks good so far), maybe just documentation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think the introduction of this feature changes this - you can already target specific shards and side-step any validation that Vitess would normally do against the queries you are executing (like ensuring inserts only are routed to the "correct" shard).