Gabriella Lotz has posted comments on this change. ( http://gerrit.cloudera.org:8080/24525 )
Change subject: KUDU-3789 auto_rebalancer: reliably clear replace marker after failed moves ...................................................................... Patch Set 3: (3 comments) http://gerrit.cloudera.org:8080/#/c/24525/2/src/kudu/master/auto_rebalancer.cc File src/kudu/master/auto_rebalancer.cc: http://gerrit.cloudera.org:8080/#/c/24525/2/src/kudu/master/auto_rebalancer.cc@885 PS2, Line 885: constexpr int kMaxClearAttempts = 3; : Status clear_replace_status; : for (int attempt = 1; attempt <= kMaxClearAttempts; ++attempt) { : clear_replace_status = TryClearReplaceMarker(move); : // OK includes the case where the marker was already cleared. : if (clear_replace_status.ok()) break; : // Nothing left to clear: the replica is gone from the config (NotFound) : / > IIUC, AutoRebalancerTask::GetTabletLeader() (which is a part of AutoRebalan Good catch, this was a real problem. Changed GetTabletLeader() to return ServiceUnavailable for the no-leader case, so it now falls through to the inline retry. http://gerrit.cloudera.org:8080/#/c/24525/2/src/kudu/master/auto_rebalancer.cc@1079 PS2, Line 1079: HostPort leader_hp; : // Resolving the leader also re-checks our own catalog leadership (it takes the : // leader lock), so if we've lost it here, or there's no tablet leader yet, we : // bail out and the caller leaves the marker pending. : RETURN_NOT_OK(GetTabletLeader(move.tablet_uuid, &leader_uuid, &leader_hp)); : vector<Sockaddr> resolved; : RETURN_NOT_OK(leader_hp.ResolveAddre > Is there a way to utilize 'cas_config_opid_index' as recorded in the 'move' Agreed. I've added a CAS on the clear, so it only removes the marker if the config's opid_index hasn't moved since we read it. If another actor changed the config in between, the request just gets rejected. I read the opid live from the leader rather than the one recorded on the move, since that one predates our marker. http://gerrit.cloudera.org:8080/#/c/24525/2/src/kudu/master/auto_rebalancer.cc@1102 PS2, Line 1102: req.add_tablet_ids(move.tablet_uuid); : RETURN_NOT_OK(proxy.GetConsensusState(req, &resp, &rpc)); : if (resp.has_error()) { : return StatusFromPB(resp.error().status()); > Once exited from this scope, how do we know that we are still the leader to Good question. The clear path re-checks leadership itself: TryClearReplaceMarker() -> GetTabletLeader() grabs the leader lock again, so if we've lost leadership by then the clear just fails and the marker stays pending. The outer check is only a cheap early-out to skip the batch when we're clearly not the leader. -- To view, visit http://gerrit.cloudera.org:8080/24525 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia47698207612252bca3ec70fc090a37bd3f87809 Gerrit-Change-Number: 24525 Gerrit-PatchSet: 3 Gerrit-Owner: Gabriella Lotz <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Gabriella Lotz <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber <[email protected]> Gerrit-Reviewer: Zoltan Martonka <[email protected]> Gerrit-Comment-Date: Mon, 13 Jul 2026 12:26:27 +0000 Gerrit-HasComments: Yes
