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

sql: provide alternative syntax for the UX surface of cluster virtualization operations #106068

Closed
11 of 15 tasks
knz opened this issue Jul 3, 2023 · 1 comment
Closed
11 of 15 tasks

sql: provide alternative syntax for the UX surface of cluster virtualization operations #106068

knz opened this issue Jul 3, 2023 · 1 comment
Assignees
@knz
Labels
A-multitenancy Related to multi-tenancy C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-multitenant Issues owned by the multi-tenant virtual team

Comments

@knz
Copy link
Contributor

knz commented Jul 3, 2023 •
edited
Loading

We want to introduce the term "virtual cluster" or "cluster virtualization" in those features that are exposed to end-users.
This part of the change would need to be backported to 23.1.

for a smooth transition, this change should alias the syntax and merely hide the variant that uses "tenant" in the name.

The following surface should be adapted:

Epic: CRDB-29380

Jira issue: CRDB-29381
@knz knz added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) A-multitenancy Related to multi-tenancy T-multitenant Issues owned by the multi-tenant virtual team labels Jul 3, 2023
@exalate-issue-sync exalate-issue-sync bot added A-multitenancy Related to multi-tenancy T-multitenant Issues owned by the multi-tenant virtual team and removed A-multitenancy Related to multi-tenancy T-multitenant Issues owned by the multi-tenant virtual team labels Jul 3, 2023
This was referenced Jul 4, 2023
Merged
Merged
Merged
Merged
This was referenced Jul 4, 2023
Merged
Merged
craig bot pushed a commit that referenced this issue Jul 5, 2023
@knz @ecwall
Merge #106122 #106203
e8aedca 
106122: server: use HTTP query parameter `cluster` for manual selection r=yuzefovich a=knz

Informs #106068.
Epic: CRDB-29380

Prior to this patch, there was a debug-only way to manually force a HTTP request to be routed to a particular virtual cluster through the server controller. This was achieved via the query parameter `tenant_name`.

This patch renames the paramater to `cluster`, for a better UX coherence with the option of the same name in `cockroach sql`.

Release note: None

106203: sql: fix CREATE AS sourcing vtable panics r=chengxiong-ruan a=ecwall

Fixes #106166
Fixes #106167
Fixes #106168
Informs #105895

Fixes panics affects CREATE AS sourcing from vtables. These statements now work properly:
```
CREATE TABLE t AS SELECT * FROM pg_catalog.pg_prepared_statements;
CREATE MATERIALIZED VIEW v AS SELECT * FROM pg_catalog.pg_prepared_statements;
CREATE TABLE t AS SELECT * FROM pg_catalog.pg_cursors;
CREATE MATERIALIZED VIEW v AS SELECT * FROM pg_catalog.pg_cursors;
CREATE TABLE t AS SELECT * FROM crdb_internal.create_statements;
CREATE MATERIALIZED VIEW v AS SELECT * FROM crdb_internal.create_statements;
```

Co-authored-by: Raphael 'kena' Poss <[email protected]>
Co-authored-by: Evan Wall <[email protected]>
@blathers-crl blathers-crl bot mentioned this issue Jul 5, 2023
Merged
craig bot pushed a commit that referenced this issue Jul 6, 2023
@knz
Merge #106110
6319c0a 
106110: sql: emphasize "virtual cluster" in SQL syntax, keep "tenant" as alias r=yuzefovich a=knz

Informs #106068.
Epic: CRDB-29380

See individual commits for details.

Co-authored-by: Raphael 'kena' Poss <[email protected]>
craig bot pushed a commit that referenced this issue Jul 6, 2023
@knz
Merge #106116
6578cef 
106116: configprofiles: emphasize "cluster virtualization" r=yuzefovich a=knz

Informs #106068.
Epic: CRDB-29380

Before this patch:
```
$ cockroach start-single-node --config-profile=help
...
replication-source                  configuration suitable for a replication source cluster (alias for "multitenant+app+sharedservice+repl")
replication-target                  configuration suitable for a replication target cluster (alias for "multitenant+noapp+repl")`
multitenant+app+sharedservice       multi-tenant cluster with one secondary tenant configured to serve SQL application traffic
multitenant+app+sharedservice+repl  multi-tenant cluster with one secondary tenant configured to serve SQL application traffic, with replication enabled
multitenant+noapp                   multi-tenant cluster with no secondary tenant defined yet
multitenant+noapp+repl              multi-tenant cluster with no secondary tenant defined yet, with replication enabled
```

After this patch:
```
replication-source              configuration suitable for a replication source cluster (alias for "virtual+app+sharedservice+repl")
replication-target              configuration suitable for a replication target cluster (alias for "virtual+noapp+repl")
virtual+app+sharedservice       one virtual cluster configured to serve SQL application traffic
virtual+app+sharedservice+repl  one virtual cluster configured to serve SQL application traffic, with replication enabled
virtual+noapp                   virtualization enabled but no virtual cluster defined yet
virtual+noapp+repl              virtualization enabled but no virtual cluster defined yet, with replication enabled
```

Release note: None

Co-authored-by: Raphael 'kena' Poss <[email protected]>
@knz
Copy link
Contributor Author

knz commented Jul 6, 2023 •
edited
Loading

backport list:

craig bot pushed a commit that referenced this issue Jul 6, 2023
@knz
Merge #106117
5e87ff5 
106117: cli/zip: emphasize "cluster virtualization" r=yuzefovich a=knz

Informs #106068.
Epic: [CRDB-29380](https://cockroachlabs.atlassian.net/browse/CRDB-29380)

Release note (cli change): When `debug zip` is pointed at a multi-tenant cluster or a cluster with virtualization enabled, data about virtual clusters is now stored in a `virtual` sub-directory (this was previously named `tenants`).

Co-authored-by: Raphael 'kena' Poss <[email protected]>
@knz knz mentioned this issue Jul 6, 2023
Merged
craig bot pushed a commit that referenced this issue Jul 6, 2023
@knz
Merge #106121
06fde61 
106121: sql: session var `disable_drop_virtual_cluster` r=yuzefovich a=knz

Informs #106068.
Epic: CRDB-29380

This replaces `disable_drop_tenant`, together with the cluster setting `sql.drop_virtual_cluster.enabled` (replaces
`sql.drop_tenant.enabled`).

We are OK with this breaking change since the feature was not yet exposed to end-users.

Release note: None

Co-authored-by: Raphael 'kena' Poss <[email protected]>
craig bot pushed a commit that referenced this issue Jul 7, 2023
@knz
Merge #106298
9dda82c 
106298: cli/zip: Use "cluster" to identify the logical cluster instead of "tenant" r=yuzefovich a=knz

Informs #106068.
Epic: CRDB-29380

Requested by Abbey Russell in replacement of #106117.

Release note (cluster virtualization): This patch renames the subdirectory inside the generated zip file to be `cluster`, which brings the UX in coherence with the parameter of the same name in `cockroach sql` and the HTTP API parameter in the same name.

Co-authored-by: Raphael 'kena' Poss <[email protected]>
@knz knz mentioned this issue Jul 7, 2023
Merged
craig bot pushed a commit that referenced this issue Oct 2, 2023
@knz @fqazi
Merge #106118 #109415 #111397
0be8311 
106118: pkg/ui: emphasize "cluster virtualization" in human-visible surfaces r=yuzefovich a=knz

Informs #106068.
Epic: CRDB-29380

At this stage, we are focusing on strings and comments that are read by humans. A separate pass will take care of the method and variable names later.

Release note: None

109415: server, all: rename some cluster settings r=rafiss a=knz

Fixes #75520.
Epic: CRDB-28893

The following settings have been renamed, to emphasize how the initial drain wait is a *mandatory wait*, whereas the others are simply *timeouts*.

Release note (cli change): The following cluster settings have been renamed; the previous names remain available for
backward-compatibility.

| Previous name                         | New Name                                           |
|---------------------------------------|----------------------------------------------------|
| `server.shutdown.drain_wait`          | `server.shutdown.initial_wait`                     |
| `server.shutdown.lease_transfer_wait` | `server.shutdown.lease_transfer_iteration.timeout` |
| `server.shutdown.query_wait`          | `server.shutdown.queries.timeout`                  |
| `server.shutdown.connection_wait`     | `server.shutdown.transactions.timeout`              |
| `server.shutdown.jobs_wait`           | `server.shutdown.jobs.timeout`                     |

111397: catalog/lease: reduce lock contention on descVersionState r=fqazi a=fqazi

Recently, a regression was noticed on master related to AcquireByName being slower between builds. Using a profiler, this was tracked down to contention on descriptorVersionState's mutex.  To address this, this patch avoids acquiring this lock twice in the hot path, which resolves the regression.

After:   BenchmarkLeaseAcquireByNameCached-24    	 2378610	       451.9 ns/op
Before:  BenchmarkLeaseAcquireByNameCached-24    	 1493899	       727.9 ns/op
f13c021: BenchmarkLeaseAcquireByNameCached-24    	 2129446	       568.1 ns/op

Fixes: #111094

Release note: None

Co-authored-by: Raphael 'kena' Poss <[email protected]>
Co-authored-by: Faizan Qazi <[email protected]>
@blathers-crl blathers-crl bot mentioned this issue Oct 2, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@knz knz

Labels
A-multitenancy Related to multi-tenancy C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-multitenant Issues owned by the multi-tenant virtual team
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@knz