server,pgwire: use a single SQL listener for multiple tenants #92580

knz · 2022-11-28T15:34:00Z

Parent PRs:

Fixes #84585.
Informs #94310.
Epic: CRDB-14537

This commit implements tenant routing using a single SQL network listener.

The tenant name can be specified:

via the client status param options, e.g. options=-ccrdb:tenant=hello
via the database name, e.g. crdb:tenant-hello.defaultdb.

Release note (backward-incompatible change): If a SQL database is created with a name that starts with crdb:tenant- (e.g. CREATE DATABASE "crdb:tenant-foo", clients will no longer be able to connect to it directly.

cockroach-teamcity · 2022-11-28T15:34:10Z

This change is

jaylim-crl · 2022-11-28T17:22:12Z

The tenant name can be specified:

via the client status param options, e.g. options=-ccrdb:tenant=hello

via the database name, e.g. crdb:tenant-hello.

If I understand correctly, this only works if we connect to the KV server, right (i.e. based on acceptTenantName)? Since sqlproxy doesn't route requests to KV pods, I'd imagine we can assume that sqlproxy won't have this support?

knz · 2022-11-28T17:24:52Z

@jaylim-crl I don't understand your question.

The logic implemented here only exists for shared-process multitenancy, which we've estimated will only be used in Self-hosted / Dedicated.

We may want to have a conversation about whether (and how) sqlproxy will be used with a multitenancy-enabled CC dedicated, but i'm not sure we need to have it now.

jaylim-crl · 2022-11-28T17:33:51Z

@jaylim-crl I don't understand your question.

The logic implemented here only exists for shared-process multitenancy, which we've estimated will only be used in Self-hosted / Dedicated.

We may want to have a conversation about whether (and how) sqlproxy will be used with a multitenancy-enabled CC dedicated, but i'm not sure we need to have it now.

I see. Thanks for the clarification; that explains it.

andy-kimball · 2022-11-29T01:15:36Z

I'm confused about what crdb:tenant-hello means. Isn't the name of the tenant hello in this case? But if that's the name of the tenant, then where is the name of the database? And if hello isn't the name of the tenant, then where is the name of the tenant specified? What's confusing me is this line in a test that's part of the PR:

db2 := serverutils.OpenDBConn(t, sqlAddr, "crdb:tenant-hello", false, s.Stopper())

I'd have expected "crdb:tenant-hello.defaultdb" here. Or maybe the defaultdb is implied if there's no DB specified?

knz · 2022-11-29T10:21:55Z

maybe the defaultdb is implied if there's no DB specified?

That is exactly what's happening.

what crdb:tenant-hello means. Isn't the name of the tenant hello in this case?

correct.

Release note: None

We need the read/parse of the parameters to be tenant-independent, so the function cannot contain tenant-specific configuration. Release note: None

This commit extracts the memory reservation into a tenant-independent memory pool. Release note: None

This demonstrates that the logic is tenant-independent and makes it ready to move into the PreServe() function. Release note: None

Release note: None

This patch introduces the new cluster setting `server.connector.tenant_name`, which determines which tenant to use to serve client connections when the client does not specify a tenant. This defaults to `system`, the name of the system tenant. Release note: None

This commit implements tenant routing using a single SQL network listener. The tenant name can be specified: - via the client status param `options`, e.g. `options=-ccluster=hello` - via the database name, e.g. `cluster:hello/mydb`. Release note (backward-incompatible change): If a SQL database is created with a name that starts with `cluster:...` (e.g. `CREATE DATABASE "cluster:foo"`, clients will no longer be able to connect to it directly via a pre-existing URL connection string. The URL needs to be modified in this case. For example: Previously: ``` postgres://servername/cluster:foo ``` Now: ``` postgres://servername/cluster:foo&options=-ccluster=system ``` (What this does: it selects the tenant named `system` and then the database named `cluster:foo` inside it. When the `-ccluster:system` option is not specified, `cluster:foo` in the database name position is interpreted as a request to connect to a tenant named `foo`, which likely does not exist.) Connections to databases whose name does not start with `cluster:` (the most common case) are not affected by this change.

knz · 2023-01-09T01:04:59Z

TFYRs!

bors r=rafiss

craig · 2023-01-09T01:50:51Z

Build succeeded:

Bazel Essential CI (Cockroach)

knz mentioned this pull request Nov 28, 2022

server,pgwire: use a single SQL listener for multiple tenants #91982

Closed

knz marked this pull request as ready for review November 28, 2022 15:44

knz requested review from a team November 28, 2022 15:44

knz added the A-multitenancy Related to multi-tenancy label Nov 28, 2022

knz force-pushed the 20221116-shared-sql-6 branch from 350d669 to 7437311 Compare November 28, 2022 16:41

knz requested review from a team, herkolategan and smg260 and removed request for a team November 28, 2022 16:41

knz force-pushed the 20221116-shared-sql-7 branch from 1603461 to 3feea70 Compare November 28, 2022 16:42

knz requested a review from rafiss November 28, 2022 16:59

knz force-pushed the 20221116-shared-sql-6 branch from 7437311 to d2ea8ad Compare November 28, 2022 17:14

knz requested review from a team November 28, 2022 17:14

knz force-pushed the 20221116-shared-sql-7 branch from 3feea70 to 9cca02f Compare November 28, 2022 17:16

knz force-pushed the 20221116-shared-sql-6 branch from d2ea8ad to 6f7b42b Compare November 28, 2022 18:28

knz force-pushed the 20221116-shared-sql-7 branch from 9cca02f to 32a0f50 Compare November 28, 2022 18:30

knz mentioned this pull request Nov 28, 2022

sqlproxycc: allow routing with composite db names #92619

Open

knz force-pushed the 20221116-shared-sql-6 branch from 6f7b42b to a8ba3a0 Compare November 29, 2022 14:59

knz force-pushed the 20221116-shared-sql-7 branch from 32a0f50 to b754185 Compare November 29, 2022 15:00

knz added 14 commits January 8, 2023 23:17

pgwire: make handleCancel unaware of conn/buf

54f789e

Release note: None

pgwire: split parseClientProvidedSessionParameters

7bd4484

We need the read/parse of the parameters to be tenant-independent, so the function cannot contain tenant-specific configuration. Release note: None

pgwire: split the pre-conn reserved memory account

c7dd014

This commit extracts the memory reservation into a tenant-independent memory pool. Release note: None

pgwire: lift the pre-serve logic into a closure

b5a2c43

This demonstrates that the logic is tenant-independent and makes it ready to move into the PreServe() function. Release note: None

pgwire: move PreServe() to its own method

e589e4b

Release note: None

pgwire: move maybeUpgradeToSecureConn to pre_serve.go

7963983

Release note: None

pgwire: move readVersion to pre_serve.go

41fe579

Release note: None

pgwire: move the parsing logic for client opts to a new file

317b426

Release note: None

pgwire: lift the PreServe() call to the server package

7eb7663

Release note: None

pgwire: pull the client addr trust logic into PreServe

6f62794

Release note: None

server: pull isReady.Set(true) out of startServeSQL

66505e5

Release note: None

server: pull pgL out of SQLServer

23ad180

Release note: None

server: pull startServeSQL out of SQLServer

7bf6fce

Release note: None

server: pull pgPreServer out of SQLServer

e5e3f31

Release note: None

knz force-pushed the 20221116-shared-sql-6 branch from b4c6bf4 to e5e3f31 Compare January 8, 2023 22:18

knz force-pushed the 20221116-shared-sql-7 branch from 88b0316 to 4067e7e Compare January 8, 2023 22:51

knz changed the base branch from 20221116-shared-sql-6 to master January 8, 2023 22:56

knz added 2 commits January 9, 2023 01:11

knz force-pushed the 20221116-shared-sql-7 branch from 4067e7e to a5ca120 Compare January 9, 2023 00:11

knz requested a review from a team as a code owner January 9, 2023 00:11

craig bot merged commit 27ebfa8 into master Jan 9, 2023

craig bot deleted the 20221116-shared-sql-7 branch January 9, 2023 01:50

szphie mentioned this pull request Jul 21, 2023

v23.1.5 utilitywarehouse/cockroachdb-manifests#69

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

server,pgwire: use a single SQL listener for multiple tenants #92580

server,pgwire: use a single SQL listener for multiple tenants #92580

knz commented Nov 28, 2022 •

edited

Loading

cockroach-teamcity commented Nov 28, 2022

jaylim-crl commented Nov 28, 2022

knz commented Nov 28, 2022

jaylim-crl commented Nov 28, 2022

andy-kimball commented Nov 29, 2022 •

edited

Loading

knz commented Nov 29, 2022

knz commented Jan 9, 2023

craig bot commented Jan 9, 2023

server,pgwire: use a single SQL listener for multiple tenants #92580

server,pgwire: use a single SQL listener for multiple tenants #92580

Conversation

knz commented Nov 28, 2022 • edited Loading

cockroach-teamcity commented Nov 28, 2022

jaylim-crl commented Nov 28, 2022

knz commented Nov 28, 2022

jaylim-crl commented Nov 28, 2022

andy-kimball commented Nov 29, 2022 • edited Loading

knz commented Nov 29, 2022

knz commented Jan 9, 2023

craig bot commented Jan 9, 2023

knz commented Nov 28, 2022 •

edited

Loading

andy-kimball commented Nov 29, 2022 •

edited

Loading