-
Notifications
You must be signed in to change notification settings - Fork 37
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
Support Coordinator table schema migration for the group commit in ConsensusCommitAdmin.upgrade()
#2060
Support Coordinator table schema migration for the group commit in ConsensusCommitAdmin.upgrade()
#2060
Conversation
in `upgrade` schema-loader command
@@ -314,6 +325,7 @@ private void dropTablesIfExist() throws ExecutionException { | |||
transactionAdmin.dropTable(namespace1, TABLE_1, true); | |||
transactionAdmin.dropNamespace(namespace1, true); | |||
transactionAdmin.dropCoordinatorTables(true); | |||
waitForCreationIfNecessary(); |
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.
Added a few waits since I noticed some tests could fail with some underlying storages
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! Thank you!
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've left some minor suggestions. Other than that, LGTM. Thank you!🙇🏻♂️
core/src/main/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitAdmin.java
Outdated
Show resolved
Hide resolved
core/src/main/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitAdmin.java
Outdated
Show resolved
Hide resolved
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, thank you!
Co-authored-by: Josh Wong <[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.
LGTM! Thank you!
Description
We added the group commit feature for the Coordinator table, which adds a new column
tx_child_ids
tocoordinator.state
table. The column is supposed to be added by default in ScalarDB 4. But, there is no way for users to address the table schema migration at the moment. This PR adds the Coordinator table schema migration inConsensusCommitAdmin.upgrade()
so that users can execute the migration via Schema Loader.Related issues and/or PRs
For ScalarDB 3.13 users, https://github.com/scalar-labs/docs-internal-scalardb/pull/130
Changes made
Admin.addNewColumnToTable(tx_child_ids)
if the column doesn't exist in the current Coordinator table metadata (without checking the underlying table)Checklist
Additional notes (optional)
What if any PREPARED state records created by ScalarDB 3 remain in tables when the
upgrade
command is executed?The transaction ID of the PREPARE record doesn't have any prefix (e.g.,
a97Zb8Mxg...
). The current ScalarDB 4 versionCoordinator
determines that the transaction ID is created with the group commit feature disabled, and treats it as a normal transaction.Release notes
Support Coordinator table schema migration for the group commit feature in Schema Loader's
upgrade
command.