Hannah Nguyen has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14177 )

Change subject: KUDU-2780: create thread for auto-rebalancing
......................................................................


Patch Set 16:

(3 comments)

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

http://gerrit.cloudera.org:8080/#/c/14177/12/src/kudu/master/auto_rebalancer.cc@732
PS12, Line 732: string orig_leader_uuid;
              :   HostPort orig_leader_hp;
              :   RETURN_NOT_OK(GetTabletLeader(tablet_uuid, &orig_leader_uuid, 
&orig_leader_hp));
              :   shared_ptr<ConsensusServiceProxy> proxy;
              :   shared_ptr<TSDescriptor> desc;
              :   if (!ts_manager_->LookupTSByUUID(orig_leader_uuid, &desc)) {
              :     return Status::NotFound("Could not find destination 
tserver");
              :   }
              :   RETURN_NOT_OK(desc->GetConsensusProxy(messenger_, &proxy));
              :
              :   // Check if replica at 'to_ts_uuid' is in the config, and if 
it has been
              :   // promoted to voter.
              :   ConsensusStatePB cstate;
              :   GetConsensusStateRequestPB req;
              :   GetConsensusStateResponsePB resp;
              :   RpcController rpc;
              :   
rpc.set_timeout(MonoDelta::FromSeconds(FLAGS_auto_rebalancing_timeout_seconds));
              :   req.set_dest_uuid(orig_leader_uuid
> Yeah I'm fine improving this later. Could you add a TODO pointing to the ot
Done


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

http://gerrit.cloudera.org:8080/#/c/14177/14/src/kudu/master/auto_rebalancer.cc@207
PS14, Line 207:     // If not the leader, don't do anything.
> I think it's better to use ConditionVariable::WaitFor(FLAGS_auto_rebalancin
Done


http://gerrit.cloudera.org:8080/#/c/14177/14/src/kudu/master/auto_rebalancer.cc@271
PS14, Line 271:         if (closing_) {
> I think it make sense to check for 'closing_' while waiting for the replica
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifca25d1063c07047cf2123e6792b3c7395be20e4
Gerrit-Change-Number: 14177
Gerrit-PatchSet: 16
Gerrit-Owner: Hannah Nguyen <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Andrew Wong <[email protected]>
Gerrit-Reviewer: Hannah Nguyen <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 20 Feb 2020 22:52:04 +0000
Gerrit-HasComments: Yes

Reply via email to