Gabriella Lotz has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/23909 )

Change subject: KUDU-3728 Add range-aware rebalancing to auto_rebalancer
......................................................................


Patch Set 2:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/23909/1/src/kudu/master/auto_rebalancer-test.cc
File src/kudu/master/auto_rebalancer-test.cc:

http://gerrit.cloudera.org:8080/#/c/23909/1/src/kudu/master/auto_rebalancer-test.cc@492
PS1, Line 492:
> It would be great to have a high-level description of what this test scenar
Done


http://gerrit.cloudera.org:8080/#/c/23909/1/src/kudu/master/auto_rebalancer-test.cc@534
PS1, Line 534: ASSERT_OK(Cr
> nit: this could be a const reference, 'const string&' or 'const auto&'
Done


http://gerrit.cloudera.org:8080/#/c/23909/1/src/kudu/master/auto_rebalancer-test.cc@592
PS1, Line 592:   rebalance::ClusterRawInfo raw_info;
> In addition to verifying internal invariants as in the newly added test sce
Done


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

http://gerrit.cloudera.org:8080/#/c/23909/1/src/kudu/master/auto_rebalancer.cc@20
PS1, Line 20: include <atomic>
> nit: move cstddef into the rest of the header group below
Done


http://gerrit.cloudera.org:8080/#/c/23909/1/src/kudu/master/auto_rebalancer.cc@147
PS1, Line 147: indep
> I guess this should be 'true'.  In a separate change, the 'kudu cluster reb
Done


http://gerrit.cloudera.org:8080/#/c/23909/1/src/kudu/master/auto_rebalancer.cc@154
PS1, Line 154: DECLARE_bool(auto_rebalancing_enabled);
             :
             : namespace kudu {
> nit: move these to be adjacent to the auto_rebalancing_enable_range_rebalan
Done


http://gerrit.cloudera.org:8080/#/c/23909/1/src/kudu/master/auto_rebalancer.cc@614
PS1, Line 614: _begin = ss_range_key_begin.str();
> There is no need for two static casts.
The two casts serve different purposes: the uint8_t ensures the byte is treated 
as 0–255 (avoids signed?char sign extension), and the uint16_t widens it so the 
stream prints a number rather than a char.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idf716bef8c21395da98dd7c9e33001190d9b5587
Gerrit-Change-Number: 23909
Gerrit-PatchSet: 2
Gerrit-Owner: Gabriella Lotz <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Gabriella Lotz <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 24 Feb 2026 16:34:31 +0000
Gerrit-HasComments: Yes

Reply via email to