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

server,cli/demo: make the default tenant configurable #94901

Merged
merged 22 commits into from
Jan 9, 2023

Conversation

knz added 21 commits January 8, 2023 23:13
Prior to this patch, we were unconditionally applying a port offset
for secondary tenant servers, even when the port allocation for the KV
node was randomized. This resulted in the demo cluster trying to
reuse arbitrary TCP ports which may be already in use elsewhere in the
system.

This patch fixes it by ensuring that tenant servers also use random
allocation when the KV node ports were randomly allocated in the first
place.

It also adds demo networking tests that verify this via the
`--sql-port` / `--http-port` flags.

Release note: None
Some of the tests were not waiting for the prompt to start sending
their input. This was causing the input to be lost in some cases.

Release note: None
This commit introduces `PreServeConnHandler`, which will
handle the preparation of a SQL connection prior to handing it off to
a specific tenant server.

Release note (ops changes): The count of network bytes sent
to report re-authentication errors to a SQL client is now
reported via the metric `sql.pre_serve.bytesout` (instead of
`sql.bytesout` previously). The count of pre-authentication errors
is now reported via the metric `sql.pre_serve.conn.failures` (instead
of `sql.conn.failures` previously).

Release note (ops changes): The count of new SQL connections
is now also reported on `sql.pre_serve.new_conns`.
Release note (ops change): The bytes read from SQL clients
prior to authentication are now reported via the metric
`sql.pre_serve.bytesin` (instead of `sql.bytesin` previously).
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
@knz knz requested a review from a team January 8, 2023 23:40
@knz knz requested review from a team as code owners January 8, 2023 23:40
@knz knz requested a review from a team January 8, 2023 23:40
@knz knz requested review from herkolategan and srosenberg and removed request for a team January 8, 2023 23:40
@cockroach-teamcity
Copy link
Member

This change is Reviewable

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
craig bot pushed a commit that referenced this pull request Jan 9, 2023
92580: server,pgwire: use a single SQL listener for multiple tenants r=rafiss a=knz

Parent PRs:
- [x] #84608
- [x] #91739
- [x] #91744
- [x] #92574
- [x] #92575
- [x] #92576
- [x] #92577
- [x] #92578
- [x] #92579
- [ ] #94901

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.

Co-authored-by: Raphael 'kena' Poss <knz@thaumogen.net>
@craig craig bot merged commit 05955c8 into cockroachdb:master Jan 9, 2023
@knz knz deleted the 20221116-shared-sql-62 branch January 9, 2023 09:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

server: make the default tenant route configurable [CRDB-14537 followup]
2 participants