release-21.1: block concurrent ADD/DROP REGION and REGIONAL BY ROW changes #64255
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
There was an error in the `RunBeforeMultiRegionUpdates` knob such that if it were set, we would short circuit. The intention, instead, was to only short circuit in the error case. Release note: None
This is a fix to a problem facing validation - namely - the type schema changer modifies the enum and the table separately. If we change the enum first, the table no longer validates partitions correctly. We can't change the table partition first as the enum members are no longer public. To alleviate this, we pre-fetch the tables before the enum gets changed and re-hydrate the enum descriptor after the enum gets changed to add the new partitioning. Release note: None
This commit adds validation that partitions for a REGIONAL BY ROW table matches regions in the type descriptor. Release note: None
Multi-region tests check to ensure partitioning is sane from a few different tests. This pulls out that logic into its own function. Release note: None
This patch adds TestAlterRegionalByRowEnclosingRegionAddDrop, with the following sketch: - Client 1 performs an `ALTER TABLE ... SET LOCALITY REGIONAL BY ROW TABLE` - Let the user txn commit. - Block in the (table) schema changer. - Client 2 performs an ALTER ADD / DROP REGION. Let the operation complete (succeed or fail, depending on the particular setup). - Resume the ALTER operation in the schema changer. Currently, this ALTER operation is bound to fail because of Once the issue is addressed, this test should be updated to assert the correct partitioning values (in all cases). Release note: None
Release note (bug fix): Previously, if a DROP INDEX failed during a REGIONAL BY ROW transition, the index may be re-inserted back into the REGIONAL BY ROW table but would be invalid if it was sharded or partitioned. This is now rectified.
This is helpful for error messages which allows the schema name to be output in the error message. Release note: None
Release note (sql change): Disallow ADD/DROP REGION whilst a REGIONAL BY ROW table has index changes underway or a table is transitioning to or from REGIONAL BY ROW.
Release note (sql change): Prevent index modification on REGIONAL BY ROW tables and locality to/from REGIONAL BY ROW changes during a ADD/DROP REGION.
|
This change is |
ajstorm
reviewed
Apr 27, 2021
There was a problem hiding this comment.
@arulajmani can you quickly confirm that this is the necessary and sufficient list of commits to backport for this issue?
Reviewable status:
complete! 1 of 0 LGTMs obtained (waiting on @arulajmani)
arulajmani
approved these changes
Apr 27, 2021
There was a problem hiding this comment.
I think technically you might not need the second commit from #64017, as that test gets removed in a subsequent commit, but probably not worth the cherry-pick/conflict hassle.
Reviewable status:
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.
Backport:
Please see individual PRs for details.
/cc @cockroachdb/release