Yuqi Du has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/19608 )

Change subject: [master] Exclude tservers in MAINTENANCE_MODE when leader 
rebalancing
......................................................................


Patch Set 11:

(5 comments)

> Patch Set 11:
>
> (6 comments)

Thanks for your advices

http://gerrit.cloudera.org:8080/#/c/19608/11/src/kudu/master/auto_leader_rebalancer-test.cc
File src/kudu/master/auto_leader_rebalancer-test.cc:

http://gerrit.cloudera.org:8080/#/c/19608/11/src/kudu/master/auto_leader_rebalancer-test.cc@225
PS11, Line 225:   master::AutoRebalancerTask* replica_rebalancer = 
master->catalog_manager()->auto_rebalancer();
> It seems nobody use it, remove it.
Done


http://gerrit.cloudera.org:8080/#/c/19608/11/src/kudu/master/auto_leader_rebalancer-test.cc@231
PS11, Line 231: currentTserverIndex
> nit: kCurrentTserverIndex
Done


http://gerrit.cloudera.org:8080/#/c/19608/11/src/kudu/master/auto_leader_rebalancer-test.cc@251
PS11, Line 251: leader_rebalancer->RunLeaderRebalancer()
> nit: Add ASSERT_OK?
Done


http://gerrit.cloudera.org:8080/#/c/19608/11/src/kudu/master/auto_leader_rebalancer-test.cc@259
PS11, Line 259:   Status status = CheckLeaderBalance();
> If replicas are not balanced, the leader replicas are not balanced too, so
1. It's not necessary to check whether replicas is balanced. because tserver is 
3 and table's replication_factor is 3. so it's replicas balanced.

2. It's checking whether leaders is balanced. The method is in 
RunLeaderRebalanceForTable in AutoLeaderRebalancerTask::ExecuteMode::TEST mode? 
if some leader tasks are generated
it's not rebalanced, the tasks is not running just for this check condition.

It's not a perfect method, later we should evolute it the a better one.


http://gerrit.cloudera.org:8080/#/c/19608/11/src/kudu/master/auto_leader_rebalancer.cc
File src/kudu/master/auto_leader_rebalancer.cc:

http://gerrit.cloudera.org:8080/#/c/19608/11/src/kudu/master/auto_leader_rebalancer.cc@251
PS11, Line 251:         // Avoid transferring leaders to tservers that are in 
MAINTENANCE_MODE.
> How about move the comments to L396, where is the MAINTENANCE_MODE apears?
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2f85a675e69fd02a62e2625881dad2ca5e27acd9
Gerrit-Change-Number: 19608
Gerrit-PatchSet: 11
Gerrit-Owner: Yuqi Du <[email protected]>
Gerrit-Reviewer: Abhishek Chennaka <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Attila Bukor <[email protected]>
Gerrit-Reviewer: KeDeng <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <[email protected]>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <[email protected]>
Gerrit-Reviewer: Yifan Zhang <[email protected]>
Gerrit-Reviewer: Yingchun Lai <[email protected]>
Gerrit-Reviewer: Yuqi Du <[email protected]>
Gerrit-Comment-Date: Tue, 04 Apr 2023 07:20:44 +0000
Gerrit-HasComments: Yes

Reply via email to