-
Notifications
You must be signed in to change notification settings - Fork 721
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
tso: implement the global TSO generation algorithm #3033
Conversation
Signed-off-by: JmPotato <[email protected]>
4089c3a
to
e13db13
Compare
Signed-off-by: JmPotato <[email protected]>
Signed-off-by: JmPotato <[email protected]>
Signed-off-by: JmPotato <[email protected]>
/run-all-tests |
Signed-off-by: JmPotato <[email protected]>
Signed-off-by: JmPotato <[email protected]>
Signed-off-by: JmPotato <[email protected]>
Signed-off-by: JmPotato <[email protected]>
/run-all-tests |
/run-integration-common-test |
Signed-off-by: JmPotato <[email protected]>
Signed-off-by: JmPotato <[email protected]>
/run-all-tests |
Signed-off-by: JmPotato <[email protected]>
Signed-off-by: JmPotato <[email protected]>
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.
reset LGTM.
if currentGlobalTSO, err = gta.getCurrentTSO(); err != nil { | ||
return pdpb.Timestamp{}, err | ||
} | ||
if tsoutil.CompareTimestamp(¤tGlobalTSO, maxTSO) < 0 { |
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.
Cloud this update guarantee atomicity?
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.
It's not atomic here. But even if the currentGlobalTSO
changed during the comparison, the later SetTSO()
will guarantee the atomicity and make sure the TSO won't fallback.
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.
LGTM
/merge |
/run-all-tests |
Signed-off-by: JmPotato <[email protected]> (cherry picked from commit e0747f9)
What problem does this PR solve?
Part of #2759. Resolve #2984. This PR includes two parts:
The reason I put these two parts together is to have a CI test fully. If this brings you guys serious difficulty in reviewing, I will take these two parts separately though.
What is changed and how it works?
Check List
Tests
Side effects
Release note