You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Yuqi Du (Code Review)" <ge...@cloudera.org> on 2022/04/12 10:02:34 UTC

[kudu-CR] [master] Add a rebalance request/response and provide a tool, refact master's rebalance

Yuqi Du has uploaded this change for review. ( http://gerrit.cloudera.org:8080/18402


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

[master] Add a rebalance request/response and provide a tool, refact master's rebalance

Change-Id: I5d506d877662c029a30dca9dd5029b9da4162946
---
M src/kudu/client/client-internal.cc
M src/kudu/client/client-internal.h
M src/kudu/client/client.cc
M src/kudu/client/client.h
M src/kudu/master/auto_rebalancer.cc
M src/kudu/master/auto_rebalancer.h
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master.proto
M src/kudu/master/master_service.cc
M src/kudu/master/master_service.h
M src/kudu/tools/tool_action_cluster.cc
12 files changed, 204 insertions(+), 64 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/02/18402/1
-- 
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: newchange
Gerrit-Change-Id: I5d506d877662c029a30dca9dd5029b9da4162946
Gerrit-Change-Number: 18402
Gerrit-PatchSet: 1
Gerrit-Owner: Yuqi Du <sh...@gmail.com>

[kudu-CR] [master] Add a rebalance request/response and provide a tool, refact master's rebalance

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18402 )

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


Patch Set 15:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/18402/15//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/18402/15//COMMIT_MSG@12
PS15, Line 12: 2. Add a rebalance request/response, user can trigger rebalance task,
             : even if the rebalancer is disabled.
Why to introduce it at all?  What's the purpose?  I'm not sure I can see the explanation here.  With current rebalancer tool, you can do rebalancing right now without those extras.


http://gerrit.cloudera.org:8080/#/c/18402/15/src/kudu/master/auto_rebalancer.h
File src/kudu/master/auto_rebalancer.h:

http://gerrit.cloudera.org:8080/#/c/18402/15/src/kudu/master/auto_rebalancer.h@84
PS15, Line 84: DataRebalance
If that's about rebalancing replica distribution, why to call this 'DataRebalance'?  Seems a bit misleading to me.


http://gerrit.cloudera.org:8080/#/c/18402/15/src/kudu/master/master.proto
File src/kudu/master/master.proto:

http://gerrit.cloudera.org:8080/#/c/18402/15/src/kudu/master/master.proto@849
PS15, Line 849: DATA_REBALANCE
This naming is misleading -- current rebalancing isn't data rebalancing, it simply evens out replica distribution in terms of number of those per tablet server.  That's not data rebalancing per se, not at all.


http://gerrit.cloudera.org:8080/#/c/18402/15/src/kudu/master/master.proto@859
PS15, Line 859:   required int64 task_id = 1;
What purpose does 'task_id' have in this message and what its significance?  Does it bring some info on the error happened, as per the comment above?  This looks a bit strange.



-- 
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: 15
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@g.ucla.edu>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Fri, 20 May 2022 16:32:17 +0000
Gerrit-HasComments: Yes

[kudu-CR] WIP [master] Add a rebalance request/response and provide a tool, refact master's rebalance

Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Kudu Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18402

to look at the new patch set (#5).

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

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

1. Refact master's auto rebalance, enable/disable auto rebalancer
need not restart master.

2. Add a rebalance request/response, user can trigger rebalance task,
even if the rebalancer is disabled.

3. Add a new rebalance tool, and compatible with incoming's leader
rebalance.

Change-Id: I5d506d877662c029a30dca9dd5029b9da4162946
---
M src/kudu/client/client-internal.cc
M src/kudu/client/client-internal.h
M src/kudu/client/client.cc
M src/kudu/client/client.h
M src/kudu/client/master_proxy_rpc.cc
M src/kudu/master/auto_rebalancer.cc
M src/kudu/master/auto_rebalancer.h
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master.proto
M src/kudu/master/master_service.cc
M src/kudu/master/master_service.h
M src/kudu/tools/tool_action_cluster.cc
13 files changed, 231 insertions(+), 68 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/02/18402/5
-- 
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: newpatchset
Gerrit-Change-Id: I5d506d877662c029a30dca9dd5029b9da4162946
Gerrit-Change-Number: 18402
Gerrit-PatchSet: 5
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>

[kudu-CR] [master] Add a rebalance request/response and provide a tool, refact master's rebalance

Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18402

to look at the new patch set (#15).

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

[master] Add a rebalance request/response and provide a tool, refact master's rebalance

1. Refact master's auto rebalance, enable/disable auto rebalancer
need not restart master.

2. Add a rebalance request/response, user can trigger rebalance task,
even if the rebalancer is disabled.

3. Add a new rebalance tool, and compatible with incoming's leader
rebalance.

Change-Id: I5d506d877662c029a30dca9dd5029b9da4162946
---
M src/kudu/master/auto_rebalancer.cc
M src/kudu/master/auto_rebalancer.h
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master.proto
M src/kudu/master/master_service.cc
M src/kudu/master/master_service.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_cluster.cc
M src/kudu/util/threadpool-test.cc
10 files changed, 330 insertions(+), 70 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/02/18402/15
-- 
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: newpatchset
Gerrit-Change-Id: I5d506d877662c029a30dca9dd5029b9da4162946
Gerrit-Change-Number: 18402
Gerrit-PatchSet: 15
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>

[kudu-CR] [master] Add a rebalance request/response and provide a tool, refact master's rebalance

Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Yuqi Du has abandoned this change. ( http://gerrit.cloudera.org:8080/18402 )

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


Abandoned

split it into 2: https://gerrit.cloudera.org/c/18679/ and https://gerrit.cloudera.org/c/18681/
-- 
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: abandon
Gerrit-Change-Id: I5d506d877662c029a30dca9dd5029b9da4162946
Gerrit-Change-Number: 18402
Gerrit-PatchSet: 15
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@g.ucla.edu>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>

[kudu-CR] WIP [master] Add a rebalance request/response and provide a tool, refact master's rebalance

Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18402

to look at the new patch set (#12).

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

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

1. Refact master's auto rebalance, enable/disable auto rebalancer
need not restart master.

2. Add a rebalance request/response, user can trigger rebalance task,
even if the rebalancer is disabled.

3. Add a new rebalance tool, and compatible with incoming's leader
rebalance.

Change-Id: I5d506d877662c029a30dca9dd5029b9da4162946
---
M src/kudu/master/auto_rebalancer.cc
M src/kudu/master/auto_rebalancer.h
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master.proto
M src/kudu/master/master_service.cc
M src/kudu/master/master_service.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_cluster.cc
M src/kudu/util/threadpool-test.cc
10 files changed, 317 insertions(+), 70 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/02/18402/12
-- 
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: newpatchset
Gerrit-Change-Id: I5d506d877662c029a30dca9dd5029b9da4162946
Gerrit-Change-Number: 18402
Gerrit-PatchSet: 12
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>

[kudu-CR] [master] Add a rebalance request/response and provide a tool, refact master's rebalance

Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18402

to look at the new patch set (#14).

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

[master] Add a rebalance request/response and provide a tool, refact master's rebalance

1. Refact master's auto rebalance, enable/disable auto rebalancer
need not restart master.

2. Add a rebalance request/response, user can trigger rebalance task,
even if the rebalancer is disabled.

3. Add a new rebalance tool, and compatible with incoming's leader
rebalance.

Change-Id: I5d506d877662c029a30dca9dd5029b9da4162946
---
M src/kudu/master/auto_rebalancer.cc
M src/kudu/master/auto_rebalancer.h
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master.proto
M src/kudu/master/master_service.cc
M src/kudu/master/master_service.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_cluster.cc
M src/kudu/util/threadpool-test.cc
10 files changed, 335 insertions(+), 70 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/02/18402/14
-- 
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: newpatchset
Gerrit-Change-Id: I5d506d877662c029a30dca9dd5029b9da4162946
Gerrit-Change-Number: 18402
Gerrit-PatchSet: 14
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>

[kudu-CR] WIP [master] Add a rebalance request/response and provide a tool, refact master's rebalance

Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Kudu Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18402

to look at the new patch set (#7).

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

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

1. Refact master's auto rebalance, enable/disable auto rebalancer
need not restart master.

2. Add a rebalance request/response, user can trigger rebalance task,
even if the rebalancer is disabled.

3. Add a new rebalance tool, and compatible with incoming's leader
rebalance.

Change-Id: I5d506d877662c029a30dca9dd5029b9da4162946
---
M src/kudu/client/client-internal.cc
M src/kudu/client/client-internal.h
M src/kudu/client/client.cc
M src/kudu/client/client.h
M src/kudu/client/master_proxy_rpc.cc
M src/kudu/master/auto_rebalancer.cc
M src/kudu/master/auto_rebalancer.h
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master.proto
M src/kudu/master/master_service.cc
M src/kudu/master/master_service.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_cluster.cc
14 files changed, 318 insertions(+), 59 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/02/18402/7
-- 
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: newpatchset
Gerrit-Change-Id: I5d506d877662c029a30dca9dd5029b9da4162946
Gerrit-Change-Number: 18402
Gerrit-PatchSet: 7
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>

[kudu-CR] WIP [master] Add a rebalance request/response and provide a tool, refact master's rebalance

Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Yuqi Du 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 11:

(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:   // a list of potentially local hosts. It would be better to iterate over all of
             :   // the local network adapters. See KUDU-327.
             :   string hostname;
             :   RETURN_NOT_OK(GetFQDN(&hostname));
             : 
             :   // We don't want to consider 'localhost' to be local - otherwise if a misconfigured
             :   // server reports its own name as localhost, all clients will hammer it.
             :   if (hostname != "localhost" && hostname != "localhost.localdomain") {
             :     local_host_names_.insert(hostname);
             :     VLOG(1) << "Considering host " << hostname << " local";
             :   }
             : 
             :   vector<Sockaddr> addresses;
             :   RETURN_NOT_OK_PREPEND(dns_resolver_->ResolveAddresses(HostPort(hostname, 0),
             :                                                         &addresses),
             :       Substitute("Could not resolve local host name '$0'", hostname));
             : 
             :   for (const Sockaddr& addr : addresses) {
             :     // Similar to above, ignore local or wildcard addresses.
             :  
> Yes.  It should be a synchronized task at master server side. I 'll think i
To support the requirement, I add a timer at thread pool, the cr is: https://gerrit.cloudera.org/c/18447/.
I'll use it to do synchronize task.


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:   ///
             :   /// This method does an RPC to ensure that the table exists and
             :   /// looks up its sc
> I'm not very sure, other approch, I just use approch the same as CreateTabl
Your idea is reasonable, I adopt the suggest and move it from client to CLI tool.



-- 
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: 11
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Thu, 28 Apr 2022 03:01:38 +0000
Gerrit-HasComments: Yes

[kudu-CR] WIP [master] Add a rebalance request/response and provide a tool, refact master's rebalance

Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Yuqi Du 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 5:

(2 comments)

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

http://gerrit.cloudera.org:8080/#/c/18402/4/src/kudu/master/catalog_manager.cc@3484
PS4, Line 3484:                                  const boost::optional<const std::string&>& /*user*/,
> warning: the parameter #3 is copied for each invocation but only used as a 
Done


http://gerrit.cloudera.org:8080/#/c/18402/4/src/kudu/master/catalog_manager.cc@3500
PS4, Line 3500:                                   const optional<const string&>& user) {
> warning: the parameter 'user' is copied for each invocation but only used a
Done



-- 
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: 5
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Wed, 13 Apr 2022 08:13:40 +0000
Gerrit-HasComments: Yes

[kudu-CR] WIP [master] Add a rebalance request/response and provide a tool, refact master's rebalance

Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Kudu Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18402

to look at the new patch set (#6).

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

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

1. Refact master's auto rebalance, enable/disable auto rebalancer
need not restart master.

2. Add a rebalance request/response, user can trigger rebalance task,
even if the rebalancer is disabled.

3. Add a new rebalance tool, and compatible with incoming's leader
rebalance.

Change-Id: I5d506d877662c029a30dca9dd5029b9da4162946
---
M src/kudu/client/client-internal.cc
M src/kudu/client/client-internal.h
M src/kudu/client/client.cc
M src/kudu/client/client.h
M src/kudu/client/master_proxy_rpc.cc
M src/kudu/master/auto_rebalancer.cc
M src/kudu/master/auto_rebalancer.h
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master.proto
M src/kudu/master/master_service.cc
M src/kudu/master/master_service.h
M src/kudu/tools/tool_action_cluster.cc
13 files changed, 250 insertions(+), 67 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/02/18402/6
-- 
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: newpatchset
Gerrit-Change-Id: I5d506d877662c029a30dca9dd5029b9da4162946
Gerrit-Change-Number: 18402
Gerrit-PatchSet: 6
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>

[kudu-CR] WIP [master] Add a rebalance request/response and provide a tool, refact master's rebalance

Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18402

to look at the new patch set (#2).

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

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

Change-Id: I5d506d877662c029a30dca9dd5029b9da4162946
---
M src/kudu/client/client-internal.cc
M src/kudu/client/client-internal.h
M src/kudu/client/client.cc
M src/kudu/client/client.h
M src/kudu/master/auto_rebalancer.cc
M src/kudu/master/auto_rebalancer.h
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master.proto
M src/kudu/master/master_service.cc
M src/kudu/master/master_service.h
M src/kudu/tools/tool_action_cluster.cc
12 files changed, 204 insertions(+), 64 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/02/18402/2
-- 
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: newpatchset
Gerrit-Change-Id: I5d506d877662c029a30dca9dd5029b9da4162946
Gerrit-Change-Number: 18402
Gerrit-PatchSet: 2
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] WIP [master] Add a rebalance request/response and provide a tool, refact master's rebalance

Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18402

to look at the new patch set (#3).

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

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

1. Refact master's auto rebalance, enable/disable auto rebalancer
need not restart master.

2. Add a rebalance request/response, user can trigger rebalance task,
even if the rebalancer is disabled.

3. Add a new rebalance tool, and compatible with incoming's leader
rebalance.

Change-Id: I5d506d877662c029a30dca9dd5029b9da4162946
---
M src/kudu/client/client-internal.cc
M src/kudu/client/client-internal.h
M src/kudu/client/client.cc
M src/kudu/client/client.h
M src/kudu/client/master_proxy_rpc.cc
M src/kudu/master/auto_rebalancer.cc
M src/kudu/master/auto_rebalancer.h
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master.proto
M src/kudu/master/master_service.cc
M src/kudu/master/master_service.h
M src/kudu/tools/tool_action_cluster.cc
13 files changed, 220 insertions(+), 66 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/02/18402/3
-- 
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: newpatchset
Gerrit-Change-Id: I5d506d877662c029a30dca9dd5029b9da4162946
Gerrit-Change-Number: 18402
Gerrit-PatchSet: 3
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] WIP [master] Add a rebalance request/response and provide a tool, refact master's rebalance

Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Kudu Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18402

to look at the new patch set (#4).

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

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

1. Refact master's auto rebalance, enable/disable auto rebalancer
need not restart master.

2. Add a rebalance request/response, user can trigger rebalance task,
even if the rebalancer is disabled.

3. Add a new rebalance tool, and compatible with incoming's leader
rebalance.

Change-Id: I5d506d877662c029a30dca9dd5029b9da4162946
---
M src/kudu/client/client-internal.cc
M src/kudu/client/client-internal.h
M src/kudu/client/client.cc
M src/kudu/client/client.h
M src/kudu/client/master_proxy_rpc.cc
M src/kudu/master/auto_rebalancer.cc
M src/kudu/master/auto_rebalancer.h
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master.proto
M src/kudu/master/master_service.cc
M src/kudu/master/master_service.h
M src/kudu/tools/tool_action_cluster.cc
13 files changed, 229 insertions(+), 66 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/02/18402/4
-- 
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: newpatchset
Gerrit-Change-Id: I5d506d877662c029a30dca9dd5029b9da4162946
Gerrit-Change-Number: 18402
Gerrit-PatchSet: 4
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)

[kudu-CR] WIP [master] Add a rebalance request/response and provide a tool, refact master's rebalance

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
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 <sh...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Mon, 18 Apr 2022 20:42:24 +0000
Gerrit-HasComments: Yes

[kudu-CR] WIP [master] Add a rebalance request/response and provide a tool, refact master's rebalance

Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Yuqi Du 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 h
Yes.  It should be a synchronized task at master server side. I 'll think it.


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 da
I'm not very sure, other approch, I just use approch the same as CreateTable. As you suggest I'll review the tool codes, then answer you.



-- 
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 <sh...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Tue, 19 Apr 2022 08:07:05 +0000
Gerrit-HasComments: Yes

[kudu-CR] WIP [master] Add a rebalance request/response and provide a tool, refact master's rebalance

Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18402

to look at the new patch set (#9).

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

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

1. Refact master's auto rebalance, enable/disable auto rebalancer
need not restart master.

2. Add a rebalance request/response, user can trigger rebalance task,
even if the rebalancer is disabled.

3. Add a new rebalance tool, and compatible with incoming's leader
rebalance.

Change-Id: I5d506d877662c029a30dca9dd5029b9da4162946
---
M src/kudu/client/client-internal.cc
M src/kudu/client/client-internal.h
M src/kudu/client/client.cc
M src/kudu/client/client.h
M src/kudu/client/master_proxy_rpc.cc
M src/kudu/master/auto_rebalancer.cc
M src/kudu/master/auto_rebalancer.h
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master.proto
M src/kudu/master/master_service.cc
M src/kudu/master/master_service.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_cluster.cc
M src/kudu/util/net/socket-test.cc
15 files changed, 284 insertions(+), 59 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/02/18402/9
-- 
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: newpatchset
Gerrit-Change-Id: I5d506d877662c029a30dca9dd5029b9da4162946
Gerrit-Change-Number: 18402
Gerrit-PatchSet: 9
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>

[kudu-CR] WIP [master] Add a rebalance request/response and provide a tool, refact master's rebalance

Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18402

to look at the new patch set (#13).

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

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

1. Refact master's auto rebalance, enable/disable auto rebalancer
need not restart master.

2. Add a rebalance request/response, user can trigger rebalance task,
even if the rebalancer is disabled.

3. Add a new rebalance tool, and compatible with incoming's leader
rebalance.

Change-Id: I5d506d877662c029a30dca9dd5029b9da4162946
---
M src/kudu/master/auto_rebalancer.cc
M src/kudu/master/auto_rebalancer.h
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master.proto
M src/kudu/master/master_service.cc
M src/kudu/master/master_service.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_cluster.cc
M src/kudu/util/threadpool-test.cc
10 files changed, 335 insertions(+), 70 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/02/18402/13
-- 
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: newpatchset
Gerrit-Change-Id: I5d506d877662c029a30dca9dd5029b9da4162946
Gerrit-Change-Number: 18402
Gerrit-PatchSet: 13
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>

[kudu-CR] WIP [master] Add a rebalance request/response and provide a tool, refact master's rebalance

Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Yuqi Du 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 4:

(6 comments)

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

http://gerrit.cloudera.org:8080/#/c/18402/3/src/kudu/client/client.cc@586
PS3, Line 586:   return KuduClient::Data::Rebalance(this);
> warning: static member accessed through instance [readability-static-access
Done


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

http://gerrit.cloudera.org:8080/#/c/18402/3/src/kudu/master/catalog_manager.cc@3482
PS3, Line 3482: Status CatalogManager::Rebalance(const RebalanceRequestPB* req,
> warning: parameter 'req' is unused [misc-unused-parameters]
Done


http://gerrit.cloudera.org:8080/#/c/18402/3/src/kudu/master/catalog_manager.cc@3483
PS3, Line 3483:                                  RebalanceResponsePB* resp,
> warning: parameter 'resp' is unused [misc-unused-parameters]
Done


http://gerrit.cloudera.org:8080/#/c/18402/3/src/kudu/master/catalog_manager.cc@3484
PS3, Line 3484:                                  boost::optional<const std::string&> /*user*/,
> warning: the parameter 'user' is copied for each invocation but only used a
Done


http://gerrit.cloudera.org:8080/#/c/18402/3/src/kudu/master/catalog_manager.cc@3485
PS3, Line 3485:                                  const security::TokenSigner* /*token_signer*/) {
> warning: parameter 'token_signer' is unused [misc-unused-parameters]
Done


http://gerrit.cloudera.org:8080/#/c/18402/3/src/kudu/master/catalog_manager.cc@3497
PS3, Line 3497: 
> warning: the parameter 'user' is copied for each invocation but only used a
temp reserve it.



-- 
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: 4
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Wed, 13 Apr 2022 07:42:49 +0000
Gerrit-HasComments: Yes

[kudu-CR] [master] Add a rebalance request/response and provide a tool, refact master's rebalance

Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/18402 )

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


Patch Set 14:

(3 comments)

Not a full review, just thought I'd put in the request to break this up

http://gerrit.cloudera.org:8080/#/c/18402/14//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/18402/14//COMMIT_MSG@8
PS14, Line 8: 
            : 1. Refact master's auto rebalance, enable/disable auto rebalancer
            : need not restart master.
            : 
            : 2. Add a rebalance request/response, user can trigger rebalance task,
            : even if the rebalancer is disabled.
            : 
            : 3. Add a new rebalance tool, and compatible with incoming's leader
            : rebalance.
            : 
Thanks for all these changes! But please split these into individual patches with unit tests for each logical behavior. It will be much easier for reviewers to review that way.


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

http://gerrit.cloudera.org:8080/#/c/18402/14/src/kudu/master/auto_rebalancer.cc@277
PS14, Line 277: #define NEXT_EXECUTE_FUNCTION                                                          \
This is never called, it seems.


http://gerrit.cloudera.org:8080/#/c/18402/14/src/kudu/master/auto_rebalancer.cc@300
PS14, Line 300:   if (moves_scheduled_this_round_for_test_ == 0) {
              :     // To Avoid double leaders.
              :     SleepFor(MonoDelta::FromMilliseconds(5 * FLAGS_raft_heartbeat_interval_ms));
              :   }
What was the behavioral change that led to this? I expected there to be few functional changes in this patch.



-- 
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: 14
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Sat, 07 May 2022 07:09:53 +0000
Gerrit-HasComments: Yes

[kudu-CR] WIP [master] Add a rebalance request/response and provide a tool, refact master's rebalance

Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18402

to look at the new patch set (#8).

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

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

1. Refact master's auto rebalance, enable/disable auto rebalancer
need not restart master.

2. Add a rebalance request/response, user can trigger rebalance task,
even if the rebalancer is disabled.

3. Add a new rebalance tool, and compatible with incoming's leader
rebalance.

Change-Id: I5d506d877662c029a30dca9dd5029b9da4162946
---
M src/kudu/client/client-internal.cc
M src/kudu/client/client-internal.h
M src/kudu/client/client.cc
M src/kudu/client/client.h
M src/kudu/client/master_proxy_rpc.cc
M src/kudu/master/auto_rebalancer.cc
M src/kudu/master/auto_rebalancer.h
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master.proto
M src/kudu/master/master_service.cc
M src/kudu/master/master_service.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_cluster.cc
14 files changed, 284 insertions(+), 58 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/02/18402/8
-- 
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: newpatchset
Gerrit-Change-Id: I5d506d877662c029a30dca9dd5029b9da4162946
Gerrit-Change-Number: 18402
Gerrit-PatchSet: 8
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>

[kudu-CR] [master] Add a rebalance request/response and provide a tool, refact master's rebalance

Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Yuqi Du has posted comments on this change. ( http://gerrit.cloudera.org:8080/18402 )

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


Patch Set 15:

(5 comments)

Thanks you crs.  

I will adopt your advices, split it into serval indepent crs.

http://gerrit.cloudera.org:8080/#/c/18402/14//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/18402/14//COMMIT_MSG@8
PS14, Line 8: 
            : 1. Refact master's auto rebalance, enable/disable auto rebalancer
            : need not restart master.
            : 
            : 2. Add a rebalance request/response, user can trigger rebalance task,
            : even if the rebalancer is disabled.
            : 
            : 3. Add a new rebalance tool, and compatible with incoming's leader
            : rebalance.
            : 
> Thanks for all these changes! But please split these into individual patche
Thanks your advices. I'll do it.


http://gerrit.cloudera.org:8080/#/c/18402/15/src/kudu/master/auto_rebalancer.h
File src/kudu/master/auto_rebalancer.h:

http://gerrit.cloudera.org:8080/#/c/18402/15/src/kudu/master/auto_rebalancer.h@84
PS15, Line 84: DataRebalance
> If that's about rebalancing replica distribution, why to call this 'DataReb
ReplicaRebalance  ok?


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

http://gerrit.cloudera.org:8080/#/c/18402/14/src/kudu/master/auto_rebalancer.cc@277
PS14, Line 277:   if (!FLAGS_auto_rebalancing_enabled) {
> This is never called, it seems.
Sorry, I forget remove it.


http://gerrit.cloudera.org:8080/#/c/18402/15/src/kudu/master/master.proto
File src/kudu/master/master.proto:

http://gerrit.cloudera.org:8080/#/c/18402/15/src/kudu/master/master.proto@849
PS15, Line 849: DATA_REBALANCE
> This naming is misleading -- current rebalancing isn't data rebalancing, it
I see.  I can change it.

The purpose is data rebalance, but the implements just use rebalance number of replicas to reach approximate  effect.


http://gerrit.cloudera.org:8080/#/c/18402/15/src/kudu/master/master.proto@859
PS15, Line 859:   required int64 task_id = 1;
> What purpose does 'task_id' have in this message and what its significance?
the task is async execution. so we should have a method to check if the task is finished.



-- 
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: 15
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@g.ucla.edu>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Tue, 28 Jun 2022 11:30:54 +0000
Gerrit-HasComments: Yes

[kudu-CR] WIP [master] Add a rebalance request/response and provide a tool, refact master's rebalance

Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18402

to look at the new patch set (#10).

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

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

1. Refact master's auto rebalance, enable/disable auto rebalancer
need not restart master.

2. Add a rebalance request/response, user can trigger rebalance task,
even if the rebalancer is disabled.

3. Add a new rebalance tool, and compatible with incoming's leader
rebalance.

Change-Id: I5d506d877662c029a30dca9dd5029b9da4162946
---
M src/kudu/master/auto_rebalancer.cc
M src/kudu/master/auto_rebalancer.h
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master.proto
M src/kudu/master/master_service.cc
M src/kudu/master/master_service.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_cluster.cc
M src/kudu/util/net/socket-test.cc
10 files changed, 278 insertions(+), 59 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/02/18402/10
-- 
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: newpatchset
Gerrit-Change-Id: I5d506d877662c029a30dca9dd5029b9da4162946
Gerrit-Change-Number: 18402
Gerrit-PatchSet: 10
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>

[kudu-CR] WIP [master] Add a rebalance request/response and provide a tool, refact master's rebalance

Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18402

to look at the new patch set (#11).

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

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

1. Refact master's auto rebalance, enable/disable auto rebalancer
need not restart master.

2. Add a rebalance request/response, user can trigger rebalance task,
even if the rebalancer is disabled.

3. Add a new rebalance tool, and compatible with incoming's leader
rebalance.

Change-Id: I5d506d877662c029a30dca9dd5029b9da4162946
---
M src/kudu/master/auto_rebalancer.cc
M src/kudu/master/auto_rebalancer.h
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master.proto
M src/kudu/master/master_service.cc
M src/kudu/master/master_service.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_cluster.cc
9 files changed, 277 insertions(+), 58 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/02/18402/11
-- 
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: newpatchset
Gerrit-Change-Id: I5d506d877662c029a30dca9dd5029b9da4162946
Gerrit-Change-Number: 18402
Gerrit-PatchSet: 11
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>