release-20.2: sql/catalog, backupccl: support offline descriptors, add all new restored descriptors in offline state #54296
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
Release justification: Bug fixes to new functionality Release note (bug fix): Fixes a bug introduced in a 20.2 alpha release where incorrect caching could incur extra writes to the store during RESTORE on user-defined schemas.
The `Version` and `ModificationTime` fields are now reset to 1 and the empty timestamp, respectively, on new descriptors created in RESTORE, to make them behave like other newly created descriptors. Release justification: Low risk, high benefit changes to existing functionality Release note: None
This change adds methods for the offline state to the descriptor interfaces and makes the state-related API more uniform. Release justification: Low risk, high benefit changes to existing functionality Release note: None
This change updates the name resolution and descriptor lookup implementations to respect the state of dropped and offline descriptors: All lookups now read the `IncludeOffline` and `IncludeDropped` flags in `tree.CommonLookupFlags` and filter results accordingly. Tests in the context of RESTORE are forthcoming in the following commit. Release justification: Bug fixes and low-risk updates to new functionality Release note: None
This change updates the RESTORE job to add all new descriptors prior to restoring data in the OFFLINE state, and update their state to PUBLIC after data has been restored. This makes all descriptors behave like tables. Release justification: Low risk, high benefit changes to existing functionality (for database descriptors; other changes apply only to new functionality) Release note (sql change): Databases being restored will now be in the offline state, invisible to users, until the data has been restored. This is the same as the existing behavior for restored tables. (This change is also applied to enums and user-defined schemas being restored, which is a change relative to only the 20.2 alpha releases.)
|
This change is |
ajwerner
approved these changes
Sep 12, 2020
There was a problem hiding this comment.
Reviewed 1 of 1 files at r1, 4 of 4 files at r2, 15 of 15 files at r3, 7 of 7 files at r4, 4 of 4 files at r5.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @pbardea)
ajwerner
added a commit
to ajwerner/cockroach
that referenced
this pull request
Sep 14, 2020
In cockroachdb#54296 we began resetting the version and modification time for all newly added descriptors. This is fine for the descriptors which did not exist or have those fields in 20.1 (schema, type, db). For tables however, this is not okay because code in release 20.1 is missing a check added in the 20.2 cycle to allow a missing modification time for version 1. This breaks restore and import in the mixed version state. This PR gates the resetting of modification time on the cluster version. I have verified that this fixes the jobs/mixed-version roachtest which previously failed deterministically due to this bug. No release note because we haven't shipped this bug. Fixes cockroachdb#54319. Release note: None
ajwerner
added a commit
to ajwerner/cockroach
that referenced
this pull request
Sep 15, 2020
In cockroachdb#54296 we began resetting the version and modification time for all newly added descriptors. This is fine for the descriptors which did not exist or have those fields in 20.1 (schema, type, db). For tables however, this is not okay because code in release 20.1 is missing a check added in the 20.2 cycle to allow a missing modification time for version 1. This breaks restore and import in the mixed version state. This PR gates the resetting of modification time on the cluster version. I have verified that this fixes the jobs/mixed-version roachtest which previously failed deterministically due to this bug. No release note because we haven't shipped this bug. Fixes cockroachdb#54319. Release note: None
ajwerner
added a commit
to ajwerner/cockroach
that referenced
this pull request
Sep 15, 2020
In cockroachdb#54296 we began resetting the version and modification time for all newly added descriptors. This is fine for the descriptors which did not exist or have those fields in 20.1 (schema, type, db). For tables however, this is not okay because code in release 20.1 is missing a check added in the 20.2 cycle to allow a missing modification time for version 1. This breaks restore and import in the mixed version state. This PR gates the resetting of modification time on the cluster version. I have verified that this fixes the jobs/mixed-version roachtest which previously failed deterministically due to this bug. No release note because we haven't shipped this bug. Fixes cockroachdb#54319. Release note: None
craig bot
pushed a commit
that referenced
this pull request
Sep 15, 2020
54363: backupccl,importccl: only reset ModificationTime on tables after upgrade r=ajwerner a=ajwerner In #54296 we began resetting the version and modification time for all newly added descriptors. This is fine for the descriptors which did not exist or have those fields in 20.1 (schema, type, db). For tables however, this is not okay because code in release 20.1 is missing a check added in the 20.2 cycle to allow a missing modification time for version 1. This breaks restore and import in the mixed version state. This PR gates the resetting of modification time on the cluster version. I have verified that this fixes the jobs/mixed-version roachtest which previously failed deterministically due to this bug. No release note because we haven't shipped this bug. Fixes #54319. Release note: None Co-authored-by: Andrew Werner <[email protected]>
ajwerner
added a commit
to ajwerner/cockroach
that referenced
this pull request
Sep 15, 2020
In cockroachdb#54296 we began resetting the version and modification time for all newly added descriptors. This is fine for the descriptors which did not exist or have those fields in 20.1 (schema, type, db). For tables however, this is not okay because code in release 20.1 is missing a check added in the 20.2 cycle to allow a missing modification time for version 1. This breaks restore and import in the mixed version state. This PR gates the resetting of modification time on the cluster version. I have verified that this fixes the jobs/mixed-version roachtest which previously failed deterministically due to this bug. No release note because we haven't shipped this bug. Fixes cockroachdb#54319. Release note: None
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 5/5 commits from #53539.
/cc @cockroachdb/release
Closes #53375.
backupccl: fix bug in writing database schema map
Release justification: Bug fixes to new functionality
Release note (bug fix): Fixes a bug introduced in a 20.2 alpha release
where incorrect caching could incur extra writes to the store during
RESTORE on user-defined schemas.
backupccl: reset Version and ModificationTime on new descriptors
The
VersionandModificationTimefields are now reset to 1 and theempty timestamp, respectively, on new descriptors created in RESTORE, to
make them behave like other newly created descriptors.
Release justification: Low risk, high benefit changes to existing
functionality
Release note: None
sql/catalog: add descriptor API support for offline state
This change adds methods for the offline state to the descriptor
interfaces and makes the state-related API more uniform.
Release justification: Low risk, high benefit changes to existing
functionality
Release note: None
sql/catalog: handle dropped and offline descriptors in lookups
This change updates the name resolution and descriptor lookup
implementations to respect the state of dropped and offline descriptors:
All lookups now read the
IncludeOfflineandIncludeDroppedflags intree.CommonLookupFlagsand filter results accordingly.Tests in the context of RESTORE are forthcoming in the following commit.
Release justification: Bug fixes and low-risk updates to new
functionality
Release note: None
backupccl: add all new restored descriptors in offline state
This change updates the RESTORE job to add all new descriptors prior to
restoring data in the OFFLINE state, and update their state to PUBLIC
after data has been restored. This makes all descriptors behave like
tables.
Release justification: Low risk, high benefit changes to existing
functionality (for database descriptors; other changes apply only to new
functionality)
Release note (sql change): Databases being restored will now be in the
offline state, invisible to users, until the data has been restored.
This is the same as the existing behavior for restored tables. (This
change is also applied to enums and user-defined schemas being restored,
which is a change relative to only the 20.2 alpha releases.)