VStream: Allow for automatic resume after Reshard across VStreams #15395
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
Tests
Documentation
New flags
If a workflow is added or modified:
Backward compatibility
|
vitess-bot
bot
added
NeedsBackportReason
mattlord
added
Type: Enhancement
mattlord
force-pushed
the
vtgate_vstream_reshard_resume
branch
3 times, most recently
from
1d284dc to
e5d3048
Compare
mattlord
commented
Mar 2, 2024
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #15395 +/- ##
==========================================
+ Coverage 65.65% 65.69% +0.03%
==========================================
Files 1563 1564 +1
Lines 194423 194623 +200
==========================================
+ Hits 127658 127863 +205
+ Misses 66765 66760 -5 ☔ View full report in Codecov by Sentry. |
mattlord
removed
the
NeedsDescriptionUpdate
mattlord
changed the title
mattlord
force-pushed
the
vtgate_vstream_reshard_resume
branch
7 times, most recently
from
b5b7974 to
dc80682
Compare
Signed-off-by: Matt Lord <[email protected]>
mattlord
force-pushed
the
vtgate_vstream_reshard_resume
branch
from
dc80682 to
d967187
Compare
Signed-off-by: Matt Lord <[email protected]>
mattlord
force-pushed
the
vtgate_vstream_reshard_resume
branch
2 times, most recently
from
a501e62 to
623cfdb
Compare
Signed-off-by: Matt Lord <[email protected]>
mattlord
force-pushed
the
vtgate_vstream_reshard_resume
branch
from
623cfdb to
77894d7
Compare
Signed-off-by: Matt Lord <[email protected]>
Signed-off-by: Matt Lord <[email protected]>
Signed-off-by: Matt Lord <[email protected]>
mattlord
changed the title
Signed-off-by: Matt Lord <[email protected]>
mattlord
force-pushed
the
vtgate_vstream_reshard_resume
branch
from
775ee87 to
3ed17b0
Compare
Signed-off-by: Matt Lord <[email protected]>
Signed-off-by: Matt Lord <[email protected]>
mattlord
requested review from
GuptaManan100,
frouioui and
harshit-gangal
as code owners
…d_resume Signed-off-by: Matt Lord <[email protected]>
go/vt/vtgate/vstream_manager.go
Outdated
Comment on lines
974
to
988
| // Now that we know there MAY have been an applicable reshard, let's make a | ||
| // definitive determination by looking at the shard keyranges. | ||
| for _, i := range shards { | ||
| for _, j := range shards { | ||
| if i.ShardName() == j.ShardName() && key.KeyRangeEqual(i.GetKeyRange(), j.GetKeyRange()) { | ||
| // It's the same shard so skip it. | ||
| continue | ||
| } | ||
| if key.KeyRangeIntersect(i.GetKeyRange(), j.GetKeyRange()) { | ||
| // We have different shards with overlapping keyranges so we know | ||
| // that a reshard has happened. | ||
| return true, nil | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Overall, LGTM. If it ever becomes a concern, then we can improve the performance of this code by sorting the two lists of shards and then reading from both simultaneously once.
There was a problem hiding this comment.
It's a single map of shards. It's all of the shards in the keyspace, so e.g. if a keyspace is being sharded for the first time the list could be:
-
-80
80-
In that case we would take the first one, then compare it with -80 and see that they have overlapping key ranges.
GuptaManan100
approved these changes
Mar 18, 2024
Signed-off-by: Matt Lord <[email protected]>
5 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
When we can see that a keyspace has been resharded — meaning that the keyspace has serving shards that differ from the ones provided in the current
VStreamRPC call, which are from the previousVStreamclient connection — then we include non-serving tablets in our tablet picker so that we can select the tablets from the old / now-non-serving shards in the keyspace in order to ensure that we stream all remaining GTIDs needed from those shards between what we had last streamed and when theReshardcutover (SwitchTrafficorReverseTraffic) took place, along with theReshardjournal event which allows us to then automatically and seamlessly start replicating from the beginning of time on the new shards.The remaining limitation, which there is no way around, is that the old shards cannot be deleted in between the VStreams — as we need to connect to those to ensure we stream all of the GTIDs created there before transitioning to the new shards.
Click here for a manual test in this branch:
Example final output:
Related Issue(s)
Checklist