-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
schemachanger: improve state management #96117
schemachanger: improve state management #96117
Conversation
This commit came about because working on #95631 made me uneasy about whether it's legal for the builder to overwrite or elide existing targets. Also I wasn't quite convinced that the |
a594919
to
38fdba3
Compare
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
38fdba3
to
2ab2ab8
Compare
Thanks for the review! bors r+ |
This commit endows the scpb.CurrentState with the set of element statuses which are in force at the beginning of the statement transaction. These are then used when planning the pre-commit reset stage. This commit also rewrites the target-setting logic in the scbuild package in a more explicit manner with extra commentary. This commit does not change any functional behavior in production. Informs cockroachdb#88294. Release note: None
2ab2ab8
to
1266859
Compare
bors r+ |
Build failed (retrying...): |
Build succeeded: |
This commit endows the scpb.CurrentState with the set of element statuses which are in force at the beginning of the statement transaction. These are then used when planning the pre-commit reset stage.
This commit also rewrites the target-setting logic in the scbuild package in a more explicit manner with extra commentary.
This commit does not change any functional behavior in production.
Informs #88294.
Release note: None