Skip to content

cli: support --locality and --max-offset flags with sql tenant pods#73943

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
nvb:nvanbenschoten/tenantLocality
Dec 17, 2021
Merged

cli: support --locality and --max-offset flags with sql tenant pods#73943
craig[bot] merged 1 commit intocockroachdb:masterfrom
nvb:nvanbenschoten/tenantLocality

Conversation

@nvb
Copy link
Copy Markdown
Contributor

@nvb nvb commented Dec 16, 2021

This commit adds support for the --locality and --max-offset flags to the cockroach mt start-sql command.

The first of these is important because tenant SQL pods should know where they reside. This will be important in the future for multi-region serverless and also for projects like #72593.

The second of these is important because the SQL pod's max-offset setting needs to be the same as the host cluster's. If we want to be able to configure the host cluster's maximum clock offset to some non-default value, we'll need SQL pods to be configured identically.

Validation of plumbing:

./cockroach start-single-node --insecure --max-offset=250ms
./cockroach sql --insecure -e 'select crdb_internal.create_tenant(2)'

 # verify --max-offset

./cockroach mt start-sql --insecure --tenant-id=2 --sql-addr=:26258 --http-addr=:0
 # CRDB crashes with error "locally configured maximum clock offset (250ms) does not match that of node [::]:62744 (500ms)"

./cockroach mt start-sql --insecure --tenant-id=2 --sql-addr=:26258 --http-addr=:0 --max-offset=250ms
 # successful

 # verify --locality

./cockroach sql --insecure --port=26258 -e 'select gateway_region()'

ERROR: gateway_region(): no region set on the locality flag on this node

./cockroach mt start-sql --insecure --tenant-id=2 --sql-addr=:26258 --http-addr=:0 --max-offset=250ms --locality=region=us-east1

./cockroach sql --insecure --port=26258 -e 'select gateway_region()'

  gateway_region
------------------
  us-east1

This commit adds support for the `--locality` and `--max-offset` flags to the
`cockroach mt start-sql` command.

The first of these is important because tenant SQL pods should know where they
reside. This will be important in the future for multi-region serverless and
also for projects like cockroachdb#72593.

The second of these is important because the SQL pod's max-offset setting needs
to be the same as the host cluster's. If we want to be able to configure the
host cluster's maximum clock offset to some non-default value, we'll need SQL
pods to be configured identically.

Validation of plumbing:
```sh
./cockroach start-single-node --insecure --max-offset=250ms
./cockroach sql --insecure -e 'select crdb_internal.create_tenant(2)'

 # verify --max-offset

./cockroach mt start-sql --insecure --tenant-id=2 --sql-addr=:26258 --http-addr=:0
 # CRDB crashes with error "locally configured maximum clock offset (250ms) does not match that of node [::]:62744 (500ms)"

./cockroach mt start-sql --insecure --tenant-id=2 --sql-addr=:26258 --http-addr=:0 --max-offset=250ms
 # successful

 # verify --locality

./cockroach sql --insecure --port=26258 -e 'select gateway_region()'

ERROR: gateway_region(): no region set on the locality flag on this node

./cockroach mt start-sql --insecure --tenant-id=2 --sql-addr=:26258 --http-addr=:0 --max-offset=250ms --locality=region=us-east1

./cockroach sql --insecure --port=26258 -e 'select gateway_region()'

  gateway_region
------------------
  us-east1
```
@nvb nvb requested review from jaylim-crl and tbg December 16, 2021 21:33
@nvb nvb requested a review from a team as a code owner December 16, 2021 21:33
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

Copy link
Copy Markdown
Contributor

@jaylim-crl jaylim-crl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @tbg)

@nvb
Copy link
Copy Markdown
Contributor Author

nvb commented Dec 16, 2021

TFTR!

bors r+

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Dec 17, 2021

Build failed (retrying...):

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Dec 17, 2021

Build succeeded:

@craig craig bot merged commit 089affc into cockroachdb:master Dec 17, 2021
@nvb nvb deleted the nvanbenschoten/tenantLocality branch December 22, 2021 01:10
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.

3 participants