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

Change subject: WIP [master] Add a rebalance request/response and provide a 
tool, refact master's rebalance
......................................................................


Patch Set 7:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/18402/7/src/kudu/client/client-internal.cc
File src/kudu/client/client-internal.cc:

http://gerrit.cloudera.org:8080/#/c/18402/7/src/kudu/client/client-internal.cc@470
PS7, Line 470: Status KuduClient::Data::Rebalance(KuduClient* client) {
             :   RebalanceRequestPB req;
             :   req.set_type(RebalanceRequestPB::DATA_REBALANCE);
             :
             :   auto deadline = MonoTime::Now() + 
client->default_admin_operation_timeout();
             :   Synchronizer sync;
             :   RebalanceResponsePB resp;
             :
             :   AsyncLeaderMasterRpc<RebalanceRequestPB, RebalanceResponsePB> 
rpc(
             :     deadline, client, BackoffType::EXPONENTIAL, req, &resp,
             :     &MasterServiceProxy::RebalanceAsync, "Rebalance",
             :     sync.AsStatusCallback(), {}
             :   );
             :   rpc.SendRpc();
             :   RETURN_NOT_OK(sync.Wait());
             :   if (resp.has_error()) {
             :     return StatusFromPB(resp.error().status());
             :   }
             :   return Status::OK();
             : }
I don't think this is viable approach: the rebalancing could take several 
hours, so this call would time out with default settings for admin operation 
timeout.


http://gerrit.cloudera.org:8080/#/c/18402/7/src/kudu/client/client.h
File src/kudu/client/client.h:

http://gerrit.cloudera.org:8080/#/c/18402/7/src/kudu/client/client.h@763
PS7, Line 763:   /// Do all tables replica rebalance once.
             :   /// @return Status object for the operation.
             :   Status Rebalance();
I don't think rebalancing is something that's related to reading/writing data 
to/from a Kudu cluster, so the public client API isn't the right place to add 
this.

Why do you need this to be in the public client API at all?  Given the current 
approach used by the 'kudu cluster rebalance tool' or other CLI tools working 
with master, why not to continue with the similar approach there?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5d506d877662c029a30dca9dd5029b9da4162946
Gerrit-Change-Number: 18402
Gerrit-PatchSet: 7
Gerrit-Owner: Yuqi Du <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yuqi Du <[email protected]>
Gerrit-Comment-Date: Mon, 18 Apr 2022 20:42:24 +0000
Gerrit-HasComments: Yes

Reply via email to