DBZ-8087 Get previous vgtid from vgtid key in offsets #204
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
jpechane
requested changes
Jul 26, 2024
| } | ||
| } | ||
|
|
||
| public Long getEpoch(String shard, String previousVgtidString, String vgtidString) { | ||
| if (previousVgtidString == null) { | ||
| if (previousVgtidString == null && shardToEpoch.get(shard) != null) { |
There was a problem hiding this comment.
I think this could be collapse into
if (previousVgtidString == null) {
if (shardToEpoch.get(shard) != null)
|
@jpechane this is ready for re-review, thanks! |
|
@twthorn Applied, thanks! |
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.
It is non-deterministic/race condition whether or not the
OFFSET_TRANSACTION_IDis present in the offsets (if we happen to store offsets during a transaction, it is added since it's not null, otherwise it is not added). For example if heartbeats are enabled, there is not transaction but we still commit offsets.Epoch needs to be based on the vgtid we store as for the VGTID_KEY since that is always present in offsets regardless of whether we are in a transaction or not. This fixes an edge case where we saw epoch being set back down to zero because we were getting null for our previous VGTID string. Also fail loudly if we ever see this case again (null previous VGTID but there is state for the epoch, this is invalid state and should throw exception).