kvserver: add AdminSplit and AdminScatter to secondary tenants API #74835
Conversation
|
This change is |
shralex
force-pushed
the
split_scatter_tenants
branch
from
13351dd
to
11cbb34
Compare
There was a problem hiding this comment.
🎉 thanks for getting to this so quickly.
Do we have a test we need to update to validate this behavior?
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @aayushshah15, @nvanbenschoten, and @shralex)
-- commits, line 2 at r1:
Might be good to give a bit more context as to why we're making this change. The context from #74389 might help. Also, can you link this epic to that issue (but maybe informs and not resolves)?
shralex
force-pushed
the
split_scatter_tenants
branch
from
11cbb34
to
2386268
Compare
There was a problem hiding this comment.
Updated now, thank you!
Reviewable status:
shralex
force-pushed
the
split_scatter_tenants
branch
from
2386268
to
4fdfc8f
Compare
There was a problem hiding this comment.
Reviewed 1 of 1 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status:
There was a problem hiding this comment.
Might be good to get someone from KV to quickly double check though.
Reviewable status:
There was a problem hiding this comment.
Reviewed 1 of 1 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status:
-- commits, line 13 at r3:
nit: extra release note.
pkg/rpc/auth_test.go, line 252 at r3 (raw file):
{ req: &roachpb.BatchRequest{Requests: makeReqs( makeAdminReq("a"),
These test cases were meant to represent an arbitrary unsupported admin operation — not a split in particular. Let's change makeAdminReq so that it doesn't perform an AdminSplit and instead performs an admin operation that still isn't permitted, like AdminRelocateRange. Then we can add additional test cases for AdminSplit and AdminScatter.
shralex
force-pushed
the
split_scatter_tenants
branch
2 times, most recently
from
d97b8a1
to
79c909e
Compare
This adds support for AdminSplit and AdminScatter for secondary tenants. This API allows indicating to KV that more data will be ingested, and so the range should be split and re-distributed across the cluster. This API will not be exposed through SQL, and in the future we might change it to give KV more control over whether and how to deal with expected ingest load. More discussion can be found in the github issue: cockroachdb#74389 and Epic: https://cockroachlabs.atlassian.net/browse/CRDB-10720 Release note: None
shralex
force-pushed
the
split_scatter_tenants
branch
from
79c909e
to
22f5264
Compare
There was a problem hiding this comment.
Reviewable status:
pkg/rpc/auth_test.go, line 252 at r3 (raw file):
AdminRelocateRange
Done
There was a problem hiding this comment.
Reviewable status:
Previously, nvanbenschoten (Nathan VanBenschoten) wrote…
nit: extra release note.
Done
There was a problem hiding this comment.
Reviewed 1 of 1 files at r4, all commit messages.
Reviewable status:
|
bors r+ |
|
Build succeeded: |
This adds support for AdminSplit and AdminScatter for secondary tenants. This API allows indicating to KV that more data will be ingested, and so the range should be split and re-distributed across the cluster. This API will not be exposed through SQL, and in the future we might change it to give KV more control over whether and how to deal with expected ingest load. More discussion can be found in the github issue: #74389 and Epic: https://cockroachlabs.atlassian.net/browse/CRDB-10720
Release Note: None