kvtenant: expose available regions to tenants #67098
Conversation
|
This change is |
otan
force-pushed
the
connector_v2
branch
2 times, most recently
from
1d3c613 to
e6c1f3e
Compare
otan
force-pushed
the
connector_v2
branch
2 times, most recently
from
1d94223 to
eadfeff
Compare
There was a problem hiding this comment.
Nice! Looks good to me, but I'm not very familiar with most of the code you've touched. Hopefully @irfansharif and/or @arulajmani can have a look too.
Reviewed 6 of 6 files at r1, 22 of 22 files at r2, 3 of 3 files at r3, 7 of 7 files at r4, 2 of 2 files at r5.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @arulajmani, @irfansharif, and @otan)
pkg/ccl/backupccl/testdata/backup-restore/multiregion, line 15 at r3 (raw file):
us-east-1 us-west-1 eu-central-1
Nit: hmm, why does "eu" come after "us" if we're ordering by region? 🤔
|
pkg/ccl/backupccl/testdata/backup-restore/multiregion, line 15 at r3 (raw file): Previously, ajstorm (Adam Storm) wrote…
The outer select can reorder things iiuc |
There was a problem hiding this comment.
Reviewed 5 of 6 files at r1, 34 of 34 files at r6, 22 of 22 files at r7, 3 of 3 files at r8.
Reviewable status:
pkg/server/serverpb/status.proto, line 1113 at r6 (raw file):
rpc Regions(RegionsRequest) returns (RegionsResponse) { option (google.api.http) = { get : "/_status/regions"
just trying to check my understanding here: we don't strictly need the http API annotation to enable this feature, right?
I think we're trying to avoid adding more HTTP endpoints under _status since we've moved ahead with the v2 API tree which will eventually end up marking everything under _status as deprecated. maybe @itsbilal can opine.
There was a problem hiding this comment.
Reviewable status:
pkg/server/serverpb/status.proto, line 1113 at r6 (raw file):
Previously, dhartunian (David Hartunian) wrote…
just trying to check my understanding here: we don't strictly need the http API annotation to enable this feature, right?
I think we're trying to avoid adding more HTTP endpoints under
_statussince we've moved ahead with thev2API tree which will eventually end up marking everything under_statusas deprecated. maybe @itsbilal can opine.
i've removed it, but it still shows up in the docs.
There was a problem hiding this comment.
Sorry for taking so long on this, breather week and what not :)
Reviewed 2 of 6 files at r1, 37 of 37 files at r11, 23 of 23 files at r12, 4 of 4 files at r13, 8 of 8 files at r14, 3 of 3 files at r15.
Reviewable status:
pkg/server/status.go, line 1299 at r11 (raw file):
ctx context.Context, req *serverpb.RegionsRequest, ) (*serverpb.RegionsResponse, error) { resp, _, err := s.nodesHelper(ctx, 0, 0)
nit: comments after constants
In preparation for exposing regions as a kvtenant.Connector API, introduce an endpoint which returns all valid regions in a cluster. Release note (general change): Introduce a /_status/regions endpoint which returns all regions along with their availability zones.
Release note (sql change): Introduce a crdb_internal.regions table which contains data on all regions in the cluster.
This currently collects the same data, but all the aggregation is already done on the crdb_internal.regions output. Release note: None
This enables the use of crdb_internal.regions from a tenant. Release note (general change): crdb_internal.regions is now accessible from a tenant.
Tests multi-region cluster with tenants. Only SHOW REGIONS FOR CLUSTER works at this stage. Release note: None
|
thanks bors r=arulajmani |
|
Build succeeded: |
5 participants
Resolves CRDB-2513
See individual commits for details.