server: implement decommission pre-check api#93950
server: implement decommission pre-check api#93950craig[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
|
This change is |
4bdfb9d to
7450245
Compare
dhartunian
left a comment
There was a problem hiding this comment.
just looked at final commit. feel free to squash with #90222 for easier merge.
Reviewed 2 of 2 files at r1, 2 of 2 files at r2, 5 of 5 files at r3, 2 of 2 files at r4, 4 of 4 files at r5.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @AlexTalks)
pkg/server/admin.go line 2661 at r6 (raw file):
// DecommissionPreCheck runs checks and returns the DecommissionPreCheckResponse // for the given nodes. func (s *systemAdminServer) DecommissionPreCheck(
Can you add a test?
pkg/server/admin.go line 2663 at r6 (raw file):
func (s *systemAdminServer) DecommissionPreCheck( ctx context.Context, req *serverpb.DecommissionPreCheckRequest, ) (*serverpb.DecommissionPreCheckResponse, error) {
add ctx = s.AnnotateCtx(ctx)
do we need any permission checks here? I notice decom operation itself doesn't seem to have any...so perhaps it's left as a separate issue.
fcabdc2 to
60acd56
Compare
kvoli
left a comment
There was a problem hiding this comment.
Reviewed the last two commits.
It would be good to add a couple tests for the main RPC
Reviewed 15 of 15 files at r7, 4 of 5 files at r8, 6 of 6 files at r9, 1 of 3 files at r10.
Reviewable status:
pkg/server/admin.go line 2639 at r10 (raw file):
// DecommissionPreCheck runs checks and returns the DecommissionPreCheckResponse // for the given nodes. func (s *systemAdminServer) DecommissionPreCheck(
Are you waiting to add a client test for this fn? It would be good to add something in this commit.
afdc2e9 to
f44f661
Compare
This comment was marked as outdated.
This comment was marked as outdated.
f44f661 to
b363fc4
Compare
kvoli
left a comment
There was a problem hiding this comment.
Reviewed 16 of 16 files at r11, all commit messages.
Reviewable status:
b363fc4 to
6698537
Compare
This change implements the `DecommissionPreCheck` RPC on the `Admin` service, using the support for evaluating node decommission readiness by checking each range introduced in cockroachdb#93758. In checking node decommission readiness, only nodes that have a valid, non-`DECOMMISSIONED` liveness status are checked, and ranges with replicas on the checked nodes that encounter errors in attempting to allocate replacement replicas are reported in the response. Ranges that have replicas on multiple checked nodes have their errors reported for each nodeID in the request list. Depends on cockroachdb#93758, cockroachdb#90222. Epic: CRDB-20924 Release note: None
6698537 to
e033cc6
Compare
|
bors r+ |
|
Build succeeded: |
This change implements the
DecommissionPreCheckRPC on theAdminservice, using the support for evaluating node decommission readiness by
checking each range introduced in #93758. In checking node decommission
readiness, only nodes that have a valid, non-
DECOMMISSIONEDlivenessstatus are checked, and ranges with replicas on the checked nodes that
encounter errors in attempting to allocate replacement replicas are
reported in the response. Ranges that have replicas on multiple checked
nodes have their errors reported for each nodeID in the request list.
Depends on #93758, #90222.
Epic: CRDB-20924
Release note: None