sql: don't query BootstrapVersionKey on tenant SQL startup #52595
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
See cockroachdb#52094 (review). We don't currently track the bootstrap version of each secondary tenant. For this to be meaningful, we'd need to record the binary version of the SQL gateway that processed the crdb_internal.create_tenant function which created the tenant, as this is what dictates the MetadataSchema that was in effect when the secondary tenant was constructed. This binary version very well may differ from the cluster-wide bootstrap version at which the system tenant was bootstrapped. Since we don't record this version anywhere, we do the next-best thing and pass a lower-bound on the bootstrap version. We know that no tenants could have been created before the start of the v20.2 dev cycle, so we pass VersionStart20_2. bootstrapVersion is only used to avoid performing superfluous but necessarily idempotent SQL migrations, so at worst, we're doing more work than strictly necessary during the first time that the migrations are run. Now that we don't query BootstrapVersionKey, we don't need to have it in the allowlists in the tenantAuth policy for Batch and RangeLookup RPCs.
|
This change is |
tbg
approved these changes
Aug 11, 2020
|
bors r+ |
|
Build succeeded: |
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.
See #52094 (review).
We don't currently track the bootstrap version of each secondary tenant.
For this to be meaningful, we'd need to record the binary version of the
SQL gateway that processed the crdb_internal.create_tenant function which
created the tenant, as this is what dictates the MetadataSchema that was
in effect when the secondary tenant was constructed. This binary version
very well may differ from the cluster-wide bootstrap version at which the
system tenant was bootstrapped.
Since we don't record this version anywhere, we do the next-best thing
and pass a lower-bound on the bootstrap version. We know that no tenants
could have been created before the start of the v20.2 dev cycle, so we
pass VersionStart20_2. bootstrapVersion is only used to avoid performing
superfluous but necessarily idempotent SQL migrations, so at worst, we're
doing more work than strictly necessary during the first time that the
migrations are run.
Now that we don't query BootstrapVersionKey, we don't need to have it
in the allowlists in the tenantAuth policy for Batch and RangeLookup
RPCs.