Marton Greber has posted comments on this change. ( http://gerrit.cloudera.org:8080/24594 )
Change subject: auto_leader_rebalancer: best-effort leader step-down ...................................................................... Patch Set 6: (3 comments) Some observations, core changes look good. http://gerrit.cloudera.org:8080/#/c/24594/6/src/kudu/master/auto_leader_rebalancer-test.cc File src/kudu/master/auto_leader_rebalancer-test.cc: http://gerrit.cloudera.org:8080/#/c/24594/6/src/kudu/master/auto_leader_rebalancer-test.cc@555 PS6, Line 555: ASSERT_OK(leader_rebalancer->RunLeaderRebalancer()); q: this asserts moves were *scheduled* and the pass returned OK, which is the key regression guard. Since moves_scheduled_this_round_for_test_ is set before the transfer loop, it stays >0 even though the flag makes every transfer fail -- so the assertion doesn't actually confirm the failures were hit. Not necessarily worth expanding, but if you wanted to nail it down you could also assert on the WARNING log count (e.g. via a scoped log capture). Fine to leave as-is. http://gerrit.cloudera.org:8080/#/c/24594/6/src/kudu/master/auto_leader_rebalancer.cc File src/kudu/master/auto_leader_rebalancer.cc: http://gerrit.cloudera.org:8080/#/c/24594/6/src/kudu/master/auto_leader_rebalancer.cc@99 PS6, Line 99: DEFINE_bool(auto_leader_rebalancing_fail_moves_for_test, false, nit: the flag description says it forces "every leader step-down RPC issued by the leader rebalancer" to fail, but the injection point is only in RunLeaderRebalanceForTable (line 449) -- RunGlobalLeaderRebalance's step-down at line 728 is unaffected. Worth narrowing the wording to the per-table path so a future reader doesn't expect the global pass to honor it. (Also: "used for test" -> "used for testing".) http://gerrit.cloudera.org:8080/#/c/24594/6/src/kudu/master/auto_leader_rebalancer.cc@102 PS6, Line 102: TAG_FLAG(auto_leader_rebalancing_fail_moves_for_test, unsafe); nit: consider also tagging this hidden (alongside unsafe), consistent with other test-only injection flags, so it doesn't surface in normal flag listings. -- To view, visit http://gerrit.cloudera.org:8080/24594 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0fb2c7a8bf750520d34d9aba62274dbe7433d80d Gerrit-Change-Number: 24594 Gerrit-PatchSet: 6 Gerrit-Owner: Gabriella Lotz <[email protected]> Gerrit-Reviewer: Abhishek Chennaka <[email protected]> Gerrit-Reviewer: Gabriella Lotz <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber <[email protected]> Gerrit-Reviewer: Zoltan Chovan <[email protected]> Gerrit-Reviewer: Zoltan Martonka <[email protected]> Gerrit-Comment-Date: Thu, 30 Jul 2026 13:33:53 +0000 Gerrit-HasComments: Yes
