Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

multitenant: tenant range endKey incorrect on tenant creation #97985

Closed
ecwall opened this issue Mar 3, 2023 · 1 comment · Fixed by #99246
Closed

multitenant: tenant range endKey incorrect on tenant creation #97985

ecwall opened this issue Mar 3, 2023 · 1 comment · Fixed by #99246
Assignees
@ecwall
Labels
A-multitenancy Related to multi-tenancy C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)

Comments

@ecwall
Copy link
Contributor

ecwall commented Mar 3, 2023 •
edited
Loading

When a tenant is created, its last range's endkey is Max instead of the startKey of the next tenant. This is automatically corrected by the reconciler later on (see https://github.com/cockroachdb/cockroach/pull/95100/files#diff-58b36925f72a8a1d1f9b496bdd458b2edd6846835ba6fb1717b1e7abc1d0e9e9R19).

As a temporary workaround TestTenant.WaitForTenantEndKeySplit was added as part of #95100 for tests (e.g. sql_keys logic test) that need the "correct" tenant range endKey (the next tenant's startKey instead of Max).

It would be better to handle this by making sure the tenant's last range's endKey is the tenant keyspace's endKey during tenant creation.

Jira issue: CRDB-25003
Epic: CRDB-23559
@ecwall ecwall added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. A-multitenancy Related to multi-tenancy T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) labels Mar 3, 2023
@ecwall ecwall self-assigned this Mar 3, 2023
@ajstorm ajstorm added the T-multitenant Issues owned by the multi-tenant virtual team label Mar 3, 2023
@exalate-issue-sync exalate-issue-sync bot removed the T-multitenant Issues owned by the multi-tenant virtual team label Mar 3, 2023
@arulajmani
Copy link
Collaborator

Maybe what we need here is an entry for the tenant's end key here:

cockroach/pkg/sql/catalog/bootstrap/metadata.go

Lines 241 to 243 in 5839fe0

} else {
splits = []roachpb.RKey{roachpb.RKey(ms.codec.TenantPrefix())}
}

@ecwall ecwall mentioned this issue Mar 22, 2023
Merged
craig bot pushed a commit that referenced this issue Mar 22, 2023
@michae2 @ecwall
Merge #99186 #99246
f4b266d 
99186: builtins: add hint for hide_sql_constants and redactable_sql_constants r=msirek a=michae2

Add a hint about using dollar-quotes to the info message for builtin functions `crdb_internal.hide_sql_constants` and
`crdb_internal.redactable_sql_constants`.

Fixes: #99132

Epic: None

Release note: None

99246: multitenant: add tenant end key split to MetadataSchema r=knz a=ecwall

Fixes #97985

Previously #95100 created tenant end key split points asynchronously which
allowed for a brief period after tenant creation where the tenant's keyspace
had an end key of /Max instead of the next tenant's start key.

This change creates that split point synchronously on tenant creation to avoid
race conditions.

Release note: None


Co-authored-by: Michael Erickson <[email protected]>
Co-authored-by: Evan Wall <[email protected]>
@craig craig bot closed this as completed in ac54eba Mar 22, 2023
blathers-crl bot pushed a commit that referenced this issue Mar 22, 2023
@ecwall
multitenant: add tenant end key split to MetadataSchema
50b0d42 
Fixes #97985

Previously #95100 created tenant end key split points asynchronously which
allowed for a brief period after tenant creation where the tenant's keyspace
had an end key of /Max instead of the next tenant's start key.

This change creates that split point synchronously on tenant creation to avoid
race conditions.

Release note: None
@blathers-crl blathers-crl bot mentioned this issue Mar 22, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ecwall ecwall

Labels
A-multitenancy Related to multi-tenancy C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

multitenant: add tenant end key split to MetadataSchema
3 participants
@arulajmani @ecwall @ajstorm