Gabriella Lotz has posted comments on this change. ( http://gerrit.cloudera.org:8080/24319 )
Change subject: KUDU-3730 Add observability counters to the auto-rebalancer ...................................................................... Patch Set 1: (4 comments) http://gerrit.cloudera.org:8080/#/c/24319/1/src/kudu/master/auto_rebalancer-test.cc File src/kudu/master/auto_rebalancer-test.cc: http://gerrit.cloudera.org:8080/#/c/24319/1/src/kudu/master/auto_rebalancer-test.cc@1433 PS1, Line 1433: > Can you please add a bit more test coverage? I've added RebalancerMetricsRoundsInStableCluster (test 4). For test 1 (leader moves): the natural approach would be an RF=1 workload, since every replica is a leader and FindReplicas always falls back to the leader path. However, BuildClusterInfo skips RF=1 tablets when move_rf1_replicas=false, which is hardcoded in the AutoRebalancerTask constructor. To exercise the leader-counter branch I would need to introduce a test-only production flag just to reach what is ultimately a two-line if/else increment that mirrors the already tested follower path. Test 2 (failure injection): auto_rebalancing_fail_moves_for_test injects failure into CheckMoveCompleted, not ExecuteMoves. The BulkChangeConfig RPC is sent and succeeds before CheckMoveCompleted is ever reached, so the leader/follower counters do increment. A test asserting they stay at zero would fail on a healthy cluster. Test 3 (continue path): There's no deterministic way to force BuildClusterRawInfo or BuildClusterInfo to fail in a mini-cluster test today without a dedicated injection flag. http://gerrit.cloudera.org:8080/#/c/24319/1/src/kudu/master/auto_rebalancer.cc File src/kudu/master/auto_rebalancer.cc: http://gerrit.cloudera.org:8080/#/c/24319/1/src/kudu/master/auto_rebalancer.cc@175 PS1, Line 175: kudu::MetricUnit::kTablets, > here and below: I might not have the best understanding here but these coun The kTablets were intentional. kUnits would be correct if we were counting something that has no natural tablet interpretation (like rounds, hence rounds_completed uses kUnits). http://gerrit.cloudera.org:8080/#/c/24319/1/src/kudu/rebalance/rebalancer.h File src/kudu/rebalance/rebalancer.h: http://gerrit.cloudera.org:8080/#/c/24319/1/src/kudu/rebalance/rebalancer.h@193 PS1, Line 193: bool FindReplicas(const TableReplicaMove& move, > If I saw correctly rebalancer_tool.cc now calls FindReplicas() and ignores Done http://gerrit.cloudera.org:8080/#/c/24319/1/src/kudu/rebalance/rebalancer.h@305 PS1, Line 305: false > That default can silently mislabel callers that forget to pass it. Requirin Done -- To view, visit http://gerrit.cloudera.org:8080/24319 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I34ab527033ed71b1b591d86387f7f3af2dd4f6a3 Gerrit-Change-Number: 24319 Gerrit-PatchSet: 1 Gerrit-Owner: Gabriella Lotz <[email protected]> Gerrit-Reviewer: Gabriella Lotz <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber <[email protected]> Gerrit-Comment-Date: Thu, 21 May 2026 14:56:50 +0000 Gerrit-HasComments: Yes
