Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/23755 )

Change subject: auto_rebalancer: enforce max_moves_per_server per tserver
......................................................................


Patch Set 4: Code-Review+2

(3 comments)

LGTM, just a tiny nit to avoid compiler warnings in release builds.

http://gerrit.cloudera.org:8080/#/c/23755/3/src/kudu/master/auto_rebalancer.cc
File src/kudu/master/auto_rebalancer.cc:

http://gerrit.cloudera.org:8080/#/c/23755/3/src/kudu/master/auto_rebalancer.cc@367
PS3, Line 367:   RETURN_NOT_OK(algo->GetNextMoves(cluster_info, max_moves, 
&moves));
             :
> Could you clarify what you mean by comparing (src_ongoing + dst_ongoing) ag
Ah, it seems I misunderstood the counters in moves_per_tserver_ and  haven't 
articulated my thought properly.

You are right -- the point was to track the total number of in-flight 
operations per server, and that's exactly what moves_per_tserver_ does.  After 
looking closely, I realized all is well in that regard, indeed.


http://gerrit.cloudera.org:8080/#/c/23755/3/src/kudu/master/auto_rebalancer.cc@723
PS3, Line 723:       replica_moves->erase(replica_moves->begi
> The counter can legitimately be zero if ExecuteMoves() failed before proces
Thanks for the clarification.


http://gerrit.cloudera.org:8080/#/c/23755/4/src/kudu/master/auto_rebalancer.cc
File src/kudu/master/auto_rebalancer.cc:

http://gerrit.cloudera.org:8080/#/c/23755/4/src/kudu/master/auto_rebalancer.cc@270
PS4, Line 270:     for (const auto& entry : moves_per_tserver_) {
             :       DCHECK_EQ(0, entry.second) << "Tserver " << entry.first << 
" still has " << entry.second
             :                                  << " moves after all operations 
completed";
             :     }
nit: to avoid having empty 'for' cycle (and possible compiler warnings), 
consider enclosing this into #if DCHECK_IS_ON() ... #endif block:

#if DCHECK_IS_ON()
  ...
#endif



--
To view, visit http://gerrit.cloudera.org:8080/23755
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic1b73045719dc69cc9e3353d318b7381f144a20d
Gerrit-Change-Number: 23755
Gerrit-PatchSet: 4
Gerrit-Owner: Gabriella Lotz <[email protected]>
Gerrit-Reviewer: Abhishek Chennaka <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Gabriella Lotz <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 15 Jan 2026 02:05:20 +0000
Gerrit-HasComments: Yes

Reply via email to