tenant: add apiv2 support for sql-over-http #91323

dhartunian · 2022-11-04T21:52:16Z

This commit adds partial support for the /api/v2 HTTP server on tenants. Currently, we only support the SQL endpoint since this functionality is needed by newer DB Console features.

The API V2 Server initialization no longer relies on the global Server object, and only requires a SQLServer instead which makes it easier to play nicely with tenants. However, we still have the problem of incompatible status and admin servers which are in use on other endpoints. Future work will unify the tenant scoped versions of these servers and allow them to be used here, enabling full API V2 compatibility on tenants.

Informs: #80789

Release note (ops change): sql tenants now support the HTTP endpoint under /api/v2/sql which allows the caller to execute an HTTP request containing SQL statements to execute. The JSON response contains the results. This endpoints works identically as on a non-tenant server, except that it naturally scopes to the target tenant for SQL execution.

Epic: CRDB-17356

cockroach-teamcity · 2022-11-04T21:52:26Z

This change is

knz

pls add "informs #80789 " in the PR description thx

otherwise

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

91081: ui: migrate statement diagnostics requests to use sql-over-http endpoint r=THardy98 a=THardy98 Part of: #89429 Addresses: #90273, #90274, #90275 Blocked from resolving by #80789 DB-Console Demo: https://www.loom.com/share/d74c02eb3dfb41fc913577d0e8992441 CC-Console Demo: https://www.loom.com/share/2adba8a53b8c48ac9f031e4481f284cf This change migrates the existing statement diagnostics requests to use the sql-over-http endpoint on our apiV2, making these requests tenant-scoped once the sql-over-http endpoint is scoped to tenants (this should be the case when #91323 is completed). 91534: lint: remove Golint r=yuzefovich a=yuzefovich This commit removes `TestGolint` linter as well as all mentions of `golint` I could find. The rationale is that - the linter is no longer maintained - is partially duplicated with the `staticcheck` - due to a recent change in `gcexportdata.Find`, if we bump some dependencies, it takes on the order of an hour when run via bazel, and we'd need to fork `golint` to go around that. Epic: None Release note: None 91626: kvstreamer: improve the behavior with very low budget r=yuzefovich a=yuzefovich This commit makes it so that the streamer tries a bit harder to not hit its low budget limit in `Enqueue`. Previously, due to some of the internal state being reused across different `Enqueue` batches (in particular, the truncation helper as well as some things in the results buffer), the streamer's budget might be insufficient to account for the newly-enqueued requests, and we would error out. This commit makes it so that - if the budget limit is reached - we first discard the overhead (freeing up some budget) and then try to consume the requests' footprint again. This shouldn't be a big deal with reasonably large workmem limits, but we have seen some problems when it is on the order of 100KiB or less. Fixes: #91587. Release note: None 91662: multi-tenant: Bump uninitialized storage cluster version r=knz a=ajstorm Previously, in cases where the version value in system.tenant_settings was uninitialized, we assumed that that the version was 22.1. With the change of the build tag to 23.1, we need to adjust the previous version to 22.1.76. This will need one more bump when we finally mint the 22.2 version number in master. Resolves: #91589 Release note: None Co-authored-by: Thomas Hardy <[email protected]> Co-authored-by: Yahor Yuzefovich <[email protected]> Co-authored-by: Adam Storm <[email protected]>

This commit adds partial support for the `/api/v2` HTTP server on tenants. Currently, we *only* support the SQL endpoint since this functionality is needed by newer DB Console features. The API V2 Server initialization no longer relies on the global `Server` object, and only requires a `SQLServer` instead which makes it easier to play nicely with tenants. However, we still have the problem of incompatible status and admin servers which are in use on other endpoints. Future work will unify the tenant scoped versions of these servers and allow them to be used here, enabling full API V2 compatibility on tenants. Release note (ops change): sql tenants now support the HTTP endpoint under `/api/v2/sql` which allows the caller to execute an HTTP request containing SQL statements to execute. The JSON response contains the results. This endpoints works identically as on a non-tenant server, except that it naturally scopes to the target tenant for SQL execution. Epic: CRDB-17356

Part of: cockroachdb#89429 Addresses: cockroachdb#90272 (blocked from resolving by cockroachdb#80789) This change migrates the existing `/events` request to use the sql-over-http endpoint on apiV2, making this request tenant-scoped once the sql-over-http endpoint is scoped to tenants (this should be the case when cockroachdb#91323 is completed). Release note: None

dhartunian · 2022-11-11T20:56:51Z

bors r=knz

craig · 2022-11-11T22:09:00Z

Build succeeded:

Bazel Essential CI (Cockroach)

blathers-crl · 2022-11-11T22:09:13Z

Encountered an error creating backports. Some common things that can go wrong:

The backport branch might have already existed.
There was a merge conflict.
The backport branch contained merge commits.

You might need to create your backport manually using the backport tool.

error creating merge commit from cb938ed to blathers/backport-release-22.2-91323: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict []

you may need to manually resolve merge conflicts with the backport tool.

Backport to branch 22.2.x failed. See errors above.

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.}

yuzefovich · 2022-11-12T03:11:47Z

It appears that the linter on Github CI complains about a change in this PR:

=== RUN   TestLint/TestStaticCheck
    lint_test.go:1751: 
        pkg/server/api_v2.go:103:2: field tenantID is unused (U1000)
    --- FAIL: TestLint/TestStaticCheck (932.91s)

dhartunian · 2022-11-14T16:17:51Z

Thanks @yuzefovich I've filed a fix: #91840

Part of: cockroachdb#89429 Addresses: cockroachdb#90272 (blocked from resolving by cockroachdb#80789) This change migrates the existing `/events` request to use the sql-over-http endpoint on apiV2, making this request tenant-scoped once the sql-over-http endpoint is scoped to tenants (this should be the case when cockroachdb#91323 is completed). Release note: None

91745: ui: move events endpoint to use sql-over-http r=THardy98 a=THardy98 Part of: #89429 Addresses: #90272 (blocked from resolving by #80789) Demo (DB-Console only): https://www.loom.com/share/bb67044a8560407fb703af2a6bbb2d9a This change migrates the existing `/events` request to use the sql-over-http endpoint on apiV2, making this request tenant-scoped once the sql-over-http endpoint is scoped to tenants (this should be the case when #91323 is completed). Release note: None Co-authored-by: Thomas Hardy <[email protected]>

dhartunian requested a review from a team November 4, 2022 21:52

dhartunian requested review from a team as code owners November 4, 2022 21:52

dhartunian requested a review from a team November 4, 2022 21:52

dhartunian requested review from a team as code owners November 4, 2022 21:52

THardy98 mentioned this pull request Nov 7, 2022

ui: migrate statement diagnostics requests to use sql-over-http endpoint #91081

Merged

knz approved these changes Nov 7, 2022

View reviewed changes

dhartunian force-pushed the api-v2-sql-over-http-only-support branch from 061264e to 7065d44 Compare November 8, 2022 16:17

dhartunian force-pushed the api-v2-sql-over-http-only-support branch from 7065d44 to cb938ed Compare November 10, 2022 22:48

THardy98 mentioned this pull request Nov 11, 2022

ui: move events endpoint to use sql-over-http #91745

Merged

dhartunian added the backport-22.2.x label Nov 11, 2022

craig bot merged commit 111f018 into cockroachdb:master Nov 11, 2022

cockroach-teamcity mentioned this pull request Nov 11, 2022

PR #91323 - tenant: add apiv2 support for sql-over-http cockroachdb/docs#15606

Closed

dhartunian deleted the api-v2-sql-over-http-only-support branch November 14, 2022 16:37

dhartunian mentioned this pull request Nov 16, 2022

release-22.2: tenant: add apiv2 support for sql-over-http #91994

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tenant: add apiv2 support for sql-over-http #91323

tenant: add apiv2 support for sql-over-http #91323

dhartunian commented Nov 4, 2022 •

edited

Loading

cockroach-teamcity commented Nov 4, 2022

knz left a comment

dhartunian commented Nov 11, 2022

craig bot commented Nov 11, 2022

blathers-crl bot commented Nov 11, 2022

yuzefovich commented Nov 12, 2022

dhartunian commented Nov 14, 2022

tenant: add apiv2 support for sql-over-http #91323

tenant: add apiv2 support for sql-over-http #91323

Conversation

dhartunian commented Nov 4, 2022 • edited Loading

cockroach-teamcity commented Nov 4, 2022

knz left a comment

Choose a reason for hiding this comment

dhartunian commented Nov 11, 2022

craig bot commented Nov 11, 2022

blathers-crl bot commented Nov 11, 2022

yuzefovich commented Nov 12, 2022

dhartunian commented Nov 14, 2022

dhartunian commented Nov 4, 2022 •

edited

Loading