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.

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

sql,ccl: new tenant info field ServiceMode #95581

Closed
wants to merge 1 commit into from

Conversation

knz
Copy link
Contributor

@knz knz commented Jan 20, 2023

Previous PRs:

Summary of changes:

  • the new TenantInfo.ServiceMode field indicates how to run servers.
  • new syntax: ALTER TENANT ... START SERVICE EXTERNAL/SHARED,
    ALTER TENANT ... STOP SERVICE.
  • tenants created via create_tenant(<id>)
    (via CC serverless control plane) start in service mode EXTERNAL.
  • other tenants start in service mode NONE.
  • need ALTER TENANT STOP SERVICE before dropping a tenant.
    • except in the case of crdb_internal.destroy_tenant
      for compat with CC serverless control plane.

This commit does not contain the logic to start/stop
servers yet.

Release note: None
Epic: CRDB-21836

@knz knz added the A-multitenancy Related to multi-tenancy label Jan 20, 2023
@knz knz requested review from dt and stevendanna January 20, 2023 15:15
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@knz knz force-pushed the 20230120-tenant-service-mode branch from b6a399c to a36956c Compare January 20, 2023 15:15
@knz knz force-pushed the 20230120-rename-tenant-state branch from dff40a5 to cd6f262 Compare January 20, 2023 15:28
@knz knz force-pushed the 20230120-tenant-service-mode branch from a36956c to 85781d4 Compare January 20, 2023 15:30
@knz knz force-pushed the 20230120-rename-tenant-state branch from cd6f262 to adb3a18 Compare January 20, 2023 15:37
@knz knz force-pushed the 20230120-tenant-service-mode branch 2 times, most recently from dd24917 to f21dd4d Compare January 20, 2023 16:03
@dt
Copy link
Member

dt commented Jan 20, 2023

Part of me wishes we stored things like this in their own columns of the tenant table instead of one giant opaque proto since we are a sql database after all e.g. for easier "WHERE service_mode = x` or whatever, but I suppose pb_to_json works too.

@knz
Copy link
Contributor Author

knz commented Jan 20, 2023

I can make it a virtual computed column like i did for name. done

@knz knz force-pushed the 20230120-rename-tenant-state branch from adb3a18 to a9a6d14 Compare January 21, 2023 18:51
@knz knz force-pushed the 20230120-tenant-service-mode branch 3 times, most recently from 4f7a75c to 9c36bc3 Compare January 21, 2023 20:02
@knz knz force-pushed the 20230120-rename-tenant-state branch from a9a6d14 to 6874b33 Compare January 21, 2023 20:08
@knz knz force-pushed the 20230120-tenant-service-mode branch from 9c36bc3 to f7ff204 Compare January 21, 2023 20:59
@knz
Copy link
Contributor Author

knz commented Jan 21, 2023

TODO for self: only allow service mode SHARED if the name is non-empty. done

@knz knz force-pushed the 20230120-rename-tenant-state branch from 6874b33 to 4d88e2d Compare January 21, 2023 22:03
@knz knz force-pushed the 20230120-tenant-service-mode branch 2 times, most recently from da7cd8c to b2b237a Compare January 23, 2023 15:04
@knz knz marked this pull request as ready for review January 23, 2023 15:04
@knz knz requested a review from a team January 23, 2023 15:04
@knz knz requested review from a team as code owners January 23, 2023 15:04
@knz knz requested a review from a team as a code owner January 23, 2023 15:04
@knz knz requested a review from a team January 23, 2023 15:04
@knz knz requested a review from a team as a code owner January 23, 2023 15:04
@knz knz requested a review from yuzefovich January 23, 2023 15:04
@knz knz changed the base branch from 20230120-rename-tenant-state to 20230120-tenant-state-base January 23, 2023 15:09
Summary of changes:
- the new TenantInfo.ServiceMode field indicates how to run servers.
- new syntax: `ALTER TENANT ... START SERVICE EXTERNAL/SHARED`,
  `ALTER TENANT ... STOP SERVICE`.
- tenants created via `create_tenant(<id>)`
  (via CC serverless control plane) start in service mode EXTERNAL.
- other tenants start in service mode NONE.
- need ALTER TENANT STOP SERVICE before dropping a tenant.
  - except in the case of `crdb_internal.destroy_tenant`
    for compat with CC serverless control plane.

This commit does not contain the logic to start/stop
servers yet.

Release note: None
@knz
Copy link
Contributor Author

knz commented Jan 23, 2023

Discussed with @ajwerner @dt : I will modify this PR to reify the data state, service mode and name into proper SQL columns.

@knz
Copy link
Contributor Author

knz commented Jan 23, 2023

replaced by #95691.

@knz knz closed this Jan 23, 2023
craig bot pushed a commit that referenced this pull request Jan 26, 2023
95631: schemachanger: generalized statement-time execution r=postamar a=postamar

This change makes it possible to plan and execute the following change
correctly:

    BEGIN;
    DROP TABLE foo;
    CREATE UNIQUE INDEX idx ON bar (x);
    COMMIT;

Inside the transaction, the table is seen as dropped right after the
DROP TABLE statement executes, but the table is not seen as dropped by
other transactions until the new, unrelated index has been validated, at
which point the schema change can no longer fail.

Fixes #88294.

Significant changes:
- descs.Collection has a new ResetUncommitted method.
- scstage.BuildStages is rewritten with unrolled loops and a new scheme:
  statement phase has revertible op-edges, pre-commit has two stages,
  one which resets the uncommitted state by scheduling the new
  scop.UndoAllInTxnImmediateMutationOpSideEffects op, which triggers a call
  to ResetUncommitted.
- This happens in scexec.executeMutationOps, which is now split into two
  pieces, the first one which executes ops whose side-effects can be undone
  by ResetUncommitted, and the other which handles those which can’t.
- These ops implement scop.ImmediateMutationOp and scop.DeferredMutationOp
  respectively.
- These have their own visitors with their own state and their own
  dependencies.

This all means that we can handle DROPs properly now: the TXN_DROPPED element
state is removed as are all the synthetic descriptor manipulations (outside of
index and constraint validation, that is). Furthermore, the
scgraph.PreviousTransactionPrecedence edge kind no longer serves any purpose
and has been replaced by PreviousStagePrecedence everywhere.

There’s a bunch of other more-or-less related changes, mostly as a consequence
to the above:
- scbuild.Build must produce a tighter target set: no-op targets are
  elided.
- nstree.MutableCatalog has new methods for deleting comments and zone configs
  which are useful.
- sctestdeps.TestState has better storage layers modelling
  (in-memory > stored > committed)
- Added missing “relation dropped before column|index no longer public” rules
  which fix DROP COLUMN CASCADE when the column is referenced in a view.
- scgraph.Graph has a new GetOpEdgeTo method which is useful.
- added debug info to assertion error messages in scstage.BuildStages to make
  debugging easier during development.
- EXPLAIN (DDL) output has a nicer root node label which puts the last
  statement first.

Release note: None

95636: sql: increase the online help for SHOW RANGES r=ecwall a=knz

The output of `\h SHOW RANGES` wasn't mentioning the KEYS option. Now it does. Also this adds some additional details about the various options.

Release note: None
Epic: None

95691: sql: improve tenant records r=stevendanna,dt,ajwerner a=knz

supersedes #95574, #95581, #95655
Epic: CRDB-21836

**TLDR** This commit contains the following changes:
- rename "state" to "data_state", "active" to "ready"
- stored, non-virtual columns for "name", "data_state", "service_mode"
- deprecate the column "active" since it mirrors "data_state'
- move `descpb.TenantInfo` to new package `mtinfopb`.
- new statements `ALTER TENANT ... START SERVICE EXTERNAL/SHARED`,
  `STOP SERVICE` to change the service mode.

Details follow.

**rename TenantInfo.State to DataState, "ACTIVE" to "READY"**

We've discovered that we'd like to separate the readiness of the data
from the activation of the service. To emphasize this, this commit
renames the field "State" to "DataState".

Additionally, the state "ACTIVE" was confusing as it suggests that
something is running, whereas it merely marks the tenant data as ready
for use. So this commit also renames that state accordingly.

**new tenant info field ServiceMode**

Summary of changes:
- the new TenantInfo.ServiceMode field indicates how to run servers.
- new syntax: `ALTER TENANT ... START SERVICE EXTERNAL/SHARED`,
  `ALTER TENANT ... STOP SERVICE`.
- tenants created via `create_tenant(<id>)`
  (via CC serverless control plane) start in service mode EXTERNAL.
- other tenants start in service mode NONE.
- need ALTER TENANT STOP SERVICE before dropping a tenant.
  - except in the case of `crdb_internal.destroy_tenant`
    for compat with CC serverless control plane.

**make the output columns of SHOW TENANT lowercase**

All the SHOW statements report status-like data in lowercase.
SHOW TENANT(s) should not be different.

**use actual SQL columns for the TenantInfo fields**

Release note: None

Co-authored-by: Marius Posta <[email protected]>
Co-authored-by: Raphael 'kena' Poss <[email protected]>
@knz knz deleted the 20230120-tenant-service-mode branch January 29, 2023 19:07
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.

3 participants