sql/catalog/lease: permit gaps in descriptor history#72714
Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom Nov 15, 2021
Merged
sql/catalog/lease: permit gaps in descriptor history#72714craig[bot] merged 1 commit intocockroachdb:masterfrom
craig[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
In cockroachdb#71239, we added a new mechanism to look up historical descriptors. I erroneously informed @jameswsj10 that we would never have gaps in the descriptor history, and, thus, when looking up historical descriptors, we could always use the earliest descriptor's modification time as the bounds for the relevant query. This turns out to not be true. Consider the case where version 3 is a historical version and then version 4 pops up and gets leased. Version 3 will get removed if it is not referenced. In the meantime, version 3 existed when we went to go find version 2. At that point, we'll inject version 2 and have version 4 leased. We need to make sure we can handle the case where we need to go fetch version 3. In the meantime, this change also removes some logic added to support the eventual resurrection of cockroachdb#59606 whereby we'll use the export request to fetch descriptor history to power historical queries even in the face of descriptors having been deleted. Fixes cockroachdb#72706. Release note: None
Member
|
This change is |
jameswsj10
approved these changes
Nov 15, 2021
jameswsj10
left a comment
There was a problem hiding this comment.
Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @postamar)
Contributor
Author
|
TFTR! bors r+ |
Contributor
|
Build succeeded: |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
In #71239, we added a new mechanism to look up historical descriptors. I
erroneously informed @jameswsj10 that we would never have gaps in the
descriptor history, and, thus, when looking up historical descriptors, we
could always use the earliest descriptor's modification time as the bounds
for the relevant query.
This turns out to not be true. Consider the case where version 3 is a
historical version and then version 4 pops up and gets leased. Version 3 will
get removed if it is not referenced. In the meantime, version 3 existed when we
went to go find version 2. At that point, we'll inject version 2 and have
version 4 leased. We need to make sure we can handle the case where we need to
go fetch version 3.
In the meantime, this change also removes some logic added to support the
eventual resurrection of #59606 whereby we'll use the export request to fetch
descriptor history to power historical queries even in the face of descriptors
having been deleted.
Fixes #72706.
Release note: None