ccl: regionless restores with rbr table(s) fail fast, add version gate #111443
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
|
This change is |
msbutler
approved these changes
Sep 28, 2023
annrpom
force-pushed
the
regionless-restore-option
branch
3 times, most recently
from
3a4b509 to
64cb124
Compare
annrpom
force-pushed
the
regionless-restore-option
branch
from
64cb124 to
4c83ea6
Compare
annrpom
force-pushed
the
regionless-restore-option
branch
3 times, most recently
from
030c63a to
42d583b
Compare
annrpom
changed the title
annrpom
changed the title
annrpom
changed the title
msbutler
approved these changes
Oct 2, 2023
annrpom
force-pushed
the
regionless-restore-option
branch
from
3f017fa to
ad52b7d
Compare
Previously, backing up an MR cluster/db/table that had a regional by row table did not work out-of-the-box (users could not write to said table without performing some operations to ensure hidden column added by regional by row enablement, `crdb_region`, and zone config behaved according to the target RESTORE cluster. This was inadequate because we allowed users to perform a RESTORE where some tables were not "available" right away. This patch addresses this by making sure we "fail fast" if users do decide to attempt a RESTORE where the BACKUP object has a regional by row table. Also: tests. A small amount of tests were fixed to separate the behavior of restoring a database vs a table (just dropping a table after we performed a db restore to test table restore is not a properly isolated test of behavior). Epic: none Informs: cockroachdb#111348 Part of: https://cockroachlabs.atlassian.net/browse/CRDB-29129 Release note (sql change): Users are not able to perform a `remove_regions` RESTORE if the object being restored contains a regional by row table.
annrpom
force-pushed
the
regionless-restore-option
branch
from
ad52b7d to
526f500
Compare
annrpom
requested review from
herkolategan and
renatolabs
and removed request for
a team
annrpom
force-pushed
the
regionless-restore-option
branch
from
526f500 to
059f55f
Compare
fqazi
approved these changes
Oct 4, 2023
There was a problem hiding this comment.
The main part of stripping localities should be safe to backport since none of the fields were are stripping are new here.
Reviewed 4 of 5 files at r1, 3 of 3 files at r2, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @annrpom, @herkolategan, @rafiss, and @renatolabs)
-- commits line 23 at r2:
This feature hasn't been released yet, so a release note isn't warranted.
-- commits line 32 at r2:
23.2
-- commits line 38 at r2:
Again similar to above
annrpom
commented
Oct 4, 2023
There was a problem hiding this comment.
Reviewable status:
Previously, fqazi (Faizan Qazi) wrote…
This feature hasn't been released yet, so a release note isn't warranted.
This one is more for docs - a merged PR specified the behavior of this restore option if the object being restored does have a rbr table & this is just here to document that such behavior has changed
Previously, fqazi (Faizan Qazi) wrote…
23.2
ah rip - thank you
Previously, fqazi (Faizan Qazi) wrote…
Again similar to above
done!
This patch adds a >= 23.2 version gate to the `remove_regions` RESTORE option, in addition to a mixed-version test to ensure that RESTORE fails fast if it is on a cluster version < 23.2. Epic: none Fixes: cockroachdb#111348 Part of: https://cockroachlabs.atlassian.net/browse/CRDB-29129 Release note: None
annrpom
force-pushed
the
regionless-restore-option
branch
from
059f55f to
831d60b
Compare
|
TFTR! ('-')7 bors r=msbutler |
|
Build succeeded: |
This was referenced Oct 5, 2023
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.
ccl: regionless restores with regional by row table(s) fail fast
Previously, backing up an MR cluster/db/table that had a regional by row table did not work out-of-the-box (users could not write to said table without performing some operations to ensure hidden column added by regional by row enablement,
crdb_region, and zone config behaved according to the target RESTORE cluster. This was inadequate because we allowed users to perform a RESTORE where some tables were not "available" right away. This patch addresses this by making sure we "fail fast" if users do decide to attempt a RESTORE where the BACKUP object has a regional by row table.Also: tests. A small amount of tests were fixed to separate the behavior of restoring a database vs a table (just dropping a table after we performed a db restore to test table restore is not a properly isolated test of behavior). Mixed-version tests were added as well.
Epic: CRDB-27601
Informs: #111348
Release note (sql change): Users are not able to perform a
remove_regionsRESTORE if the object being restored contains a regional by row table.ccl: add 23.2 version gate to regionless restore
This patch adds a >= 23.2 version gate to the
remove_regionsRESTORE option, in addition to a mixed-version test to ensure
that RESTORE fails fast if it is on a cluster version < 23.2.
Epic: CRDB-27601
Fixes: #111348
Release note: None