kvserver: add AdminSplit and AdminScatter to secondary tenants API#74835
kvserver: add AdminSplit and AdminScatter to secondary tenants API#74835craig[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
13351dd to
11cbb34
Compare
ajstorm
left a comment
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)?
11cbb34 to
2386268
Compare
shralex
left a comment
There was a problem hiding this comment.
Updated now, thank you!
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @aayushshah15, @nvanbenschoten, and @shralex)
2386268 to
4fdfc8f
Compare
ajstorm
left a comment
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:complete! 1 of 0 LGTMs obtained (waiting on @aayushshah15, @nvanbenschoten, and @shralex)
ajstorm
left a comment
There was a problem hiding this comment.
Might be good to get someone from KV to quickly double check though.
Reviewable status:
complete! 1 of 0 LGTMs obtained (waiting on @aayushshah15, @nvanbenschoten, and @shralex)
nvb
left a comment
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:complete! 1 of 0 LGTMs obtained (waiting on @aayushshah15 and @shralex)
-- 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.
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
79c909e to
22f5264
Compare
shralex
left a comment
There was a problem hiding this comment.
Reviewable status:
complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @aayushshah15, @ajstorm, @nvanbenschoten, and @shralex)
pkg/rpc/auth_test.go, line 252 at r3 (raw file):
AdminRelocateRange
Done
shralex
left a comment
There was a problem hiding this comment.
Reviewable status:
complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @aayushshah15, @ajstorm, @nvanbenschoten, and @shralex)
Previously, nvanbenschoten (Nathan VanBenschoten) wrote…
nit: extra release note.
Done
nvb
left a comment
There was a problem hiding this comment.
Reviewed 1 of 1 files at r4, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (and 1 stale) (waiting on @aayushshah15 and @shralex)
|
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