Skip to content

sql: don't query BootstrapVersionKey on tenant SQL startup#52595

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
nvb:nvanbenschoten/bootstrapVersionTenant
Aug 11, 2020
Merged

sql: don't query BootstrapVersionKey on tenant SQL startup#52595
craig[bot] merged 1 commit intocockroachdb:masterfrom
nvb:nvanbenschoten/bootstrapVersionTenant

Conversation

@nvb
Copy link
Copy Markdown
Contributor

@nvb nvb commented Aug 10, 2020

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.

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.
@nvb nvb requested a review from tbg August 10, 2020 20:03
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

@lunevalex lunevalex added the A-multitenancy Related to multi-tenancy label Aug 11, 2020
@nvb
Copy link
Copy Markdown
Contributor Author

nvb commented Aug 11, 2020

bors r+

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Aug 11, 2020

Build succeeded:

@craig craig bot merged commit e9abbac into cockroachdb:master Aug 11, 2020
@nvb nvb deleted the nvanbenschoten/bootstrapVersionTenant branch August 14, 2020 22:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-multitenancy Related to multi-tenancy

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants