sql: Remove cluster version input to GetInitialValues #48504
Merged
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
|
This change is |
|
❌ The GitHub CI (Cockroach) build has failed on 502d837d. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan. |
|
❌ The GitHub CI (Cockroach) build has failed on 502d837d. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan. |
Addresses a TODO and helps clear the way for cockroachdb#47904.
nvanbenschoten
force-pushed
the
nvanbenschoten/bootstrapNamespace
branch
from
502d837
to
c2dabe0
Compare
|
Friendly ping. |
nvanbenschoten
added a commit
to nvanbenschoten/cockroach
that referenced
this pull request
May 13, 2020
Most of cockroachdb#47904. First commit from cockroachdb#48504. Second and third commit from cockroachdb#48773. This commit implements the initial structure for tenant creation and deletion. It does so by introducing a new system table to track tenants in a multi-tenant cluster and two new builtin functions to manipulate this table and the overall multi-tenant state. The change starts by introducing a new `system.tenants` table with the following schema: ``` CREATE TABLE system.tenants ( id INT8 NOT NULL PRIMARY KEY, active BOOL NOT NULL DEFAULT true, info BYTES ); ``` The `id` column is self-explanatory. The `active` column is used to coordinate tenant destruction in an asynchronous job. The `info` column is an opaque byte slice to allow users to associate arbitrary information with specific tenants. I don't know exactly how this third field will be used (mapping tenants back to CockroachCloud user IDs?), but it seems like a good idea to add some flexibility since we do intend to eventually expose this externally. The table is given an ID of 8, which means that it falls within the "system config span". I believe this is ok, because the entire concept of the "system config span" should be going away with cockroachdb#47150. The table is also only exposed to the system-tenant and is never created for secondary tenants. The change then introduces two new builtin functions: `crdb_internal.create_tenant` and `crdb_internal.destroy_tenant`. These do what you would expect - creating and destroying tenant keyspaces, along with updating metadata in system.tenants.
|
@asubiotto can you take this one? Solon is off these days. |
asubiotto
approved these changes
May 13, 2020
There was a problem hiding this comment.
Reviewed 10 of 10 files at r1.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @solongordon)
|
TFTR! I didn't realize Solon was on paternity leave - I missed that on the calendar. I'll merge this now but am happy to revisit if someone else takes a look. bors r+ |
Build succeeded |
nvanbenschoten
added a commit
to nvanbenschoten/cockroach
that referenced
this pull request
May 14, 2020
Most of cockroachdb#47904. First commit from cockroachdb#48504. Second and third commit from cockroachdb#48773. This commit implements the initial structure for tenant creation and deletion. It does so by introducing a new system table to track tenants in a multi-tenant cluster and two new builtin functions to manipulate this table and the overall multi-tenant state. The change starts by introducing a new `system.tenants` table with the following schema: ``` CREATE TABLE system.tenants ( id INT8 NOT NULL PRIMARY KEY, active BOOL NOT NULL DEFAULT true, info BYTES ); ``` The `id` column is self-explanatory. The `active` column is used to coordinate tenant destruction in an asynchronous job. The `info` column is an opaque byte slice to allow users to associate arbitrary information with specific tenants. I don't know exactly how this third field will be used (mapping tenants back to CockroachCloud user IDs?), but it seems like a good idea to add some flexibility since we do intend to eventually expose this externally. The table is given an ID of 8, which means that it falls within the "system config span". I believe this is ok, because the entire concept of the "system config span" should be going away with cockroachdb#47150. The table is also only exposed to the system-tenant and is never created for secondary tenants. The change then introduces two new builtin functions: `crdb_internal.create_tenant` and `crdb_internal.destroy_tenant`. These do what you would expect - creating and destroying tenant keyspaces, along with updating metadata in system.tenants.
jordanlewis
reviewed
May 15, 2020
| roachpb.KeyValue{ | ||
| Key: NewPublicSchemaKey(desc.GetID()).Key(ms.codec), | ||
| Value: publicSchemaValue, | ||
| }) |
There was a problem hiding this comment.
I thought this could be problematic given #49092, but I'm pretty sure it's not. This change simply edits what new clusters have as initial values. New clusters don't need anything for the old namespace table.
Does that square with your understanding @nvanbenschoten?
There was a problem hiding this comment.
Yes, that's correct. This is what new clusters use for their initial values.
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.
Addresses a TODO and helps clear the way for #47904.
This does not remove the rest of the references to
VersionNamespaceTableWithSchemas.