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/28 03:29:42 UTC
[kudu-CR] WIP [master] KUDU-3061 support auto rebalance tablet leaders across TServers
Yuqi Du has uploaded this change for review. ( http://gerrit.cloudera.org:8080/18454
Change subject: WIP [master] KUDU-3061 support auto rebalance tablet leaders across TServers
......................................................................
WIP [master] KUDU-3061 support auto rebalance tablet leaders across TServers
The number of leader tablets per tablet server can become imbalanced
over time, putting additional pressure on a few nodes.
A CLI tool or an extension to the existing balancer should be added to
take care of this.
Currently the only option is running leader_step_down manually.
TODO:
1. adding unit test, use TimerThread KUDU-3364 to refact codes.
2. Add a leader rebalance CLI tool.
Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
---
M src/kudu/master/CMakeLists.txt
A src/kudu/master/auto_leader_rebalancer-test.cc
A src/kudu/master/auto_leader_rebalancer.cc
A src/kudu/master/auto_leader_rebalancer.h
M src/kudu/master/auto_rebalancer-test.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
9 files changed, 951 insertions(+), 31 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18454/1
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 1
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
[kudu-CR] [master] KUDU-3390 support auto rebalance tablet leaders across TServers
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Yuqi Du has posted comments on this change. ( http://gerrit.cloudera.org:8080/18454 )
Change subject: [master] KUDU-3390 support auto rebalance tablet leaders across TServers
......................................................................
Patch Set 34:
(27 comments)
I am glad to receive lots of crs.
Thank you very much.
I have fixed them and you can review them again.
http://gerrit.cloudera.org:8080/#/c/18454/32/src/kudu/master/auto_leader_rebalancer-test.cc
File src/kudu/master/auto_leader_rebalancer-test.cc:
http://gerrit.cloudera.org:8080/#/c/18454/32/src/kudu/master/auto_leader_rebalancer-test.cc@60
PS32, Line 60: (auto_rebalancing_interval_seconds);
> Is this needed here?
Remove it.
And some DECLARE_* and METRIC_DECLARE* below also not used, they should be removed.
http://gerrit.cloudera.org:8080/#/c/18454/32/src/kudu/master/auto_leader_rebalancer-test.cc@62
PS32, Line 62: (auto_rebalancing_wait_for
> And the same for several other declarations of gflags in this section: are
Done
http://gerrit.cloudera.org:8080/#/c/18454/32/src/kudu/master/auto_leader_rebalancer-test.cc@70
PS32, Line 70: class LeaderRebalancerTest : public KuduTest {
: public:
: Status CreateAndStartCluster() {
> Are these needed in this file?
Done
http://gerrit.cloudera.org:8080/#/c/18454/32/src/kudu/master/auto_leader_rebalancer-test.cc@139
PS32, Line 139: rs;
> How do we know that the distribution of the leader replicas is balanced? I
Yes, the describe is not correct, I will fix it.
when create tables, the table's tablet replicas and leaders is decided by algorithms of assign tablets.
http://gerrit.cloudera.org:8080/#/c/18454/32/src/kudu/master/auto_leader_rebalancer-test.cc@150
PS32, Line 150:
> How do we know at this point that the cluster is leader-balanced?
That's default status, leader not balanced. I fix the comments.
http://gerrit.cloudera.org:8080/#/c/18454/32/src/kudu/master/auto_leader_rebalancer-test.cc@168
PS32, Line 168: SleepFor(MonoDelta::FromMillisecond
> That's a really long sleep time: that's not a good idea to have something l
ok
http://gerrit.cloudera.org:8080/#/c/18454/32/src/kudu/master/auto_leader_rebalancer-test.cc@169
PS32, Line 169:
> I'm curious why is the number of retries so high? Is it possible to make t
I set a small value for each iteration to cover more branches, eg: auto_leader_balancer.cc:296
http://gerrit.cloudera.org:8080/#/c/18454/32/src/kudu/master/auto_leader_rebalancer-test.cc@175
PS32, Line 175: const int kNumTServers = 4;
> Why is it necessary to have a pause here? Would be great to add a comment
I looks the sleep is not necessary, but I try to remove it the test is failed.
I think master's tablets meta about roles, distributed is not realtime, it need wait heartbeat reports.
http://gerrit.cloudera.org:8080/#/c/18454/32/src/kudu/master/auto_leader_rebalancer.h
File src/kudu/master/auto_leader_rebalancer.h:
http://gerrit.cloudera.org:8080/#/c/18454/32/src/kudu/master/auto_leader_rebalancer.h@45
PS32, Line 45: // A CatalogManager background task which auto-rebalances tablets' leaders distribution.
> nit: remove this period
I have not understand this.
http://gerrit.cloudera.org:8080/#/c/18454/32/src/kudu/master/auto_leader_rebalancer.h@46
PS32, Line 46: by transferring leadership
> by transferring leadership between tablet replicas
Done
http://gerrit.cloudera.org:8080/#/c/18454/32/src/kudu/master/auto_leader_rebalancer.h@95
PS32, Line 95: // send rpc messages for ’LeaderStepDown‘
> nit: please document this member as well -- what is it for?
Done
http://gerrit.cloudera.org:8080/#/c/18454/32/src/kudu/master/auto_leader_rebalancer.cc
File src/kudu/master/auto_leader_rebalancer.cc:
http://gerrit.cloudera.org:8080/#/c/18454/32/src/kudu/master/auto_leader_rebalancer.cc@55
PS32, Line 55: using kudu::consensus::ConsensusServiceProxy;
: using kudu::consensus::LeaderStepDownMode;
: using kudu::consensus::LeaderStepDownRequestPB;
: using kudu::consensus::LeaderStepDownResponsePB;
: using kudu::consensus::RaftPeerPB;
: using kudu::rpc::MessengerBuilder;
: using kudu::rpc::RpcController;
: using std::map;
: using std::nullopt;
: using std::string;
: using std::vector;
: using strings::Substitute;
:
: DEFINE_uint32(auto_leader_
> nit: remove empty lines and sort the rest of non-empty lines alphabetically
Done
http://gerrit.cloudera.org:8080/#/c/18454/32/src/kudu/master/auto_leader_rebalancer.cc@293
PS32, Line 293:
> What's "synchronized rpc"?
I mean the thread would wait the response synchronously.
I fix the comments.
http://gerrit.cloudera.org:8080/#/c/18454/32/src/kudu/master/auto_leader_rebalancer.cc@293
PS32, Line 293: = 0;
> very?
Done
http://gerrit.cloudera.org:8080/#/c/18454/32/src/kudu/master/auto_leader_rebalancer.cc@299
PS32, Line 299: break;
> I don't think this sort of information is worth it having in the INFO log:
Done
http://gerrit.cloudera.org:8080/#/c/18454/32/src/kudu/master/auto_leader_rebalancer.cc@304
PS32, Line 304: reques
> nit: could this be 'const string&' or 'const auto&' to avoid copying?
Done
http://gerrit.cloudera.org:8080/#/c/18454/32/src/kudu/master/auto_leader_rebalancer.cc@330
PS32, Line 330:
> Drop the namespace prefix since there is corresponding 'using' using string
Done
http://gerrit.cloudera.org:8080/#/c/18454/32/src/kudu/master/auto_leader_rebalancer.cc@337
PS32, Line 337:
> What's VLOG(0), and why not to use VLOG(1) for this?
The log can show information about the leader rebalance runs simply and directly.
The log print only 1 record every period (default 1 hour), I think it's a necessary for INFO currently, In the future, I think we can add some metrics and remove the log.
http://gerrit.cloudera.org:8080/#/c/18454/32/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:
http://gerrit.cloudera.org:8080/#/c/18454/32/src/kudu/master/catalog_manager.cc@353
PS32, Line 353: automatic leader rebala
> automatic leader rebalancing
The style like line 346, whether it's necessary to change it ?
I change it firstly.
http://gerrit.cloudera.org:8080/#/c/18454/32/src/kudu/master/catalog_manager.cc@1062
PS32, Line 1062: depend
> depends
Done
http://gerrit.cloudera.org:8080/#/c/18454/32/src/kudu/master/catalog_manager.cc@1063
PS32, Line 1063: auto-rebalancing
> auto-rebalancing
Done
http://gerrit.cloudera.org:8080/#/c/18454/32/src/kudu/master/catalog_manager.cc@1063
PS32, Line 1063: rebalanci
> rebalancing
Done
http://gerrit.cloudera.org:8080/#/c/18454/32/src/kudu/master/catalog_manager.cc@1064
PS32, Line 1064: kudu
> cannot
Done
http://gerrit.cloudera.org:8080/#/c/18454/32/src/kudu/master/catalog_manager.cc@1064
PS32, Line 1064: the effect of
> What is 'load rebalance'?
the effect of leader rebalancing, I fix the words.
http://gerrit.cloudera.org:8080/#/c/18454/32/src/kudu/master/catalog_manager.cc@1067
PS32, Line 1067: isabled,
> ?
I did not know "?", maybe I should rewrite the comments, I try to do this.
http://gerrit.cloudera.org:8080/#/c/18454/32/src/kudu/master/catalog_manager.cc@1067
PS32, Line 1067: ven if a
> because
Done
http://gerrit.cloudera.org:8080/#/c/18454/32/src/kudu/master/catalog_manager.cc@1067
PS32, Line 1067: ries to
> followers?
Done
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 34
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.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: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Fri, 07 Oct 2022 08:38:15 +0000
Gerrit-HasComments: Yes
[kudu-CR] [master] KUDU-3061 support auto rebalance tablet leaders across TServers
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Yuqi Du has posted comments on this change. ( http://gerrit.cloudera.org:8080/18454 )
Change subject: [master] KUDU-3061 support auto rebalance tablet leaders across TServers
......................................................................
Patch Set 21:
(2 comments)
Thanks your crs.
http://gerrit.cloudera.org:8080/#/c/18454/19//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18454/19//COMMIT_MSG@10
PS19, Line 10: which lead to load skew on some nodes.
> Do you have any data to show how much imbalance it is? I'm not convinced t
I take some time to find an issue about leader imbalance at our internal JIRA before.
The issue is about user query kudu failed because of timeout. Because the cluster only 3 tservers so the replica is balanced.
We can see leader imbalance, All tables' leader distribution:
kudu cluster ksck $master_list -ksck_format=plain_full | grep LEADER | awk '{print$2}' | sort | uniq -c
60 (YYY-prod1.ZZZ.XXX.cloud:7050):
4 (tYYY-prod2.ZZZ.XXX.cloud:7050):
113 (YYY-prod3.ZZZ.XXX.cloud:7050):
node prod2 only 4 leaders and node prod3 has 113 leaders, the good distribution is (59, 59, 59) or so.
The table event_wos_p2_correction's leader distribution:
kudu cluster ksck $master_list -ksck_format=plain_full --tables=event_wos_p2_correction | grep LEADER | awk '{print$2}' | sort | uniq -c
6 (YYY-prod1.ZZZ.XXX.cloud:7050):
12 (YYY-prod3.ZZZ.XXX.cloud:7050):
We can see node2 has no leader, the good distribution is (6, 6, 6) or so.
http://gerrit.cloudera.org:8080/#/c/18454/19//COMMIT_MSG@21
PS19, Line 21: maybe cause imbalanced load
> Yes, I understand it's not easy to setup a proper measurement experiment an
OK, I have found a history issue and reply an above cr, but cpu usage and memory cann't find.
After a few days, I can make a workload to test some data about cpu and memory usage.
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 21
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: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Tue, 26 Jul 2022 04:25:05 +0000
Gerrit-HasComments: Yes
[kudu-CR] [master] KUDU-3390 support auto rebalance tablet leaders across TServers
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Yingchun Lai, Yifan Zhang, Kudu Jenkins, Andrew Wong, Abhishek Chennaka,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18454
to look at the new patch set (#37).
Change subject: [master] KUDU-3390 support auto rebalance tablet leaders across TServers
......................................................................
[master] KUDU-3390 support auto rebalance tablet leaders across TServers
The number of leader replicas per tablet server can become imbalanced
over time, which lead to load skew on some nodes. This patch adds
auto leader rebalance task to avoid leader replicas skew.
Two reasons of load skew:
- The main reason. Scan Requests has two modes: LeaderOnly(default) and CLOSEST_REPLICA.
For more accurate results, users will choose the LeaderOnly(default) mode.
Mostly, the scan load is positive correlation with leader numbers.
- The other reason. Write requests, leaders receive write requests and followers receive
UpdateConsensus request(raft called AppendEntries), the flow of processing is a little
different: the queue, pipeline, batch size, that may cause imbalanced load. Leader
rebalance will make leader and followers balanced and make service more stable.
An experiment present leader rebalance's effect (more details at KUDU-3390):
|leader ratio| scan cost | cpu usage | io
before|40: 0: 0 | 811.586 s | 47%, 18%, 19% | 102MB/s ioutil:55%, 8KB/s 2%, 64KB/s 3%
after |13: 14: 13 | 611.012 s | 39%, 45%, 35% | 53MB/s ioutil:31%, 80MB/s 18%, 45MB/s 24%
Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
---
M src/kudu/master/CMakeLists.txt
A src/kudu/master/auto_leader_rebalancer-test.cc
A src/kudu/master/auto_leader_rebalancer.cc
A src/kudu/master/auto_leader_rebalancer.h
M src/kudu/master/auto_rebalancer-test.cc
M src/kudu/master/auto_rebalancer.h
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
8 files changed, 836 insertions(+), 15 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18454/37
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 37
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.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: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
[kudu-CR] [master] KUDU-3061 support auto rebalance tablet leaders across TServers
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/18454 )
Change subject: [master] KUDU-3061 support auto rebalance tablet leaders across TServers
......................................................................
Patch Set 20:
> For a specific table, its replicas assigned to tservers(by create table or rebalance task), at a specific tserver should satisfy
> number of leaders : number of followers is 1 : (replica_refactor -1).
>
> every table should do leader rebalance.
(number of leaders) / (number of followers) == 1 / (replica_refactor - 1), is too rough IMO, there are some cases you should cover and mention:
1. the replica factor is 1.
2. the number of tserver is far greater than the number of total replicas.
And I suggest to make sure replica is balanced before rebalance replica leader.
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 20
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: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Fri, 08 Jul 2022 08:26:29 +0000
Gerrit-HasComments: No
[kudu-CR] [master] KUDU-3390 support auto rebalance tablet leaders across TServers
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Yingchun Lai, Yifan Zhang, Kudu Jenkins, Andrew Wong, Abhishek Chennaka,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18454
to look at the new patch set (#39).
Change subject: [master] KUDU-3390 support auto rebalance tablet leaders across TServers
......................................................................
[master] KUDU-3390 support auto rebalance tablet leaders across TServers
The number of leader replicas per tablet server can become imbalanced
over time, which lead to load skew on some nodes. This patch adds
auto leader rebalance task to avoid leader replicas skew.
Two reasons of load skew:
- The main reason. Scan Requests has two modes: LeaderOnly(default) and CLOSEST_REPLICA.
For more accurate results, users will choose the LeaderOnly(default) mode.
Mostly, the scan load is positive correlation with leader numbers.
- The other reason. Write requests, leaders receive write requests and followers receive
UpdateConsensus request(raft called AppendEntries), the flow of processing is a little
different: the queue, pipeline, batch size, that may cause imbalanced load. Leader
rebalance will make leader and followers balanced and make service more stable.
An experiment present leader rebalance's effect (more details at KUDU-3390):
|leader ratio| scan cost | cpu usage | io
before|40: 0: 0 | 811.586 s | 47%, 18%, 19% | 102MB/s ioutil:55%, 8KB/s 2%, 64KB/s 3%
after |13: 14: 13 | 611.012 s | 39%, 45%, 35% | 53MB/s ioutil:31%, 80MB/s 18%, 45MB/s 24%
Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
---
M src/kudu/master/CMakeLists.txt
A src/kudu/master/auto_leader_rebalancer-test.cc
A src/kudu/master/auto_leader_rebalancer.cc
A src/kudu/master/auto_leader_rebalancer.h
M src/kudu/master/auto_rebalancer-test.cc
M src/kudu/master/auto_rebalancer.h
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
8 files changed, 836 insertions(+), 15 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18454/39
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 39
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.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: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
[kudu-CR] [master] KUDU-3390 support auto rebalance tablet leaders across TServers
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Yuqi Du has posted comments on this change. ( http://gerrit.cloudera.org:8080/18454 )
Change subject: [master] KUDU-3390 support auto rebalance tablet leaders across TServers
......................................................................
Patch Set 40:
(2 comments)
Thank you for your advices.
http://gerrit.cloudera.org:8080/#/c/18454/40/src/kudu/master/auto_leader_rebalancer-test.cc
File src/kudu/master/auto_leader_rebalancer-test.cc:
http://gerrit.cloudera.org:8080/#/c/18454/40/src/kudu/master/auto_leader_rebalancer-test.cc@174
PS40, Line 174: TEST_F(LeaderRebalancerTest, RestartTserver) {
> From what I can see, the following isn't covered by tests: the case when ma
Some test cases, I have put them at 'auto_rebalancer-test.cc'.
The case what you point out is in 'AutoRebalancerTest.NextLeaderResumesAutoRebalancing'
http://gerrit.cloudera.org:8080/#/c/18454/40/src/kudu/master/auto_leader_rebalancer.cc
File src/kudu/master/auto_leader_rebalancer.cc:
http://gerrit.cloudera.org:8080/#/c/18454/40/src/kudu/master/auto_leader_rebalancer.cc@349
PS40, Line 349: MutexLock auto_lock(running_mutex_);
> What is this for? RunLeaderRebalancer() is called from a single thread, so
I am sorry, the lock is not necessary at this patch.
One of my plans, it should be used next patch, at that patch another thread would run 'RunLeaderRebalancer()'.
Sorry for that.
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 40
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.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: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Thu, 17 Nov 2022 09:50:50 +0000
Gerrit-HasComments: Yes
[kudu-CR] [master] KUDU-3390 support auto rebalance tablet leaders across TServers
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Yingchun Lai, Yifan Zhang, Kudu Jenkins, Andrew Wong, Abhishek Chennaka,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18454
to look at the new patch set (#35).
Change subject: [master] KUDU-3390 support auto rebalance tablet leaders across TServers
......................................................................
[master] KUDU-3390 support auto rebalance tablet leaders across TServers
The number of leader replicas per tablet server can become imbalanced
over time, which lead to load skew on some nodes. This patch adds
auto leader rebalance task to avoid leader replicas skew.
Two reasons of load skew:
- The main reason. Scan Requests has two modes: LeaderOnly(default) and CLOSEST_REPLICA.
For more accurate results, users will choose the LeaderOnly(default) mode.
Mostly, the scan load is positive correlation with leader numbers.
- The other reason. Write requests, leaders receive write requests and followers receive
UpdateConsensus request(raft called AppendEntries), the flow of processing is a little
different: the queue, pipeline, batch size, that may cause imbalanced load. Leader
rebalance will make leader and followers balanced and make service more stable.
An experiment present leader rebalance's effect (more details at KUDU-3390):
|leader ratio| scan cost | cpu usage | io
before|40: 0: 0 | 811.586 s | 47%, 18%, 19% | 102MB/s ioutil:55%, 8KB/s 2%, 64KB/s 3%
after |13: 14: 13 | 611.012 s | 39%, 45%, 35% | 53MB/s ioutil:31%, 80MB/s 18%, 45MB/s 24%
Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
---
M src/kudu/master/CMakeLists.txt
A src/kudu/master/auto_leader_rebalancer-test.cc
A src/kudu/master/auto_leader_rebalancer.cc
A src/kudu/master/auto_leader_rebalancer.h
M src/kudu/master/auto_rebalancer-test.cc
M src/kudu/master/auto_rebalancer.h
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
8 files changed, 828 insertions(+), 15 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18454/35
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 35
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.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: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
[kudu-CR] [master] KUDU-3061 support auto rebalance tablet leaders across TServers
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/18454 )
Change subject: [master] KUDU-3061 support auto rebalance tablet leaders across TServers
......................................................................
Patch Set 13:
(20 comments)
http://gerrit.cloudera.org:8080/#/c/18454/13/src/kudu/master/auto_leader_rebalancer.cc
File src/kudu/master/auto_leader_rebalancer.cc:
http://gerrit.cloudera.org:8080/#/c/18454/13/src/kudu/master/auto_leader_rebalancer.cc@92
PS13, Line 92: DCHECK(!thread_) << "AutoleaderRebalancerTask is already initialized";
thread_ is not initialized after calling Init() when FLAGS_auto_leader_rebalancing_enabled is false, it's a bit of strange that an object can be 'init' multiple times.
http://gerrit.cloudera.org:8080/#/c/18454/13/src/kudu/master/auto_leader_rebalancer.cc@93
PS13, Line 93: RETURN_NOT_OK(MessengerBuilder("auto-leader-rebalancer").Build(&messenger_));
Is it needed to initialize messenger_ when FLAGS_auto_leader_rebalancing_enabled is false?
http://gerrit.cloudera.org:8080/#/c/18454/13/src/kudu/master/auto_leader_rebalancer.cc@116
PS13, Line 116: Status AutoLeaderRebalancerTask::LeaderRebalance(const scoped_refptr<TableInfo>& table_info,
How about rename to RunLeaderRebalanceForTable, or something like that?
http://gerrit.cloudera.org:8080/#/c/18454/13/src/kudu/master/auto_leader_rebalancer.cc@117
PS13, Line 117: std::
nit: same
http://gerrit.cloudera.org:8080/#/c/18454/13/src/kudu/master/auto_leader_rebalancer.cc@117
PS13, Line 117: std::
nit: can be omitted
http://gerrit.cloudera.org:8080/#/c/18454/13/src/kudu/master/auto_leader_rebalancer.cc@136
PS13, Line 136: kudu::
nit: can be omitted
http://gerrit.cloudera.org:8080/#/c/18454/13/src/kudu/master/auto_leader_rebalancer.cc@165
PS13, Line 165: if (ContainsKey(uuid_leaders_map, uuid)) {
: uuid_leaders_map[uuid].emplace_back(tablet->id());
: } else {
: uuid_leaders_map.insert({uuid, vector<string>({tablet->id()})});
: }
can be simplified by:
auto& uuid_leaders = LookupOrInsert(&uuid_leaders_map, uuid, {});
uuid_leaders.emplace_back(tablet->id());
http://gerrit.cloudera.org:8080/#/c/18454/13/src/kudu/master/auto_leader_rebalancer.cc@174
PS13, Line 174: if (ContainsKey(follower_uuid_map, tablet->id())) {
: follower_uuid_map[tablet->id()].emplace_back(uuid);
: } else {
: follower_uuid_map.insert({tablet->id(), vector<string>({uuid})});
: }
also can be simplified as above.
http://gerrit.cloudera.org:8080/#/c/18454/13/src/kudu/master/auto_leader_rebalancer.cc@184
PS13, Line 184: auto it = uuid_replicas_map.find(ts_info.permanent_uuid());
: if (ContainsKey(uuid_replicas_map, ts_info.permanent_uuid())) {
: it->second.emplace_back(tablet->id());
: } else {
: uuid_replicas_map.insert(
: {ts_info.permanent_uuid(), std::vector<string>({tablet->id()})});
: }
same
http://gerrit.cloudera.org:8080/#/c/18454/13/src/kudu/master/auto_leader_rebalancer.cc@203
PS13, Line 203: auto it1 = uuid_leaders_map.find(uuid);
: if (it1 != uuid_leaders_map.end()) {
: leader_count = it1->second.size();
: } else {
: leader_count = 0;
: }
FindWithDefault() in map-util.h may help.
http://gerrit.cloudera.org:8080/#/c/18454/13/src/kudu/master/auto_leader_rebalancer.cc@209
PS13, Line 209: auto it2 = uuid_replicas_map.find(uuid);
: if (it2 != uuid_replicas_map.end()) {
: replica_count = it2->second.size();
: } else {
: // no replica, skip it
: continue;
: }
use FindOrNull?
http://gerrit.cloudera.org:8080/#/c/18454/13/src/kudu/master/auto_leader_rebalancer.cc@218
PS13, Line 218: if (mode == AutoLeaderRebalancerTask::ExecuteMode::TEST) {
I think use VLOG here wouble be more reasonable than depends on mode.
http://gerrit.cloudera.org:8080/#/c/18454/13/src/kudu/master/auto_leader_rebalancer.cc@234
PS13, Line 234: std::
nit: can be omitted.
http://gerrit.cloudera.org:8080/#/c/18454/13/src/kudu/master/auto_leader_rebalancer.cc@247
PS13, Line 247: std::
omit
http://gerrit.cloudera.org:8080/#/c/18454/13/src/kudu/master/auto_leader_rebalancer.cc@359
PS13, Line 359: number_of_loop_iterations_for_test_
How about add a metric for it?
http://gerrit.cloudera.org:8080/#/c/18454/13/src/kudu/master/auto_leader_rebalancer.cc@366
PS13, Line 366: set<string> tserver_uuid_set;
why use std::set? aren't the tserver uuids unique?
http://gerrit.cloudera.org:8080/#/c/18454/13/src/kudu/master/auto_leader_rebalancer.cc@390
PS13, Line 390: , Substitute("RunLeaderRebalancer not ok")
this log is helpless
http://gerrit.cloudera.org:8080/#/c/18454/12/src/kudu/master/auto_rebalancer.cc
File src/kudu/master/auto_rebalancer.cc:
http://gerrit.cloudera.org:8080/#/c/18454/12/src/kudu/master/auto_rebalancer.cc@174
PS12, Line 174: if (FLAGS_auto_rebalancing_enabled) {
> I want to support to execute RunReplicaRebalancer() by external tool cmd la
You can do it in the next patch, it not needed to modify it now, and you can enable it in unit test manually.
http://gerrit.cloudera.org:8080/#/c/18454/13/src/kudu/master/catalog_manager.h
File src/kudu/master/catalog_manager.h:
http://gerrit.cloudera.org:8080/#/c/18454/13/src/kudu/master/catalog_manager.h@771
PS13, Line 771: > *table
nit: >* table
http://gerrit.cloudera.org:8080/#/c/18454/13/src/kudu/master/catalog_manager.h@776
PS13, Line 776: > *table
nit: >* table
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 13
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: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Sun, 19 Jun 2022 09:04:11 +0000
Gerrit-HasComments: Yes
[kudu-CR] [master] KUDU-3390 support auto rebalance tablet leaders across TServers
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/18454 )
Change subject: [master] KUDU-3390 support auto rebalance tablet leaders across TServers
......................................................................
Patch Set 37: Code-Review+1
(2 comments)
http://gerrit.cloudera.org:8080/#/c/18454/35/src/kudu/master/auto_leader_rebalancer.cc
File src/kudu/master/auto_leader_rebalancer.cc:
http://gerrit.cloudera.org:8080/#/c/18454/35/src/kudu/master/auto_leader_rebalancer.cc@167
PS35, Line 167: vector<string>()
> 'auto& leader_uuids' is a left reference , it cannot be '{}'
I meant the last parameter, it's similar to line 173.
http://gerrit.cloudera.org:8080/#/c/18454/35/src/kudu/master/auto_leader_rebalancer.cc@295
PS35, Line 295:
> ??
That's OK, I see the same judgement in line 273.
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 37
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.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: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Sun, 13 Nov 2022 15:32:16 +0000
Gerrit-HasComments: Yes
[kudu-CR] [master] KUDU-3390 support auto rebalance tablet leaders across TServers
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Yingchun Lai, Yifan Zhang, Kudu Jenkins, Andrew Wong, Abhishek Chennaka,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18454
to look at the new patch set (#30).
Change subject: [master] KUDU-3390 support auto rebalance tablet leaders across TServers
......................................................................
[master] KUDU-3390 support auto rebalance tablet leaders across TServers
The number of leader replicas per tablet server can become imbalanced
over time, which lead to load skew on some nodes. This patch adding and
auto leader rebalance task to avoid leader replicas skew.
Two reasons of load skew:
- The main reason. Scan Requests has two modes: LeaderOnly(default) and CLOSEST_REPLICA.
For more accurate results, users will choose the LeaderOnly(default) mode.
Mostly, the scan load is positive correlation with leader numbers.
- The other reason. Write requests, leaders receive write requests and followers receive
UpdateConsensus request(raft called AppendEntries), the flow of processing is a little
different, which is hidden variables, maybe cause imbalanced load. Leader rebalance will
make leader and followers balanced and eliminate hidden variables and make service more stable.
An experiment present leader rebalance's effect(more detail infomations at KUDU-3390):
|leader ratio| scan cost | cpu usage | io
before|40: 0: 0 | 811.586 s | 47%, 18%, 19% | 102MB/s ioutil:55%, 8KB/s 2%, 64KB/s 3%
after |13: 14: 13 | 611.012 s | 39%, 45%, 35% | 53MB/s ioutil:31%, 80MB/s 18%, 45MB/s 24%
Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
---
M src/kudu/master/CMakeLists.txt
A src/kudu/master/auto_leader_rebalancer-test.cc
A src/kudu/master/auto_leader_rebalancer.cc
A src/kudu/master/auto_leader_rebalancer.h
M src/kudu/master/auto_rebalancer-test.cc
M src/kudu/master/auto_rebalancer.h
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
8 files changed, 805 insertions(+), 15 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18454/30
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 30
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.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: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
[kudu-CR] [master] KUDU-3390 support auto rebalance tablet leaders across TServers
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Yingchun Lai, Yifan Zhang, Kudu Jenkins, Andrew Wong, Abhishek Chennaka,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18454
to look at the new patch set (#31).
Change subject: [master] KUDU-3390 support auto rebalance tablet leaders across TServers
......................................................................
[master] KUDU-3390 support auto rebalance tablet leaders across TServers
The number of leader replicas per tablet server can become imbalanced
over time, which lead to load skew on some nodes. This patch adding and
auto leader rebalance task to avoid leader replicas skew.
Two reasons of load skew:
- The main reason. Scan Requests has two modes: LeaderOnly(default) and CLOSEST_REPLICA.
For more accurate results, users will choose the LeaderOnly(default) mode.
Mostly, the scan load is positive correlation with leader numbers.
- The other reason. Write requests, leaders receive write requests and followers receive
UpdateConsensus request(raft called AppendEntries), the flow of processing is a little
different: the queue, pipeline, batch size, that may cause imbalanced load. Leader
rebalance will make leader and followers balanced and make service more stable.
An experiment present leader rebalance's effect(more detail infomations at KUDU-3390):
|leader ratio| scan cost | cpu usage | io
before|40: 0: 0 | 811.586 s | 47%, 18%, 19% | 102MB/s ioutil:55%, 8KB/s 2%, 64KB/s 3%
after |13: 14: 13 | 611.012 s | 39%, 45%, 35% | 53MB/s ioutil:31%, 80MB/s 18%, 45MB/s 24%
Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
---
M src/kudu/master/CMakeLists.txt
A src/kudu/master/auto_leader_rebalancer-test.cc
A src/kudu/master/auto_leader_rebalancer.cc
A src/kudu/master/auto_leader_rebalancer.h
M src/kudu/master/auto_rebalancer-test.cc
M src/kudu/master/auto_rebalancer.h
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
8 files changed, 834 insertions(+), 15 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18454/31
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 31
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.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: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
[kudu-CR] [master] KUDU-3061 support auto rebalance tablet leaders across TServers
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/18454
to look at the new patch set (#3).
Change subject: [master] KUDU-3061 support auto rebalance tablet leaders across TServers
......................................................................
[master] KUDU-3061 support auto rebalance tablet leaders across TServers
The number of leader tablets per tablet server can become imbalanced
over time, putting additional pressure on a few nodes.
A CLI tool or an extension to the existing balancer should be added to
take care of this.
Currently the only option is running leader_step_down manually.
TODO:
1. adding unit test, use TimerThread KUDU-3364 to refact codes.
2. Add a leader rebalance CLI tool.
Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
---
M src/kudu/master/CMakeLists.txt
A src/kudu/master/auto_leader_rebalancer-test.cc
A src/kudu/master/auto_leader_rebalancer.cc
A src/kudu/master/auto_leader_rebalancer.h
M src/kudu/master/auto_rebalancer-test.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
9 files changed, 874 insertions(+), 30 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18454/3
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 3
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] [master] KUDU-3390 support auto rebalance tablet leaders across TServers
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/18454 )
Change subject: [master] KUDU-3390 support auto rebalance tablet leaders across TServers
......................................................................
Patch Set 38: Code-Review+2
Thanks for the contribution.
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 38
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.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: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Mon, 14 Nov 2022 06:58:07 +0000
Gerrit-HasComments: No
[kudu-CR] [master] KUDU-3390 support auto rebalance tablet leaders across TServers
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18454 )
Change subject: [master] KUDU-3390 support auto rebalance tablet leaders across TServers
......................................................................
Patch Set 40:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/18454/40/src/kudu/master/auto_leader_rebalancer-test.cc
File src/kudu/master/auto_leader_rebalancer-test.cc:
http://gerrit.cloudera.org:8080/#/c/18454/40/src/kudu/master/auto_leader_rebalancer-test.cc@174
PS40, Line 174: TEST_F(LeaderRebalancerTest, RestartTserver) {
> Some test cases, I have put them at 'auto_rebalancer-test.cc'.
Ah, I see. Thank you for the clarification.
http://gerrit.cloudera.org:8080/#/c/18454/40/src/kudu/master/auto_leader_rebalancer.cc
File src/kudu/master/auto_leader_rebalancer.cc:
http://gerrit.cloudera.org:8080/#/c/18454/40/src/kudu/master/auto_leader_rebalancer.cc@349
PS40, Line 349: MutexLock auto_lock(running_mutex_);
> I am sorry, the lock is not necessary at this patch.
Ah, no problem!
Probably, I missed a discussion thread on this topic in earlier comments.
If that's going to be used or somehow addressed in later patches, that's fine.
Thank you for the contribution!
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 40
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.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: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Thu, 17 Nov 2022 16:18:58 +0000
Gerrit-HasComments: Yes
[kudu-CR] [master] KUDU-3061 support auto rebalance tablet leaders across TServers
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18454 )
Change subject: [master] KUDU-3061 support auto rebalance tablet leaders across TServers
......................................................................
Patch Set 21:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18454/19//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18454/19//COMMIT_MSG@21
PS19, Line 21: maybe cause imbalanced load
> It's very hard to explain the number's difference of UpdateConsensus and wr
Yes, I understand it's not easy to setup a proper measurement experiment and collect the required data. But maybe you could run the newly implemented leader rebalancing on your test/development/production cluster, collecting memory acnd CPU usage statistics while running the same workload before and after. That would be some data to take into consideration.
I think leader rebalancing makes sense. I'm just trying to get better understanding how much is the imbalance of leader vs non-leader tablet replica we are talking about.
Thank you very much for working on this!
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 21
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: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Tue, 12 Jul 2022 18:25:34 +0000
Gerrit-HasComments: Yes
[kudu-CR] [master] KUDU-3061 support auto rebalance tablet leaders across TServers
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Yingchun Lai, Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18454
to look at the new patch set (#13).
Change subject: [master] KUDU-3061 support auto rebalance tablet leaders across TServers
......................................................................
[master] KUDU-3061 support auto rebalance tablet leaders across TServers
The number of leader replicas per tablet server can become imbalanced
over time, which lead to load skew on some nodes.
Adding an auto leader rebalance task to avoid leader replicas skew.
Two reasons of load skew:
1. The main reason. Scan Requests has two modes: LeaderOnly(default) and CLOSEST_REPLICA.
For more accurate results, users will choose the LeaderOnly(default) mode.
Mostly, the scan load is positive correlation with leader numbers.
2. The other reason. Write requests, leaders receive write requests and followers receive
appendEntries(kudu is UpdateConsensus), the flow of processing is a little different, which
is hidden variables, maybe cause imbalanced load. Leader rebalance will make leader and
followers balanced and eliminate hidden variables and make service more stable.
Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
---
M src/kudu/client/flex_partitioning_client-test.cc
M src/kudu/master/CMakeLists.txt
A src/kudu/master/auto_leader_rebalancer-test.cc
A src/kudu/master/auto_leader_rebalancer.cc
A src/kudu/master/auto_leader_rebalancer.h
M src/kudu/master/auto_rebalancer-test.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_path_handlers.cc
11 files changed, 861 insertions(+), 42 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18454/13
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 13
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: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
[kudu-CR] [master] KUDU-3061 support auto rebalance tablet leaders across TServers
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Yingchun Lai, Yifan Zhang, Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18454
to look at the new patch set (#22).
Change subject: [master] KUDU-3061 support auto rebalance tablet leaders across TServers
......................................................................
[master] KUDU-3061 support auto rebalance tablet leaders across TServers
The number of leader replicas per tablet server can become imbalanced
over time, which lead to load skew on some nodes. This patch adding and
auto leader rebalance task to avoid leader replicas skew.
Two reasons of load skew:
- The main reason. Scan Requests has two modes: LeaderOnly(default) and CLOSEST_REPLICA.
For more accurate results, users will choose the LeaderOnly(default) mode.
Mostly, the scan load is positive correlation with leader numbers.
- The other reason. Write requests, leaders receive write requests and followers receive
appendEntries(kudu is UpdateConsensus), the flow of processing is a little different, which
is hidden variables, maybe cause imbalanced load. Leader rebalance will make leader and
followers balanced and eliminate hidden variables and make service more stable.
Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
---
M src/kudu/master/CMakeLists.txt
A src/kudu/master/auto_leader_rebalancer-test.cc
A src/kudu/master/auto_leader_rebalancer.cc
A src/kudu/master/auto_leader_rebalancer.h
M src/kudu/master/auto_rebalancer-test.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
9 files changed, 794 insertions(+), 14 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18454/22
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 22
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: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
[kudu-CR] [master] KUDU-3061 support auto rebalance tablet leaders across TServers
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18454 )
Change subject: [master] KUDU-3061 support auto rebalance tablet leaders across TServers
......................................................................
Patch Set 10:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18454/10//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18454/10//COMMIT_MSG@10
PS10, Line 10: to serve pressure skew on some nodes
I'm not sure I can parse this. Could you rephrase or clarify on this?
Overall, before starting this work, could you please present some numbers showing how much extra load a leader replica has compared with non-leader one? KUDU-3061 claims that there might be some imbalance due to having many leader replicas per tablet server, but it doesn't provide any proof that it's indeed consumes much more resources or induce extra load, and if so, how much.
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 10
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: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Fri, 20 May 2022 16:20:55 +0000
Gerrit-HasComments: Yes
[kudu-CR] [master] KUDU-3061 support auto rebalance tablet leaders across TServers
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Yingchun Lai, Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18454
to look at the new patch set (#14).
Change subject: [master] KUDU-3061 support auto rebalance tablet leaders across TServers
......................................................................
[master] KUDU-3061 support auto rebalance tablet leaders across TServers
The number of leader replicas per tablet server can become imbalanced
over time, which lead to load skew on some nodes.
Adding an auto leader rebalance task to avoid leader replicas skew.
Two reasons of load skew:
1. The main reason. Scan Requests has two modes: LeaderOnly(default) and CLOSEST_REPLICA.
For more accurate results, users will choose the LeaderOnly(default) mode.
Mostly, the scan load is positive correlation with leader numbers.
2. The other reason. Write requests, leaders receive write requests and followers receive
appendEntries(kudu is UpdateConsensus), the flow of processing is a little different, which
is hidden variables, maybe cause imbalanced load. Leader rebalance will make leader and
followers balanced and eliminate hidden variables and make service more stable.
Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
---
M src/kudu/client/flex_partitioning_client-test.cc
M src/kudu/master/CMakeLists.txt
A src/kudu/master/auto_leader_rebalancer-test.cc
A src/kudu/master/auto_leader_rebalancer.cc
A src/kudu/master/auto_leader_rebalancer.h
M src/kudu/master/auto_rebalancer-test.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_path_handlers.cc
11 files changed, 801 insertions(+), 33 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18454/14
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 14
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: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
[kudu-CR] [master] KUDU-3390 support auto rebalance tablet leaders across TServers
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Yuqi Du has posted comments on this change. ( http://gerrit.cloudera.org:8080/18454 )
Change subject: [master] KUDU-3390 support auto rebalance tablet leaders across TServers
......................................................................
Patch Set 30:
(19 comments)
Very thank you for the code review works.
I have fixed them and you can review it again.
http://gerrit.cloudera.org:8080/#/c/18454/19//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18454/19//COMMIT_MSG@10
PS19, Line 10: which lead to load skew on some nodes.
> Agree, I also saw many cases that the leader imbalanced and it lead to writ
Done
http://gerrit.cloudera.org:8080/#/c/18454/19//COMMIT_MSG@14
PS19, Line 14: - The main reason. Scan R
> It's not only the results of qualitative analysis, but also actual situatio
At jira KUDU-3390 and the messages, include the effect of leader rebalance.
http://gerrit.cloudera.org:8080/#/c/18454/19//COMMIT_MSG@20
PS19, Line 20: cause imbalanced load. Leader rebalance will
: make leader and fol
> Above.
Done
http://gerrit.cloudera.org:8080/#/c/18454/19//COMMIT_MSG@21
PS19, Line 21: wers balanced and eliminate
> OK, I have found a history issue and reply an above cr, but cpu usage and m
Done
http://gerrit.cloudera.org:8080/#/c/18454/29/src/kudu/master/auto_leader_rebalancer.h
File src/kudu/master/auto_leader_rebalancer.h:
http://gerrit.cloudera.org:8080/#/c/18454/29/src/kudu/master/auto_leader_rebalancer.h@59
PS29, Line 59: tus Init();
> nit: how about?
Have removed it
http://gerrit.cloudera.org:8080/#/c/18454/29/src/kudu/master/auto_leader_rebalancer.cc
File src/kudu/master/auto_leader_rebalancer.cc:
http://gerrit.cloudera.org:8080/#/c/18454/29/src/kudu/master/auto_leader_rebalancer.cc@68
PS29, Line 68: using strings::Substitute;
> Add some tags for these new added flags, consider ‘experimental’, 'unsafe',
Done
http://gerrit.cloudera.org:8080/#/c/18454/29/src/kudu/master/auto_leader_rebalancer.cc@107
PS29, Line 107: return;
> Logs in this function add the table name to distinguish different tables ar
Done
http://gerrit.cloudera.org:8080/#/c/18454/29/src/kudu/master/auto_leader_rebalancer.cc@114
PS29, Line 114: const scoped_refptr<TableInfo>& table_info,
> nit: DCHECK_GT(replication_factor, 0);
Done
http://gerrit.cloudera.org:8080/#/c/18454/29/src/kudu/master/auto_leader_rebalancer.cc@120
PS29, Line 120: replication_factor = table
> nit: tablet_id -> leader tserver uuid ?
yes. DONE
http://gerrit.cloudera.org:8080/#/c/18454/29/src/kudu/master/auto_leader_rebalancer.cc@121
PS29, Line 121: n_factor, 0);
> In Kudu's naming habit, the map offen named as 'value_by_key', for example
Done
http://gerrit.cloudera.org:8080/#/c/18454/29/src/kudu/master/auto_leader_rebalancer.cc@122
PS29, Line 122: (table_data.state() == SysTablesEntry
> nit: tablet_id -> follower tserver uuids ?
Done
http://gerrit.cloudera.org:8080/#/c/18454/29/src/kudu/master/auto_leader_rebalancer.cc@124
PS29, Line 124: eturn Status::OK();
:
> nit: tserver uuid -> leader replicas ?
Done
http://gerrit.cloudera.org:8080/#/c/18454/29/src/kudu/master/auto_leader_rebalancer.cc@126
PS29, Line 126:
> nit: tserver uuid -> all replicas ?
Done
http://gerrit.cloudera.org:8080/#/c/18454/29/src/kudu/master/auto_leader_rebalancer.cc@157
PS29, Line 157:
: // Build a summary for each replica of the t
> nit: InsertOrDie(&uuid_leaders_map, uuid, vector<string>(tablet->id())); ?
no. First Die is not correct.
My intention:
if uuid_leaders_map[uuid] exists, uuid_leaders_map[uuid].emplace_back(tablet->id())
if uuid_leaders_map[uuid] do not exists, uuid_leaders_map[uuid] = vector<string>({tablet->id()})
So it's may be repetitive. maybe need construct a vector object vector.
http://gerrit.cloudera.org:8080/#/c/18454/29/src/kudu/master/auto_leader_rebalancer.cc@159
PS29, Line 159: (const auto& r : locs_pb.interned_replicas()) {
: int index = r.ts_info_idx();
> nit: Recommand to use InsertOrDie
Done
http://gerrit.cloudera.org:8080/#/c/18454/29/src/kudu/master/auto_leader_rebalancer.cc@165
PS29, Line 165: leader_uuids.emplace_back(tablet->id());
> Maybe it's a warning or even fatal?
WARNING, Done
http://gerrit.cloudera.org:8080/#/c/18454/29/src/kudu/master/auto_leader_rebalancer.cc@181
PS29, Line 181: }
: }
:
: // step 2.
: //
> nit: FindWithDefault
Done
http://gerrit.cloudera.org:8080/#/c/18454/29/src/kudu/master/auto_leader_rebalancer.cc@190
PS29, Line 190: r (const auto& uuid : tserver_uuids) {
: const vector<string>& tablet_ids = FindWithDefault(tablet_ids_by_ts_uuid, uuid, {});
: uint32_t replica_count = tablet_ids.size();
: if (replica_count == 0) {
:
> nit: FindWithDefault
Done
http://gerrit.cloudera.org:8080/#/c/18454/29/src/kudu/master/auto_rebalancer.cc
File src/kudu/master/auto_rebalancer.cc:
http://gerrit.cloudera.org:8080/#/c/18454/29/src/kudu/master/auto_rebalancer.cc@179
PS29, Line 179: return Thread::Create("catalog manager", "auto-rebalancer",
> nit: revert it
Done
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 30
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.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: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Mon, 26 Sep 2022 03:26:33 +0000
Gerrit-HasComments: Yes
[kudu-CR] [master] KUDU-3061 support auto rebalance tablet leaders across TServers
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Yingchun Lai, Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18454
to look at the new patch set (#18).
Change subject: [master] KUDU-3061 support auto rebalance tablet leaders across TServers
......................................................................
[master] KUDU-3061 support auto rebalance tablet leaders across TServers
The number of leader replicas per tablet server can become imbalanced
over time, which lead to load skew on some nodes.
Adding an auto leader rebalance task to avoid leader replicas skew.
Two reasons of load skew:
1. The main reason. Scan Requests has two modes: LeaderOnly(default) and CLOSEST_REPLICA.
For more accurate results, users will choose the LeaderOnly(default) mode.
Mostly, the scan load is positive correlation with leader numbers.
2. The other reason. Write requests, leaders receive write requests and followers receive
appendEntries(kudu is UpdateConsensus), the flow of processing is a little different, which
is hidden variables, maybe cause imbalanced load. Leader rebalance will make leader and
followers balanced and eliminate hidden variables and make service more stable.
Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
---
M src/kudu/client/flex_partitioning_client-test.cc
M src/kudu/master/CMakeLists.txt
A src/kudu/master/auto_leader_rebalancer-test.cc
A src/kudu/master/auto_leader_rebalancer.cc
A src/kudu/master/auto_leader_rebalancer.h
M src/kudu/master/auto_rebalancer-test.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_path_handlers.cc
11 files changed, 792 insertions(+), 23 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18454/18
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 18
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: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
[kudu-CR] [master] KUDU-3061 support auto rebalance tablet leaders across TServers
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Yuqi Du has posted comments on this change. ( http://gerrit.cloudera.org:8080/18454 )
Change subject: [master] KUDU-3061 support auto rebalance tablet leaders across TServers
......................................................................
Patch Set 19:
(10 comments)
Thanks your crs. Very Kind.
> Patch Set 19:
>
> (1 comment)
>
> I didn't really look into the codes because I'd like to understand the design consideration and the use of algorithm first.
> Here are some questions I don't know much about:
> - Is this leader rebalance task only transfer the leadership between replicas of the same tablet? Or it also copy replicas from one server to another?
Only do leader transfer, do not copy replicas.
> - Do we have to rebalance replicas in a cluster before we run this leader rebalance task?
Yes, it's better enable rebalance replicas.
If enable leader rebalancer but disable auto rebalancer the algorithm work well but the effect is not good. The algorithm can be convergence, and the algorithm's target is every tserver' replicas, number of leader : number of follower is 1 : (replica_refactor -1).
> - Can we finally get a fully balanced cluster in all cases?
Yes, we should enable auto rebalancer and rebalancer leader rebalancer together.
>
> If you already have a design document about this issue, can you share it so wen can discuss about the details.
I'm sorry, I only write an informal document in chinese. The basic and core idea can be described as:
For a specific table, its replicas assigned to tservers(by create table or rebalance task), at a specific tserver should satisfy
number of leaders : number of followers is 1 : (replica_refactor -1).
every table should do leader rebalance.
http://gerrit.cloudera.org:8080/#/c/18454/19//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18454/19//COMMIT_MSG@14
PS19, Line 14: Two reasons of load skew:
> Besides clarify why we need to balance leader replicas across TServers, it
ok, I' ll do this at unit test.
http://gerrit.cloudera.org:8080/#/c/18454/19/src/kudu/client/flex_partitioning_client-test.cc
File src/kudu/client/flex_partitioning_client-test.cc:
http://gerrit.cloudera.org:8080/#/c/18454/19/src/kudu/client/flex_partitioning_client-test.cc@341
PS19, Line 341: // Using one-level hash bucketing { 3, "key" } as the custom hash schema
> Sorry, I didn't get the point. Is this related to this patch?
According to crs before, GetTableInfo() is changed. catalog_manager.h#771.
http://gerrit.cloudera.org:8080/#/c/18454/19/src/kudu/master/auto_leader_rebalancer.cc
File src/kudu/master/auto_leader_rebalancer.cc:
http://gerrit.cloudera.org:8080/#/c/18454/19/src/kudu/master/auto_leader_rebalancer.cc@68
PS19, Line 68: 10,
> nit: would it be more convenient if the gflag name and default value are in
Done
http://gerrit.cloudera.org:8080/#/c/18454/19/src/kudu/master/auto_leader_rebalancer.cc@72
PS19, Line 72: 3600,
> same
Done
http://gerrit.cloudera.org:8080/#/c/18454/19/src/kudu/master/auto_leader_rebalancer.cc@96
PS19, Line 96: "catalog-manager"
> nit: better to use the same category name, i.e. "catalog manager", as other
Done
http://gerrit.cloudera.org:8080/#/c/18454/19/src/kudu/master/auto_leader_rebalancer.cc@103
PS19, Line 103: if (thread_) {
> Is there any case thread_ is not initialized?
Done
http://gerrit.cloudera.org:8080/#/c/18454/19/src/kudu/master/auto_leader_rebalancer.cc@181
PS19, Line 181: const vector<string>& leaders = FindWithDefault(uuid_leaders_map, uuid, {});
: int32_t leader_count = leaders.size();
> nit: move these code closer to the place where it used, means after L188.
Done
http://gerrit.cloudera.org:8080/#/c/18454/19/src/kudu/master/auto_leader_rebalancer.cc@192
PS19, Line 192: int32_t should_transfer_count = leader_count - (replica_count / table_data.num_replicas() + 1);
> Could you please add some comments to describe the algorithm?
Done
http://gerrit.cloudera.org:8080/#/c/18454/19/src/kudu/master/auto_rebalancer.cc
File src/kudu/master/auto_rebalancer.cc:
http://gerrit.cloudera.org:8080/#/c/18454/19/src/kudu/master/auto_rebalancer.cc@a181
PS19, Line 181:
> why remove it?
Done
http://gerrit.cloudera.org:8080/#/c/18454/19/src/kudu/master/auto_rebalancer.cc@177
PS19, Line 177: [this]() { this->RunLoop(); }, &thread_);
:
> nit: alignment and remove blank line.
Done
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 19
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: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Wed, 06 Jul 2022 07:19:37 +0000
Gerrit-HasComments: Yes
[kudu-CR] [master] KUDU-3061 support auto rebalance tablet leaders across TServers
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Yuqi Du has posted comments on this change. ( http://gerrit.cloudera.org:8080/18454 )
Change subject: [master] KUDU-3061 support auto rebalance tablet leaders across TServers
......................................................................
Patch Set 13:
(13 comments)
Thanks your crs.
http://gerrit.cloudera.org:8080/#/c/18454/12/src/kudu/master/auto_leader_rebalancer.cc
File src/kudu/master/auto_leader_rebalancer.cc:
http://gerrit.cloudera.org:8080/#/c/18454/12/src/kudu/master/auto_leader_rebalancer.cc@71
PS12, Line 71: DEFINE_uint32(auto_leader_rebalancing_interval_seconds, 3600,
> nit: need improve description
Done
http://gerrit.cloudera.org:8080/#/c/18454/12/src/kudu/master/auto_leader_rebalancer.cc@136
PS12, Line 136: map<s
> nit: can be omit
Done
http://gerrit.cloudera.org:8080/#/c/18454/12/src/kudu/master/auto_leader_rebalancer.cc@147
PS12, Line 147: CatalogManager::TSInfosDict ts_infos_dict;
> Better to comment closer to the code.
Remote it. Done
http://gerrit.cloudera.org:8080/#/c/18454/12/src/kudu/master/auto_leader_rebalancer.cc@165
PS12, Line 165: if (ContainsKey(uuid_leaders_map, uuid)) {
: uuid_leaders_map[uuid].emplace_back(tablet->id());
: } else {
: uuid_leaders_map.insert({uuid, vector<string>({tablet->id()})});
: }
: l
> There maybe some useful function you can use, see src/kudu/gutil/map-util.h
Changed it, and I'll study it later.
http://gerrit.cloudera.org:8080/#/c/18454/12/src/kudu/master/auto_leader_rebalancer.cc@229
PS12, Line 229: }
> I think this log would be in VLOG(x) level.
Done
http://gerrit.cloudera.org:8080/#/c/18454/12/src/kudu/master/auto_leader_rebalancer.cc@337
PS12, Line 337: table_data.name(), task.first, leader_uuid, task.second.second);
> Also should be in VLOG(x)
Done
http://gerrit.cloudera.org:8080/#/c/18454/12/src/kudu/master/auto_leader_rebalancer.cc@368
PS12, Line 368: if (e->PresumedDead
> set<string>
Done
http://gerrit.cloudera.org:8080/#/c/18454/12/src/kudu/master/auto_leader_rebalancer.cc@392
PS12, Line 392: }
> You can use WARN_NOT_OK to simplify code.
Done
http://gerrit.cloudera.org:8080/#/c/18454/12/src/kudu/master/auto_rebalancer.cc
File src/kudu/master/auto_rebalancer.cc:
http://gerrit.cloudera.org:8080/#/c/18454/12/src/kudu/master/auto_rebalancer.cc@174
PS12, Line 174: if (FLAGS_auto_rebalancing_enabled) {
> Why still create AutoRebalancerTask when FLAGS_auto_rebalancing_enabled is
I want to support to execute RunReplicaRebalancer() by external tool cmd laster when FLAGS_auto_rebalancing_enabled is false.
And now, it was used by rebalancer unit test.
http://gerrit.cloudera.org:8080/#/c/18454/12/src/kudu/master/auto_rebalancer.cc@180
PS12, Line 180:
> nit: remove it
Done
http://gerrit.cloudera.org:8080/#/c/18454/12/src/kudu/master/auto_rebalancer.cc@264
PS12, Line 264:
> Is it necessary to judge whether should stop loop by an extra variable 'sho
You are right. Fixed.
http://gerrit.cloudera.org:8080/#/c/18454/12/src/kudu/master/auto_rebalancer.cc@267
PS12, Line 267: const ClusterLocalityInfo& locality,
> nit: remove this useless comment
Done
http://gerrit.cloudera.org:8080/#/c/18454/12/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:
http://gerrit.cloudera.org:8080/#/c/18454/12/src/kudu/master/catalog_manager.cc@3695
PS12, Line 3695: scoped_refptr<TableInfo> *table) {
> nit: alignment
Done
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 13
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: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Thu, 16 Jun 2022 09:50:00 +0000
Gerrit-HasComments: Yes
[kudu-CR] [master] KUDU-3061 support auto rebalance tablet leaders across TServers
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Yuqi Du has posted comments on this change. ( http://gerrit.cloudera.org:8080/18454 )
Change subject: [master] KUDU-3061 support auto rebalance tablet leaders across TServers
......................................................................
Patch Set 8:
PTAL
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 8
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: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Mon, 09 May 2022 02:18:17 +0000
Gerrit-HasComments: No
[kudu-CR] [master] KUDU-3390 support auto rebalance tablet leaders across TServers
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Yingchun Lai, Yifan Zhang, Kudu Jenkins, Andrew Wong, Abhishek Chennaka,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18454
to look at the new patch set (#26).
Change subject: [master] KUDU-3390 support auto rebalance tablet leaders across TServers
......................................................................
[master] KUDU-3390 support auto rebalance tablet leaders across TServers
The number of leader replicas per tablet server can become imbalanced
over time, which lead to load skew on some nodes. This patch adding and
auto leader rebalance task to avoid leader replicas skew.
Two reasons of load skew:
- The main reason. Scan Requests has two modes: LeaderOnly(default) and CLOSEST_REPLICA.
For more accurate results, users will choose the LeaderOnly(default) mode.
Mostly, the scan load is positive correlation with leader numbers.
- The other reason. Write requests, leaders receive write requests and followers receive
UpdateConsensus request(raft called AppendEntries), the flow of processing is a little
different, which is hidden variables, maybe cause imbalanced load. Leader rebalance will
make leader and followers balanced and eliminate hidden variables and make service more stable.
An experiment present leader rebalance's effect(more detail infomations at KUDU-3390):
|leader ratio| scan cost | cpu usage | io
before|40: 0: 0 | 811.586 s | 47%, 18%, 19% | 102MB/s ioutil:55%, 8KB/s 2%, 64KB/s 3%
after |13: 14: 13 | 611.012 s | 39%, 45%, 35% | 53MB/s ioutil:31%, 80MB/s 18%, 45MB/s 24%
Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
---
M src/kudu/master/CMakeLists.txt
A src/kudu/master/auto_leader_rebalancer-test.cc
A src/kudu/master/auto_leader_rebalancer.cc
A src/kudu/master/auto_leader_rebalancer.h
M src/kudu/master/auto_rebalancer-test.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
9 files changed, 798 insertions(+), 12 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18454/26
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 26
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.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: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
[kudu-CR] [master] KUDU-3390 support auto rebalance tablet leaders across TServers
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Yingchun Lai, Yifan Zhang, Kudu Jenkins, Andrew Wong, Abhishek Chennaka,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18454
to look at the new patch set (#27).
Change subject: [master] KUDU-3390 support auto rebalance tablet leaders across TServers
......................................................................
[master] KUDU-3390 support auto rebalance tablet leaders across TServers
The number of leader replicas per tablet server can become imbalanced
over time, which lead to load skew on some nodes. This patch adding and
auto leader rebalance task to avoid leader replicas skew.
Two reasons of load skew:
- The main reason. Scan Requests has two modes: LeaderOnly(default) and CLOSEST_REPLICA.
For more accurate results, users will choose the LeaderOnly(default) mode.
Mostly, the scan load is positive correlation with leader numbers.
- The other reason. Write requests, leaders receive write requests and followers receive
UpdateConsensus request(raft called AppendEntries), the flow of processing is a little
different, which is hidden variables, maybe cause imbalanced load. Leader rebalance will
make leader and followers balanced and eliminate hidden variables and make service more stable.
An experiment present leader rebalance's effect(more detail infomations at KUDU-3390):
|leader ratio| scan cost | cpu usage | io
before|40: 0: 0 | 811.586 s | 47%, 18%, 19% | 102MB/s ioutil:55%, 8KB/s 2%, 64KB/s 3%
after |13: 14: 13 | 611.012 s | 39%, 45%, 35% | 53MB/s ioutil:31%, 80MB/s 18%, 45MB/s 24%
Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
---
M src/kudu/master/CMakeLists.txt
A src/kudu/master/auto_leader_rebalancer-test.cc
A src/kudu/master/auto_leader_rebalancer.cc
A src/kudu/master/auto_leader_rebalancer.h
M src/kudu/master/auto_rebalancer-test.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
9 files changed, 803 insertions(+), 16 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18454/27
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 27
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.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: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
[kudu-CR] [master] KUDU-3061 support auto rebalance tablet leaders across TServers
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/18454
to look at the new patch set (#8).
Change subject: [master] KUDU-3061 support auto rebalance tablet leaders across TServers
......................................................................
[master] KUDU-3061 support auto rebalance tablet leaders across TServers
The number of leader tablets per tablet server can become imbalanced
over time, putting additional pressure on a few nodes.
A CLI tool or an extension to the existing balancer should be added to
take care of this.
Currently the only option is running leader_step_down manually.
TODO:
1. adding unit test, use TimerThread KUDU-3364 to refact codes.
2. Add a leader rebalance CLI tool.
Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
---
M src/kudu/master/CMakeLists.txt
A src/kudu/master/auto_leader_rebalancer-test.cc
A src/kudu/master/auto_leader_rebalancer.cc
A src/kudu/master/auto_leader_rebalancer.h
M src/kudu/master/auto_rebalancer-test.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
9 files changed, 863 insertions(+), 30 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18454/8
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 8
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] KUDU-3061 support auto rebalance tablet leaders across TServers
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Yingchun Lai, Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18454
to look at the new patch set (#9).
Change subject: [master] KUDU-3061 support auto rebalance tablet leaders across TServers
......................................................................
[master] KUDU-3061 support auto rebalance tablet leaders across TServers
The number of leader replicas per tablet server can become imbalanced
over time, which lead to serve pressure skew on some nodes.
Adding an auto leader rebalance task to avoid leader replicas skew.
Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
---
M src/kudu/master/CMakeLists.txt
A src/kudu/master/auto_leader_rebalancer-test.cc
A src/kudu/master/auto_leader_rebalancer.cc
A src/kudu/master/auto_leader_rebalancer.h
M src/kudu/master/auto_rebalancer-test.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
9 files changed, 871 insertions(+), 34 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18454/9
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 9
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: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
[kudu-CR] [master] KUDU-3061 support auto rebalance tablet leaders across TServers
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/18454 )
Change subject: [master] KUDU-3061 support auto rebalance tablet leaders across TServers
......................................................................
Patch Set 8:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/18454/8//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18454/8//COMMIT_MSG@9
PS8, Line 9: tablets
nit? tablet replicas
http://gerrit.cloudera.org:8080/#/c/18454/8/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:
http://gerrit.cloudera.org:8080/#/c/18454/8/src/kudu/master/catalog_manager.cc@3695
PS8, Line 3695: Status
Always return OK, can we return void? Same to GetTableInfo above.
http://gerrit.cloudera.org:8080/#/c/18454/8/src/kudu/master/catalog_manager.cc@3697
PS8, Line 3697: // leader_lock_.AssertAcquiredForReading();
Should AssertAcquired this lock?
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 8
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: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Thu, 19 May 2022 02:04:51 +0000
Gerrit-HasComments: Yes
[kudu-CR] [master] KUDU-3061 support auto rebalance tablet leaders across TServers
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Yingchun Lai, Yifan Zhang, Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18454
to look at the new patch set (#20).
Change subject: [master] KUDU-3061 support auto rebalance tablet leaders across TServers
......................................................................
[master] KUDU-3061 support auto rebalance tablet leaders across TServers
The number of leader replicas per tablet server can become imbalanced
over time, which lead to load skew on some nodes.
Adding an auto leader rebalance task to avoid leader replicas skew.
Two reasons of load skew:
1. The main reason. Scan Requests has two modes: LeaderOnly(default) and CLOSEST_REPLICA.
For more accurate results, users will choose the LeaderOnly(default) mode.
Mostly, the scan load is positive correlation with leader numbers.
2. The other reason. Write requests, leaders receive write requests and followers receive
appendEntries(kudu is UpdateConsensus), the flow of processing is a little different, which
is hidden variables, maybe cause imbalanced load. Leader rebalance will make leader and
followers balanced and eliminate hidden variables and make service more stable.
Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
---
M src/kudu/client/flex_partitioning_client-test.cc
M src/kudu/master/CMakeLists.txt
A src/kudu/master/auto_leader_rebalancer-test.cc
A src/kudu/master/auto_leader_rebalancer.cc
A src/kudu/master/auto_leader_rebalancer.h
M src/kudu/master/auto_rebalancer-test.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_path_handlers.cc
11 files changed, 792 insertions(+), 21 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18454/20
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 20
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: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
[kudu-CR] [master] KUDU-3061 support auto rebalance tablet leaders across TServers
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Yingchun Lai, Yifan Zhang, Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18454
to look at the new patch set (#23).
Change subject: [master] KUDU-3061 support auto rebalance tablet leaders across TServers
......................................................................
[master] KUDU-3061 support auto rebalance tablet leaders across TServers
The number of leader replicas per tablet server can become imbalanced
over time, which lead to load skew on some nodes. This patch adding and
auto leader rebalance task to avoid leader replicas skew.
Two reasons of load skew:
- The main reason. Scan Requests has two modes: LeaderOnly(default) and CLOSEST_REPLICA.
For more accurate results, users will choose the LeaderOnly(default) mode.
Mostly, the scan load is positive correlation with leader numbers.
- The other reason. Write requests, leaders receive write requests and followers receive
appendEntries(kudu is UpdateConsensus), the flow of processing is a little different, which
is hidden variables, maybe cause imbalanced load. Leader rebalance will make leader and
followers balanced and eliminate hidden variables and make service more stable.
Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
---
M src/kudu/master/CMakeLists.txt
A src/kudu/master/auto_leader_rebalancer-test.cc
A src/kudu/master/auto_leader_rebalancer.cc
A src/kudu/master/auto_leader_rebalancer.h
M src/kudu/master/auto_rebalancer-test.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
9 files changed, 798 insertions(+), 18 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18454/23
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 23
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: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
[kudu-CR] [master] KUDU-3061 support auto rebalance tablet leaders across TServers
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/18454
to look at the new patch set (#6).
Change subject: [master] KUDU-3061 support auto rebalance tablet leaders across TServers
......................................................................
[master] KUDU-3061 support auto rebalance tablet leaders across TServers
The number of leader tablets per tablet server can become imbalanced
over time, putting additional pressure on a few nodes.
A CLI tool or an extension to the existing balancer should be added to
take care of this.
Currently the only option is running leader_step_down manually.
TODO:
1. adding unit test, use TimerThread KUDU-3364 to refact codes.
2. Add a leader rebalance CLI tool.
Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
---
M src/kudu/master/CMakeLists.txt
A src/kudu/master/auto_leader_rebalancer-test.cc
A src/kudu/master/auto_leader_rebalancer.cc
A src/kudu/master/auto_leader_rebalancer.h
M src/kudu/master/auto_rebalancer-test.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
9 files changed, 863 insertions(+), 30 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18454/6
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
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] [master] KUDU-3390 support auto rebalance tablet leaders across TServers
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Yingchun Lai, Yifan Zhang, Kudu Jenkins, Andrew Wong, Abhishek Chennaka,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18454
to look at the new patch set (#38).
Change subject: [master] KUDU-3390 support auto rebalance tablet leaders across TServers
......................................................................
[master] KUDU-3390 support auto rebalance tablet leaders across TServers
The number of leader replicas per tablet server can become imbalanced
over time, which lead to load skew on some nodes. This patch adds
auto leader rebalance task to avoid leader replicas skew.
Two reasons of load skew:
- The main reason. Scan Requests has two modes: LeaderOnly(default) and CLOSEST_REPLICA.
For more accurate results, users will choose the LeaderOnly(default) mode.
Mostly, the scan load is positive correlation with leader numbers.
- The other reason. Write requests, leaders receive write requests and followers receive
UpdateConsensus request(raft called AppendEntries), the flow of processing is a little
different: the queue, pipeline, batch size, that may cause imbalanced load. Leader
rebalance will make leader and followers balanced and make service more stable.
An experiment present leader rebalance's effect (more details at KUDU-3390):
|leader ratio| scan cost | cpu usage | io
before|40: 0: 0 | 811.586 s | 47%, 18%, 19% | 102MB/s ioutil:55%, 8KB/s 2%, 64KB/s 3%
after |13: 14: 13 | 611.012 s | 39%, 45%, 35% | 53MB/s ioutil:31%, 80MB/s 18%, 45MB/s 24%
Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
---
M src/kudu/master/CMakeLists.txt
A src/kudu/master/auto_leader_rebalancer-test.cc
A src/kudu/master/auto_leader_rebalancer.cc
A src/kudu/master/auto_leader_rebalancer.h
M src/kudu/master/auto_rebalancer-test.cc
M src/kudu/master/auto_rebalancer.h
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
8 files changed, 836 insertions(+), 15 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18454/38
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 38
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.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: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
[kudu-CR] [master] KUDU-3390 support auto rebalance tablet leaders across TServers
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18454 )
Change subject: [master] KUDU-3390 support auto rebalance tablet leaders across TServers
......................................................................
Patch Set 32:
(35 comments)
Thank you for the patch!
I took a quick first look.
http://gerrit.cloudera.org:8080/#/c/18454/29//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18454/29//COMMIT_MSG@23
PS29, Line 23: detail infomations
detailed information
http://gerrit.cloudera.org:8080/#/c/18454/32//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18454/32//COMMIT_MSG@7
PS32, Line 7: auto rebalance tablet leaders
I'm curious why this has started with that background task dong the rebalancing. Historically, the current replica rebalancing has appeared first as a CLI tool since it was easier to see it in action, and only after some time that functionality appeared as a background task run by the catalog manager.
So, a couple of questions here just out of curiousity:
1) Do you plan to add a kudu CLI tool that performs leader rebalancing?
2) If the answer for the first question is 'yes', why did you prefer to start with the auto-rebalancing task first?
Thanks!
http://gerrit.cloudera.org:8080/#/c/18454/32//COMMIT_MSG@10
PS32, Line 10: and
Drop 'and'
http://gerrit.cloudera.org:8080/#/c/18454/32//COMMIT_MSG@10
PS32, Line 10: adding
adds
http://gerrit.cloudera.org:8080/#/c/18454/32//COMMIT_MSG@20
PS32, Line 20: may cause imbalanced load
It would be nice to quantify this statement, adding measurement similar to those you posted for the scan case.
http://gerrit.cloudera.org:8080/#/c/18454/32//COMMIT_MSG@23
PS32, Line 23: (
nit: add a space before this opening parenthesis
http://gerrit.cloudera.org:8080/#/c/18454/32//COMMIT_MSG@23
PS32, Line 23: detail infomations
details
http://gerrit.cloudera.org:8080/#/c/18454/32/src/kudu/master/auto_leader_rebalancer-test.cc
File src/kudu/master/auto_leader_rebalancer-test.cc:
http://gerrit.cloudera.org:8080/#/c/18454/32/src/kudu/master/auto_leader_rebalancer-test.cc@60
PS32, Line 60: consensus_inject_latency_ms_in_notifications
Is this needed here?
http://gerrit.cloudera.org:8080/#/c/18454/32/src/kudu/master/auto_leader_rebalancer-test.cc@62
PS32, Line 62: raft_heartbeat_interval_ms
And the same for several other declarations of gflags in this section: are these needed?
http://gerrit.cloudera.org:8080/#/c/18454/32/src/kudu/master/auto_leader_rebalancer-test.cc@70
PS32, Line 70: METRIC_DECLARE_gauge_int32(tablet_copy_open_client_sessions);
: METRIC_DECLARE_counter(tablet_copy_bytes_fetched);
: METRIC_DECLARE_counter(tablet_copy_bytes_sent);
Are these needed in this file?
http://gerrit.cloudera.org:8080/#/c/18454/32/src/kudu/master/auto_leader_rebalancer-test.cc@139
PS32, Line 139: and leader balanced
How do we know that the distribution of the leader replicas is balanced? I didn't see anything in the scenario below that guarantees that, but maybe I'm missing something.
http://gerrit.cloudera.org:8080/#/c/18454/32/src/kudu/master/auto_leader_rebalancer-test.cc@150
PS32, Line 150: CreateWorkloadTable(kNumTablets, /*num_replicas*/ 3);
How do we know at this point that the cluster is leader-balanced?
http://gerrit.cloudera.org:8080/#/c/18454/32/src/kudu/master/auto_leader_rebalancer-test.cc@168
PS32, Line 168: SleepFor(MonoDelta::FromSeconds(20));
That's a really long sleep time: that's not a good idea to have something like this in the test because it increases the run-time of the whole test suite. Is it possible to use some custom setting for the parameters of the auto-rebalancing task to shorten this?
http://gerrit.cloudera.org:8080/#/c/18454/32/src/kudu/master/auto_leader_rebalancer-test.cc@169
PS32, Line 169: 50
I'm curious why is the number of retries so high? Is it possible to make the rebalancer making more progress each iteration instead just in the sake of shortening run-time of this test scenario?
http://gerrit.cloudera.org:8080/#/c/18454/32/src/kudu/master/auto_leader_rebalancer-test.cc@175
PS32, Line 175: SleepFor(MonoDelta::FromSeconds(2));
Why is it necessary to have a pause here? Would be great to add a comment to clarify.
http://gerrit.cloudera.org:8080/#/c/18454/32/src/kudu/master/auto_leader_rebalancer.h
File src/kudu/master/auto_leader_rebalancer.h:
http://gerrit.cloudera.org:8080/#/c/18454/32/src/kudu/master/auto_leader_rebalancer.h@45
PS32, Line 45: // A CatalogManager background task which auto-rebalances tablets' leaders distribution.
nit: remove this period
http://gerrit.cloudera.org:8080/#/c/18454/32/src/kudu/master/auto_leader_rebalancer.h@46
PS32, Line 46: by the way leader transfer
by transferring leadership between tablet replicas
http://gerrit.cloudera.org:8080/#/c/18454/32/src/kudu/master/auto_leader_rebalancer.h@95
PS32, Line 95: std::shared_ptr<rpc::Messenger> messenger_;
nit: please document this member as well -- what is it for?
http://gerrit.cloudera.org:8080/#/c/18454/32/src/kudu/master/auto_leader_rebalancer.cc
File src/kudu/master/auto_leader_rebalancer.cc:
http://gerrit.cloudera.org:8080/#/c/18454/32/src/kudu/master/auto_leader_rebalancer.cc@55
PS32, Line 55: using std::map;
: using std::nullopt;
: using std::string;
: using std::vector;
:
: using kudu::consensus::ConsensusServiceProxy;
: using kudu::consensus::LeaderStepDownMode;
: using kudu::consensus::LeaderStepDownRequestPB;
: using kudu::consensus::LeaderStepDownResponsePB;
: using kudu::consensus::RaftPeerPB;
:
: using kudu::rpc::MessengerBuilder;
: using kudu::rpc::RpcController;
: using strings::Substitute;
nit: remove empty lines and sort the rest of non-empty lines alphabetically
http://gerrit.cloudera.org:8080/#/c/18454/32/src/kudu/master/auto_leader_rebalancer.cc@293
PS32, Line 293: vary
very?
http://gerrit.cloudera.org:8080/#/c/18454/32/src/kudu/master/auto_leader_rebalancer.cc@293
PS32, Line 293: synchronized rpc
What's "synchronized rpc"?
http://gerrit.cloudera.org:8080/#/c/18454/32/src/kudu/master/auto_leader_rebalancer.cc@299
PS32, Line 299: LOG(INFO)
I don't think this sort of information is worth it having in the INFO log: maybe, change this into VLOG(1) instead?
http://gerrit.cloudera.org:8080/#/c/18454/32/src/kudu/master/auto_leader_rebalancer.cc@304
PS32, Line 304: string
nit: could this be 'const string&' or 'const auto&' to avoid copying?
http://gerrit.cloudera.org:8080/#/c/18454/32/src/kudu/master/auto_leader_rebalancer.cc@330
PS32, Line 330: strings::
Drop the namespace prefix since there is corresponding 'using' using strings::Substitute' in the beginning of this file.
http://gerrit.cloudera.org:8080/#/c/18454/32/src/kudu/master/auto_leader_rebalancer.cc@337
PS32, Line 337: VLOG(0)
What's VLOG(0), and why not to use VLOG(1) for this?
http://gerrit.cloudera.org:8080/#/c/18454/32/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:
http://gerrit.cloudera.org:8080/#/c/18454/32/src/kudu/master/catalog_manager.cc@353
PS32, Line 353: auto-leader-rebalancing
automatic leader rebalancing
http://gerrit.cloudera.org:8080/#/c/18454/32/src/kudu/master/catalog_manager.cc@1062
PS32, Line 1062: depend
depends
http://gerrit.cloudera.org:8080/#/c/18454/32/src/kudu/master/catalog_manager.cc@1062
PS32, Line 1062: good replicas balance
Do you mean it relies on the even distribution of tablet replicas across all existing tablet servers, so every tablet server contains almost the same number of tablet replicas as any other tablet server?
http://gerrit.cloudera.org:8080/#/c/18454/32/src/kudu/master/catalog_manager.cc@1063
PS32, Line 1063: rebalance
rebalancing
http://gerrit.cloudera.org:8080/#/c/18454/32/src/kudu/master/catalog_manager.cc@1063
PS32, Line 1063: auto_rebalancing
auto-rebalancing
http://gerrit.cloudera.org:8080/#/c/18454/32/src/kudu/master/catalog_manager.cc@1064
PS32, Line 1064: load rebalance
What is 'load rebalance'?
http://gerrit.cloudera.org:8080/#/c/18454/32/src/kudu/master/catalog_manager.cc@1064
PS32, Line 1064: cann't
cannot
http://gerrit.cloudera.org:8080/#/c/18454/32/src/kudu/master/catalog_manager.cc@1067
PS32, Line 1067: becaunse
because
http://gerrit.cloudera.org:8080/#/c/18454/32/src/kudu/master/catalog_manager.cc@1067
PS32, Line 1067: follows
followers?
http://gerrit.cloudera.org:8080/#/c/18454/32/src/kudu/master/catalog_manager.cc@1067
PS32, Line 1067: propotion
?
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 32
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.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: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Sat, 01 Oct 2022 01:58:09 +0000
Gerrit-HasComments: Yes
[kudu-CR] [master] KUDU-3390 support auto rebalance tablet leaders across TServers
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Yuqi Du has posted comments on this change. ( http://gerrit.cloudera.org:8080/18454 )
Change subject: [master] KUDU-3390 support auto rebalance tablet leaders across TServers
......................................................................
Patch Set 35:
(13 comments)
Thank you for you advices. I have fixed them, You can review them again.
http://gerrit.cloudera.org:8080/#/c/18454/35/src/kudu/master/auto_leader_rebalancer.h
File src/kudu/master/auto_leader_rebalancer.h:
http://gerrit.cloudera.org:8080/#/c/18454/35/src/kudu/master/auto_leader_rebalancer.h@45
PS35, Line 45: .
> nit: remove
Done
http://gerrit.cloudera.org:8080/#/c/18454/35/src/kudu/master/auto_leader_rebalancer.h@45
PS35, Line 45: .
> nit: remove
Done
http://gerrit.cloudera.org:8080/#/c/18454/35/src/kudu/master/auto_leader_rebalancer.h@95
PS35, Line 95: ’LeaderStepDown‘
> nit: 'LeaderStepDown'
Done
http://gerrit.cloudera.org:8080/#/c/18454/35/src/kudu/master/auto_leader_rebalancer.cc
File src/kudu/master/auto_leader_rebalancer.cc:
http://gerrit.cloudera.org:8080/#/c/18454/35/src/kudu/master/auto_leader_rebalancer.cc@167
PS35, Line 167: vector<string>()
> nit: can be simplified by "{}"?
'auto& leader_uuids' is a left reference , it cannot be '{}'
http://gerrit.cloudera.org:8080/#/c/18454/35/src/kudu/master/auto_leader_rebalancer.cc@191
PS35, Line 191: statistics
> 'statistics' is not very clear, how about replica_and_leader_count_by_ts_uu
Done
http://gerrit.cloudera.org:8080/#/c/18454/35/src/kudu/master/auto_leader_rebalancer.cc@223
PS35, Line 223: expected_count
> nit: how about need_transfer_count?
Done
http://gerrit.cloudera.org:8080/#/c/18454/35/src/kudu/master/auto_leader_rebalancer.cc@227
PS35, Line 227: pick count leader, and pick dest uuid
> The comment is not clear enough, could you improve it?
Done
http://gerrit.cloudera.org:8080/#/c/18454/35/src/kudu/master/auto_leader_rebalancer.cc@239
PS35, Line 239: pair
> nit: replica_and_leader_count ?
Done
http://gerrit.cloudera.org:8080/#/c/18454/35/src/kudu/master/auto_leader_rebalancer.cc@256
PS35, Line 256: pair
> same
Done
http://gerrit.cloudera.org:8080/#/c/18454/35/src/kudu/master/auto_leader_rebalancer.cc@295
PS35, Line 295: if (leader_transfer_count >= FLAGS_leader_rebalancing_max_moves_per_round) {
> How about limit the leader_transfer_tasks size when collect it? then reduce
??
http://gerrit.cloudera.org:8080/#/c/18454/35/src/kudu/master/auto_leader_rebalancer.cc@312
PS35, Line 312: auto it = host_port_by_leader_ts_uuid.find(leader_uuid);
> Use FindOrNull ?
At this, I think it's no distinguish.
`
auto it = host_port_by_leader_ts_uuid.find(leader_uuid);
if (it == host_port_by_leader_ts_uuid.end()) {
continue;
}
or
auto* host_port = FindOrNull(host_port_by_leader_ts_uuid, leader_uuid);
if (!host_port) {
continue;
}
`
Fixed.
http://gerrit.cloudera.org:8080/#/c/18454/35/src/kudu/master/auto_leader_rebalancer.cc@381
PS35, Line 381: LOG(INFO) << "all tables leader rebalance finished this round";
> It would be better if printing the summary of the rebalance work, for examp
'RunLeaderRebalanceForTable' has printed the detail log. Yes, Next patch can try to rerich the log and add metrics for leader rebalancer.
http://gerrit.cloudera.org:8080/#/c/18454/35/src/kudu/master/auto_leader_rebalancer.cc@390
PS35, Line 390: Something wrong, maybe the master instance isn't leader
> Will RunLeaderRebalancer return the corresponding error? use it directly an
The only failed status is 'RETURN_NOT_OK(leader_lock.first_failed_status());' so I direct change the log.
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 35
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.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: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Thu, 10 Nov 2022 11:01:41 +0000
Gerrit-HasComments: Yes
[kudu-CR] [master] KUDU-3061 support auto rebalance tablet leaders across TServers
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Yingchun Lai, Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18454
to look at the new patch set (#16).
Change subject: [master] KUDU-3061 support auto rebalance tablet leaders across TServers
......................................................................
[master] KUDU-3061 support auto rebalance tablet leaders across TServers
The number of leader replicas per tablet server can become imbalanced
over time, which lead to load skew on some nodes.
Adding an auto leader rebalance task to avoid leader replicas skew.
Two reasons of load skew:
1. The main reason. Scan Requests has two modes: LeaderOnly(default) and CLOSEST_REPLICA.
For more accurate results, users will choose the LeaderOnly(default) mode.
Mostly, the scan load is positive correlation with leader numbers.
2. The other reason. Write requests, leaders receive write requests and followers receive
appendEntries(kudu is UpdateConsensus), the flow of processing is a little different, which
is hidden variables, maybe cause imbalanced load. Leader rebalance will make leader and
followers balanced and eliminate hidden variables and make service more stable.
Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
---
M src/kudu/client/flex_partitioning_client-test.cc
M src/kudu/master/CMakeLists.txt
A src/kudu/master/auto_leader_rebalancer-test.cc
A src/kudu/master/auto_leader_rebalancer.cc
A src/kudu/master/auto_leader_rebalancer.h
M src/kudu/master/auto_rebalancer-test.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_path_handlers.cc
11 files changed, 792 insertions(+), 23 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18454/16
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 16
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: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
[kudu-CR] [master] KUDU-3061 support auto rebalance tablet leaders across TServers
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/18454 )
Change subject: [master] KUDU-3061 support auto rebalance tablet leaders across TServers
......................................................................
Patch Set 19:
(9 comments)
http://gerrit.cloudera.org:8080/#/c/18454/19/src/kudu/client/flex_partitioning_client-test.cc
File src/kudu/client/flex_partitioning_client-test.cc:
http://gerrit.cloudera.org:8080/#/c/18454/19/src/kudu/client/flex_partitioning_client-test.cc@341
PS19, Line 341: // Using one-level hash bucketing { 3, "key" } as the custom hash schema
Sorry, I didn't get the point. Is this related to this patch?
http://gerrit.cloudera.org:8080/#/c/18454/19/src/kudu/master/auto_leader_rebalancer.cc
File src/kudu/master/auto_leader_rebalancer.cc:
http://gerrit.cloudera.org:8080/#/c/18454/19/src/kudu/master/auto_leader_rebalancer.cc@68
PS19, Line 68: 10,
nit: would it be more convenient if the gflag name and default value are in one line when we grep it?
http://gerrit.cloudera.org:8080/#/c/18454/19/src/kudu/master/auto_leader_rebalancer.cc@72
PS19, Line 72: 3600,
same
http://gerrit.cloudera.org:8080/#/c/18454/19/src/kudu/master/auto_leader_rebalancer.cc@96
PS19, Line 96: "catalog-manager"
nit: better to use the same category name, i.e. "catalog manager", as other threads in CatalogManager, then they will be in the same group on webUI.
http://gerrit.cloudera.org:8080/#/c/18454/19/src/kudu/master/auto_leader_rebalancer.cc@103
PS19, Line 103: if (thread_) {
Is there any case thread_ is not initialized?
http://gerrit.cloudera.org:8080/#/c/18454/19/src/kudu/master/auto_leader_rebalancer.cc@181
PS19, Line 181: const vector<string>& leaders = FindWithDefault(uuid_leaders_map, uuid, {});
: int32_t leader_count = leaders.size();
nit: move these code closer to the place where it used, means after L188.
http://gerrit.cloudera.org:8080/#/c/18454/19/src/kudu/master/auto_leader_rebalancer.cc@192
PS19, Line 192: int32_t should_transfer_count = leader_count - (replica_count / table_data.num_replicas() + 1);
Could you please add some comments to describe the algorithm?
http://gerrit.cloudera.org:8080/#/c/18454/19/src/kudu/master/auto_rebalancer.cc
File src/kudu/master/auto_rebalancer.cc:
http://gerrit.cloudera.org:8080/#/c/18454/19/src/kudu/master/auto_rebalancer.cc@a181
PS19, Line 181:
why remove it?
http://gerrit.cloudera.org:8080/#/c/18454/19/src/kudu/master/auto_rebalancer.cc@177
PS19, Line 177: [this]() { this->RunLoop(); }, &thread_);
:
nit: alignment and remove blank line.
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 19
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: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Tue, 05 Jul 2022 16:44:39 +0000
Gerrit-HasComments: Yes
[kudu-CR] [master] KUDU-3061 support auto rebalance tablet leaders across TServers
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18454 )
Change subject: [master] KUDU-3061 support auto rebalance tablet leaders across TServers
......................................................................
Patch Set 19:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/18454/19//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18454/19//COMMIT_MSG@10
PS19, Line 10: which lead to load skew on some nodes.
Do you have any data to show how much imbalance it is? I'm not convinced there is a lot of sense doing leader rebalancing without the evidence that supports it.
http://gerrit.cloudera.org:8080/#/c/18454/19//COMMIT_MSG@20
PS19, Line 20: flow of processing is a little different, which
: is hidden variables
So, how much is that it total? Is that just minuscule amount or memory per node?
http://gerrit.cloudera.org:8080/#/c/18454/19//COMMIT_MSG@21
PS19, Line 21: maybe cause imbalanced load
Do you have any data to support this statement? How much imbalance we are talking about here?
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 19
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: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Fri, 08 Jul 2022 16:42:36 +0000
Gerrit-HasComments: Yes
[kudu-CR] [master] KUDU-3390 support auto rebalance tablet leaders across TServers
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Yingchun Lai, Yifan Zhang, Kudu Jenkins, Andrew Wong, Abhishek Chennaka,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18454
to look at the new patch set (#28).
Change subject: [master] KUDU-3390 support auto rebalance tablet leaders across TServers
......................................................................
[master] KUDU-3390 support auto rebalance tablet leaders across TServers
The number of leader replicas per tablet server can become imbalanced
over time, which lead to load skew on some nodes. This patch adding and
auto leader rebalance task to avoid leader replicas skew.
Two reasons of load skew:
- The main reason. Scan Requests has two modes: LeaderOnly(default) and CLOSEST_REPLICA.
For more accurate results, users will choose the LeaderOnly(default) mode.
Mostly, the scan load is positive correlation with leader numbers.
- The other reason. Write requests, leaders receive write requests and followers receive
UpdateConsensus request(raft called AppendEntries), the flow of processing is a little
different, which is hidden variables, maybe cause imbalanced load. Leader rebalance will
make leader and followers balanced and eliminate hidden variables and make service more stable.
An experiment present leader rebalance's effect(more detail infomations at KUDU-3390):
|leader ratio| scan cost | cpu usage | io
before|40: 0: 0 | 811.586 s | 47%, 18%, 19% | 102MB/s ioutil:55%, 8KB/s 2%, 64KB/s 3%
after |13: 14: 13 | 611.012 s | 39%, 45%, 35% | 53MB/s ioutil:31%, 80MB/s 18%, 45MB/s 24%
Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
---
M src/kudu/master/CMakeLists.txt
A src/kudu/master/auto_leader_rebalancer-test.cc
A src/kudu/master/auto_leader_rebalancer.cc
A src/kudu/master/auto_leader_rebalancer.h
M src/kudu/master/auto_rebalancer-test.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
9 files changed, 804 insertions(+), 16 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18454/28
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 28
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.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: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
[kudu-CR] [master] KUDU-3390 support auto rebalance tablet leaders across TServers
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Yingchun Lai, Yifan Zhang, Kudu Jenkins, Andrew Wong, Abhishek Chennaka,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18454
to look at the new patch set (#29).
Change subject: [master] KUDU-3390 support auto rebalance tablet leaders across TServers
......................................................................
[master] KUDU-3390 support auto rebalance tablet leaders across TServers
The number of leader replicas per tablet server can become imbalanced
over time, which lead to load skew on some nodes. This patch adding and
auto leader rebalance task to avoid leader replicas skew.
Two reasons of load skew:
- The main reason. Scan Requests has two modes: LeaderOnly(default) and CLOSEST_REPLICA.
For more accurate results, users will choose the LeaderOnly(default) mode.
Mostly, the scan load is positive correlation with leader numbers.
- The other reason. Write requests, leaders receive write requests and followers receive
UpdateConsensus request(raft called AppendEntries), the flow of processing is a little
different, which is hidden variables, maybe cause imbalanced load. Leader rebalance will
make leader and followers balanced and eliminate hidden variables and make service more stable.
An experiment present leader rebalance's effect(more detail infomations at KUDU-3390):
|leader ratio| scan cost | cpu usage | io
before|40: 0: 0 | 811.586 s | 47%, 18%, 19% | 102MB/s ioutil:55%, 8KB/s 2%, 64KB/s 3%
after |13: 14: 13 | 611.012 s | 39%, 45%, 35% | 53MB/s ioutil:31%, 80MB/s 18%, 45MB/s 24%
Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
---
M src/kudu/master/CMakeLists.txt
A src/kudu/master/auto_leader_rebalancer-test.cc
A src/kudu/master/auto_leader_rebalancer.cc
A src/kudu/master/auto_leader_rebalancer.h
M src/kudu/master/auto_rebalancer-test.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
9 files changed, 804 insertions(+), 16 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18454/29
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 29
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.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: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
[kudu-CR] [master] KUDU-3390 support auto rebalance tablet leaders across TServers
Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Yifan Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/18454 )
Change subject: [master] KUDU-3390 support auto rebalance tablet leaders across TServers
......................................................................
Patch Set 30:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/18454/30//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18454/30//COMMIT_MSG@20
PS30, Line 20: which is hidden variables
Maybe you need to explain what leads to an unbalanced load, I really don't know what this means.
http://gerrit.cloudera.org:8080/#/c/18454/30/src/kudu/master/auto_leader_rebalancer.cc
File src/kudu/master/auto_leader_rebalancer.cc:
http://gerrit.cloudera.org:8080/#/c/18454/30/src/kudu/master/auto_leader_rebalancer.cc@287
PS30, Line 287: // if the num of tasks is vary large, synchronized rpc will be slow,
: // causing the thread cost much time to do the job.
Besides, if the leadership of many tablets is changed, clients will send more GetTabletLocations requests to the leader master, and the master will be overloaded. Maybe we can introduce a flag similar to 'auto_rebalancing_max_moves_per_server'.
http://gerrit.cloudera.org:8080/#/c/18454/30/src/kudu/master/auto_leader_rebalancer.cc@350
PS30, Line 350: if (e->PresumedDead()) {
If there are some temporarily unavailable tservers in a cluster, is it a good idea to just ignore them in this leader rebalancing task? What if they can recover in a short time?
http://gerrit.cloudera.org:8080/#/c/18454/30/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:
http://gerrit.cloudera.org:8080/#/c/18454/30/src/kudu/master/catalog_manager.cc@352
PS30, Line 352: auto_leader_rebalancing_enabled
This flag should be tagged with 'runtime' too.
http://gerrit.cloudera.org:8080/#/c/18454/30/src/kudu/master/catalog_manager.cc@1061
PS30, Line 1061: // Leader rebalancer depend on a good replicas topology, that means we'd better enable
: // auto_rebalancing, but when auto_rebalancing is disabled and leader rebalance is enabled,
: // that is ok, we support it.
:
I think it is worth pointing out the difference between rebalancing tablet leaders based on a balanced cluster and an unbalanced cluster.
Have you also tried starting leader rebalancing tasks only on an extremely unbalanced cluster? Would that make things worse?
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 30
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.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: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Mon, 26 Sep 2022 13:45:48 +0000
Gerrit-HasComments: Yes
[kudu-CR] [master] KUDU-3390 support auto rebalance tablet leaders across TServers
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18454 )
Change subject: [master] KUDU-3390 support auto rebalance tablet leaders across TServers
......................................................................
Patch Set 40:
(2 comments)
I'm late with my review, but this is my feedback so far for PS40
http://gerrit.cloudera.org:8080/#/c/18454/40/src/kudu/master/auto_leader_rebalancer-test.cc
File src/kudu/master/auto_leader_rebalancer-test.cc:
http://gerrit.cloudera.org:8080/#/c/18454/40/src/kudu/master/auto_leader_rebalancer-test.cc@174
PS40, Line 174: TEST_F(LeaderRebalancerTest, RestartTserver) {
From what I can see, the following isn't covered by tests: the case when master leadership changes, it's necessary to make sure the leader rebalancing is going to be run by a new leader.
http://gerrit.cloudera.org:8080/#/c/18454/40/src/kudu/master/auto_leader_rebalancer.cc
File src/kudu/master/auto_leader_rebalancer.cc:
http://gerrit.cloudera.org:8080/#/c/18454/40/src/kudu/master/auto_leader_rebalancer.cc@349
PS40, Line 349: MutexLock auto_lock(running_mutex_);
What is this for? RunLeaderRebalancer() is called from a single thread, so why on earth this mutex is here?
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 40
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.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: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Wed, 16 Nov 2022 18:13:27 +0000
Gerrit-HasComments: Yes
[kudu-CR] [master] KUDU-3390 support auto rebalance tablet leaders across TServers
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Yingchun Lai, Yifan Zhang, Kudu Jenkins, Andrew Wong, Abhishek Chennaka,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18454
to look at the new patch set (#36).
Change subject: [master] KUDU-3390 support auto rebalance tablet leaders across TServers
......................................................................
[master] KUDU-3390 support auto rebalance tablet leaders across TServers
The number of leader replicas per tablet server can become imbalanced
over time, which lead to load skew on some nodes. This patch adds
auto leader rebalance task to avoid leader replicas skew.
Two reasons of load skew:
- The main reason. Scan Requests has two modes: LeaderOnly(default) and CLOSEST_REPLICA.
For more accurate results, users will choose the LeaderOnly(default) mode.
Mostly, the scan load is positive correlation with leader numbers.
- The other reason. Write requests, leaders receive write requests and followers receive
UpdateConsensus request(raft called AppendEntries), the flow of processing is a little
different: the queue, pipeline, batch size, that may cause imbalanced load. Leader
rebalance will make leader and followers balanced and make service more stable.
An experiment present leader rebalance's effect (more details at KUDU-3390):
|leader ratio| scan cost | cpu usage | io
before|40: 0: 0 | 811.586 s | 47%, 18%, 19% | 102MB/s ioutil:55%, 8KB/s 2%, 64KB/s 3%
after |13: 14: 13 | 611.012 s | 39%, 45%, 35% | 53MB/s ioutil:31%, 80MB/s 18%, 45MB/s 24%
Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
---
M src/kudu/master/CMakeLists.txt
A src/kudu/master/auto_leader_rebalancer-test.cc
A src/kudu/master/auto_leader_rebalancer.cc
A src/kudu/master/auto_leader_rebalancer.h
M src/kudu/master/auto_rebalancer-test.cc
M src/kudu/master/auto_rebalancer.h
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
8 files changed, 836 insertions(+), 15 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18454/36
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 36
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.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: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
[kudu-CR] [master] KUDU-3390 support auto rebalance tablet leaders across TServers
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/18454 )
Change subject: [master] KUDU-3390 support auto rebalance tablet leaders across TServers
......................................................................
Patch Set 35:
(12 comments)
http://gerrit.cloudera.org:8080/#/c/18454/35/src/kudu/master/auto_leader_rebalancer.h
File src/kudu/master/auto_leader_rebalancer.h:
http://gerrit.cloudera.org:8080/#/c/18454/35/src/kudu/master/auto_leader_rebalancer.h@45
PS35, Line 45: .
nit: remove
http://gerrit.cloudera.org:8080/#/c/18454/35/src/kudu/master/auto_leader_rebalancer.h@95
PS35, Line 95: ’LeaderStepDown‘
nit: 'LeaderStepDown'
http://gerrit.cloudera.org:8080/#/c/18454/35/src/kudu/master/auto_leader_rebalancer.cc
File src/kudu/master/auto_leader_rebalancer.cc:
http://gerrit.cloudera.org:8080/#/c/18454/35/src/kudu/master/auto_leader_rebalancer.cc@167
PS35, Line 167: vector<string>()
nit: can be simplified by "{}"?
http://gerrit.cloudera.org:8080/#/c/18454/35/src/kudu/master/auto_leader_rebalancer.cc@191
PS35, Line 191: statistics
'statistics' is not very clear, how about replica_and_leader_count_by_ts_uuid or sth like this ?
http://gerrit.cloudera.org:8080/#/c/18454/35/src/kudu/master/auto_leader_rebalancer.cc@223
PS35, Line 223: expected_count
nit: how about need_transfer_count?
http://gerrit.cloudera.org:8080/#/c/18454/35/src/kudu/master/auto_leader_rebalancer.cc@227
PS35, Line 227: pick count leader, and pick dest uuid
The comment is not clear enough, could you improve it?
http://gerrit.cloudera.org:8080/#/c/18454/35/src/kudu/master/auto_leader_rebalancer.cc@239
PS35, Line 239: pair
nit: replica_and_leader_count ?
http://gerrit.cloudera.org:8080/#/c/18454/35/src/kudu/master/auto_leader_rebalancer.cc@256
PS35, Line 256: pair
same
http://gerrit.cloudera.org:8080/#/c/18454/35/src/kudu/master/auto_leader_rebalancer.cc@295
PS35, Line 295: if (leader_transfer_count >= FLAGS_leader_rebalancing_max_moves_per_round) {
How about limit the leader_transfer_tasks size when collect it? then reduce the cost of collection as well.
http://gerrit.cloudera.org:8080/#/c/18454/35/src/kudu/master/auto_leader_rebalancer.cc@312
PS35, Line 312: auto it = host_port_by_leader_ts_uuid.find(leader_uuid);
Use FindOrNull ?
http://gerrit.cloudera.org:8080/#/c/18454/35/src/kudu/master/auto_leader_rebalancer.cc@381
PS35, Line 381: LOG(INFO) << "all tables leader rebalance finished this round";
It would be better if printing the summary of the rebalance work, for example, how many replicas switch leadership. You can do it in another patch.
http://gerrit.cloudera.org:8080/#/c/18454/35/src/kudu/master/auto_leader_rebalancer.cc@390
PS35, Line 390: Something wrong, maybe the master instance isn't leader
Will RunLeaderRebalancer return the corresponding error? use it directly and not guess what the reason is, it will make user confused if they are not match.
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 35
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.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: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Tue, 08 Nov 2022 08:33:36 +0000
Gerrit-HasComments: Yes
[kudu-CR] [master] KUDU-3061 support auto rebalance tablet leaders across TServers
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Yuqi Du has posted comments on this change. ( http://gerrit.cloudera.org:8080/18454 )
Change subject: [master] KUDU-3061 support auto rebalance tablet leaders across TServers
......................................................................
Patch Set 9:
(3 comments)
Thanks for your crs.
http://gerrit.cloudera.org:8080/#/c/18454/8//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18454/8//COMMIT_MSG@9
PS8, Line 9: replica
> nit? tablet replicas
Done
http://gerrit.cloudera.org:8080/#/c/18454/8/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:
http://gerrit.cloudera.org:8080/#/c/18454/8/src/kudu/master/catalog_manager.cc@3695
PS8, Line 3695:
> Always return OK, can we return void? Same to GetTableInfo above.
Done
http://gerrit.cloudera.org:8080/#/c/18454/8/src/kudu/master/catalog_manager.cc@3697
PS8, Line 3697:
> Should AssertAcquired this lock?
Done
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 9
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: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Thu, 19 May 2022 06:59:33 +0000
Gerrit-HasComments: Yes
[kudu-CR] [master] KUDU-3390 support auto rebalance tablet leaders across TServers
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Yingchun Lai, Yifan Zhang, Kudu Jenkins, Andrew Wong, Abhishek Chennaka,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18454
to look at the new patch set (#32).
Change subject: [master] KUDU-3390 support auto rebalance tablet leaders across TServers
......................................................................
[master] KUDU-3390 support auto rebalance tablet leaders across TServers
The number of leader replicas per tablet server can become imbalanced
over time, which lead to load skew on some nodes. This patch adding and
auto leader rebalance task to avoid leader replicas skew.
Two reasons of load skew:
- The main reason. Scan Requests has two modes: LeaderOnly(default) and CLOSEST_REPLICA.
For more accurate results, users will choose the LeaderOnly(default) mode.
Mostly, the scan load is positive correlation with leader numbers.
- The other reason. Write requests, leaders receive write requests and followers receive
UpdateConsensus request(raft called AppendEntries), the flow of processing is a little
different: the queue, pipeline, batch size, that may cause imbalanced load. Leader
rebalance will make leader and followers balanced and make service more stable.
An experiment present leader rebalance's effect(more detail infomations at KUDU-3390):
|leader ratio| scan cost | cpu usage | io
before|40: 0: 0 | 811.586 s | 47%, 18%, 19% | 102MB/s ioutil:55%, 8KB/s 2%, 64KB/s 3%
after |13: 14: 13 | 611.012 s | 39%, 45%, 35% | 53MB/s ioutil:31%, 80MB/s 18%, 45MB/s 24%
Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
---
M src/kudu/master/CMakeLists.txt
A src/kudu/master/auto_leader_rebalancer-test.cc
A src/kudu/master/auto_leader_rebalancer.cc
A src/kudu/master/auto_leader_rebalancer.h
M src/kudu/master/auto_rebalancer-test.cc
M src/kudu/master/auto_rebalancer.h
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
8 files changed, 835 insertions(+), 15 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18454/32
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 32
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.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: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
[kudu-CR] [master] KUDU-3390 support auto rebalance tablet leaders across TServers
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/18454 )
Change subject: [master] KUDU-3390 support auto rebalance tablet leaders across TServers
......................................................................
Patch Set 29:
(10 comments)
http://gerrit.cloudera.org:8080/#/c/18454/19//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18454/19//COMMIT_MSG@10
PS19, Line 10: which lead to load skew on some nodes.
> I take some time to find an issue about leader imbalance at our internal JI
Agree, I also saw many cases that the leader imbalanced and it lead to write and scan (especially when use LEADER_ONLY) performance degraded.
http://gerrit.cloudera.org:8080/#/c/18454/29/src/kudu/master/auto_leader_rebalancer.h
File src/kudu/master/auto_leader_rebalancer.h:
http://gerrit.cloudera.org:8080/#/c/18454/29/src/kudu/master/auto_leader_rebalancer.h@59
PS29, Line 59: Initializes the auto-leader-rebalancer.
nit: how about?
Initializes the objects and starts the thread.
or remove it?
http://gerrit.cloudera.org:8080/#/c/18454/29/src/kudu/master/auto_leader_rebalancer.cc
File src/kudu/master/auto_leader_rebalancer.cc:
http://gerrit.cloudera.org:8080/#/c/18454/29/src/kudu/master/auto_leader_rebalancer.cc@68
PS29, Line 68: DEFINE_uint32(auto_leader_rebalancing_rpc_timeout_seconds, 10,
Add some tags for these new added flags, consider ‘experimental’, 'unsafe', 'runtime'.
http://gerrit.cloudera.org:8080/#/c/18454/29/src/kudu/master/auto_leader_rebalancer.cc@114
PS29, Line 114: CHECK(replication_factor > 0);
nit: DCHECK_GT(replication_factor, 0);
http://gerrit.cloudera.org:8080/#/c/18454/29/src/kudu/master/auto_leader_rebalancer.cc@120
PS29, Line 120: tablet_id of leader -> uuid
nit: tablet_id -> leader tserver uuid ?
http://gerrit.cloudera.org:8080/#/c/18454/29/src/kudu/master/auto_leader_rebalancer.cc@121
PS29, Line 121: leader_uuid_map
In Kudu's naming habit, the map offen named as 'value_by_key', for example leader_ts_uuid_by_tablet_id.
http://gerrit.cloudera.org:8080/#/c/18454/29/src/kudu/master/auto_leader_rebalancer.cc@122
PS29, Line 122: tablet_id of follower -> vector<uuid>
nit: tablet_id -> follower tserver uuids ?
http://gerrit.cloudera.org:8080/#/c/18454/29/src/kudu/master/auto_leader_rebalancer.cc@124
PS29, Line 124: tserver uuid -> vector<all leaders' tablet id>
:
nit: tserver uuid -> leader replicas ?
http://gerrit.cloudera.org:8080/#/c/18454/29/src/kudu/master/auto_leader_rebalancer.cc@126
PS29, Line 126: tserver uuid -> vector<all replicas' tablet id>
nit: tserver uuid -> all replicas ?
http://gerrit.cloudera.org:8080/#/c/18454/29/src/kudu/master/auto_rebalancer.cc
File src/kudu/master/auto_rebalancer.cc:
http://gerrit.cloudera.org:8080/#/c/18454/29/src/kudu/master/auto_rebalancer.cc@179
PS29, Line 179:
nit: revert it
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 29
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.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: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Sat, 24 Sep 2022 17:07:19 +0000
Gerrit-HasComments: Yes
[kudu-CR] [master] KUDU-3390 support auto rebalance tablet leaders across TServers
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Yingchun Lai, Yifan Zhang, Kudu Jenkins, Andrew Wong, Abhishek Chennaka,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18454
to look at the new patch set (#34).
Change subject: [master] KUDU-3390 support auto rebalance tablet leaders across TServers
......................................................................
[master] KUDU-3390 support auto rebalance tablet leaders across TServers
The number of leader replicas per tablet server can become imbalanced
over time, which lead to load skew on some nodes. This patch adds
auto leader rebalance task to avoid leader replicas skew.
Two reasons of load skew:
- The main reason. Scan Requests has two modes: LeaderOnly(default) and CLOSEST_REPLICA.
For more accurate results, users will choose the LeaderOnly(default) mode.
Mostly, the scan load is positive correlation with leader numbers.
- The other reason. Write requests, leaders receive write requests and followers receive
UpdateConsensus request(raft called AppendEntries), the flow of processing is a little
different: the queue, pipeline, batch size, that may cause imbalanced load. Leader
rebalance will make leader and followers balanced and make service more stable.
An experiment present leader rebalance's effect (more details at KUDU-3390):
|leader ratio| scan cost | cpu usage | io
before|40: 0: 0 | 811.586 s | 47%, 18%, 19% | 102MB/s ioutil:55%, 8KB/s 2%, 64KB/s 3%
after |13: 14: 13 | 611.012 s | 39%, 45%, 35% | 53MB/s ioutil:31%, 80MB/s 18%, 45MB/s 24%
Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
---
M src/kudu/master/CMakeLists.txt
A src/kudu/master/auto_leader_rebalancer-test.cc
A src/kudu/master/auto_leader_rebalancer.cc
A src/kudu/master/auto_leader_rebalancer.h
M src/kudu/master/auto_rebalancer-test.cc
M src/kudu/master/auto_rebalancer.h
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
8 files changed, 828 insertions(+), 15 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18454/34
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 34
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.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: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
[kudu-CR] [master] KUDU-3061 support auto rebalance tablet leaders across TServers
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/18454 )
Change subject: [master] KUDU-3061 support auto rebalance tablet leaders across TServers
......................................................................
Patch Set 12:
(16 comments)
http://gerrit.cloudera.org:8080/#/c/18454/12/src/kudu/master/auto_leader_rebalancer.cc
File src/kudu/master/auto_leader_rebalancer.cc:
http://gerrit.cloudera.org:8080/#/c/18454/12/src/kudu/master/auto_leader_rebalancer.cc@71
PS12, Line 71: "FLAGS_auto_leader_rebalancing_interval_seconds");
nit: need improve description
http://gerrit.cloudera.org:8080/#/c/18454/12/src/kudu/master/auto_leader_rebalancer.cc@136
PS12, Line 136: std::
nit: can be omit
http://gerrit.cloudera.org:8080/#/c/18454/12/src/kudu/master/auto_leader_rebalancer.cc@147
PS12, Line 147: // GetTabletLocations() will fail if the catalog manager is not the
Better to comment closer to the code.
http://gerrit.cloudera.org:8080/#/c/18454/12/src/kudu/master/auto_leader_rebalancer.cc@165
PS12, Line 165: auto it = uuid_leaders_map.find(uuid);
: if (it != uuid_leaders_map.end()) {
: it->second.emplace_back(tablet->id());
: } else {
: uuid_leaders_map.insert({uuid, std::vector<string>({tablet->id()})});
: }
There maybe some useful function you can use, see src/kudu/gutil/map-util.h
http://gerrit.cloudera.org:8080/#/c/18454/12/src/kudu/master/auto_leader_rebalancer.cc@175
PS12, Line 175: auto it = follower_uuid_map.find(tablet->id());
: if (it != follower_uuid_map.end()) {
: follower_uuid_map[tablet->id()].push_back(uuid);
: } else {
: follower_uuid_map.insert({tablet->id(), std::vector<string>({uuid})});
: }
Same
http://gerrit.cloudera.org:8080/#/c/18454/12/src/kudu/master/auto_leader_rebalancer.cc@186
PS12, Line 186: auto it = uuid_replicas_map.find(ts_info.permanent_uuid());
: if (it == uuid_replicas_map.end()) {
: uuid_replicas_map.insert(
: {ts_info.permanent_uuid(), std::vector<string>({tablet->id()})});
: } else {
: it->second.emplace_back(tablet->id());
: }
Same
http://gerrit.cloudera.org:8080/#/c/18454/12/src/kudu/master/auto_leader_rebalancer.cc@229
PS12, Line 229: LOG(INFO) << uuid
I think this log would be in VLOG(x) level.
http://gerrit.cloudera.org:8080/#/c/18454/12/src/kudu/master/auto_leader_rebalancer.cc@323
PS12, Line 323: auto it = leader_uuid_host_port_map.find(leader_uuid);
There are several code can be optimized by util functions in src/kudu/gutil/map-util.h
http://gerrit.cloudera.org:8080/#/c/18454/12/src/kudu/master/auto_leader_rebalancer.cc@337
PS12, Line 337: LOG(INFO) << strings::Substitute(
Also should be in VLOG(x)
http://gerrit.cloudera.org:8080/#/c/18454/12/src/kudu/master/auto_leader_rebalancer.cc@368
PS12, Line 368: std::set<std::string>
set<string>
http://gerrit.cloudera.org:8080/#/c/18454/12/src/kudu/master/auto_leader_rebalancer.cc@392
PS12, Line 392: Status s = RunLeaderRebalancer();
You can use WARN_NOT_OK to simplify code.
http://gerrit.cloudera.org:8080/#/c/18454/12/src/kudu/master/auto_rebalancer.cc
File src/kudu/master/auto_rebalancer.cc:
http://gerrit.cloudera.org:8080/#/c/18454/12/src/kudu/master/auto_rebalancer.cc@174
PS12, Line 174: if (FLAGS_auto_rebalancing_enabled) {
Why still create AutoRebalancerTask when FLAGS_auto_rebalancing_enabled is false?
http://gerrit.cloudera.org:8080/#/c/18454/12/src/kudu/master/auto_rebalancer.cc@180
PS12, Line 180: ;
nit: remove it
http://gerrit.cloudera.org:8080/#/c/18454/12/src/kudu/master/auto_rebalancer.cc@264
PS12, Line 264: bool should_stop = false;
Is it necessary to judge whether should stop loop by an extra variable 'should_stop'? The loop is using shutdown_.WaitFor, it will break automatically when shutdown_ become 0.
http://gerrit.cloudera.org:8080/#/c/18454/12/src/kudu/master/auto_rebalancer.cc@267
PS12, Line 267: // stop the loop
nit: remove this useless comment
http://gerrit.cloudera.org:8080/#/c/18454/12/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:
http://gerrit.cloudera.org:8080/#/c/18454/12/src/kudu/master/catalog_manager.cc@3695
PS12, Line 3695: scoped_refptr<TableInfo> *table) {
nit: alignment
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 12
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: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Wed, 01 Jun 2022 10:09:10 +0000
Gerrit-HasComments: Yes
[kudu-CR] [master] KUDU-3061 support auto rebalance tablet leaders across TServers
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Yingchun Lai, Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18454
to look at the new patch set (#11).
Change subject: [master] KUDU-3061 support auto rebalance tablet leaders across TServers
......................................................................
[master] KUDU-3061 support auto rebalance tablet leaders across TServers
The number of leader replicas per tablet server can become imbalanced
over time, which lead to load skew on some nodes.
Adding an auto leader rebalance task to avoid leader replicas skew.
Two reasons of load skew:
1. The main reason. Scan Requests has two modes: LeaderOnly(default) and CLOSEST_REPLICA.
For more accurate results, users will choose the LeaderOnly(default) mode.
Mostly, the scan load is positive correlation with leader numbers.
2. The other reason. Write requests, leaders receive write requests and followers receive
appendEntries(kudu is UpdateConsensus), the flow of processing is a little different, which
is hidden variables, maybe cause imbalanced load. Leader rebalance will make leader and
followers balanced and eliminate hidden variables and make service more stable.
Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
---
M src/kudu/client/flex_partitioning_client-test.cc
M src/kudu/master/CMakeLists.txt
A src/kudu/master/auto_leader_rebalancer-test.cc
A src/kudu/master/auto_leader_rebalancer.cc
A src/kudu/master/auto_leader_rebalancer.h
M src/kudu/master/auto_rebalancer-test.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_path_handlers.cc
11 files changed, 875 insertions(+), 42 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18454/11
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 11
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: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
[kudu-CR] [master] KUDU-3061 support auto rebalance tablet leaders across TServers
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Yuqi Du has posted comments on this change. ( http://gerrit.cloudera.org:8080/18454 )
Change subject: [master] KUDU-3061 support auto rebalance tablet leaders across TServers
......................................................................
Patch Set 11:
(1 comment)
Thanks your crs.
http://gerrit.cloudera.org:8080/#/c/18454/10//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18454/10//COMMIT_MSG@10
PS10, Line 10: to load skew on some nodes.
> I'm not sure I can parse this. Could you rephrase or clarify on this?
Ok, I can rewrite some sentences.
The work can start without the load data, it can be supported by results of the qualitative analysis.
1. ScanRequests default is LeaderOnly, mostly , so the scan load is positive correlation with leader numbers. This is main reason.
2. For write requests, leaders receive write request, followers receive appendEntries(kudu is UpdateConsensus), the flow
of processing is different , which maybe cause imbalanced load.
The quantity income of lauto eader rebalancer can be solved by some study if needed.
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 11
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: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Wed, 01 Jun 2022 02:47:30 +0000
Gerrit-HasComments: Yes
[kudu-CR] [master] KUDU-3061 support auto rebalance tablet leaders across TServers
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Yuqi Du has posted comments on this change. ( http://gerrit.cloudera.org:8080/18454 )
Change subject: [master] KUDU-3061 support auto rebalance tablet leaders across TServers
......................................................................
Patch Set 11:
Thanks your crs.
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 11
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: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Wed, 01 Jun 2022 02:49:22 +0000
Gerrit-HasComments: No
[kudu-CR] [master] KUDU-3061 support auto rebalance tablet leaders across TServers
Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Yifan Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/18454 )
Change subject: [master] KUDU-3061 support auto rebalance tablet leaders across TServers
......................................................................
Patch Set 19:
(1 comment)
I didn't really look into the codes because I'd like to understand the design consideration and the use of algorithm first.
Here are some questions I don't know much about:
- Is this leader rebalance task only transfer the leadership between replicas of the same tablet? Or it also copy replicas from one server to another?
- Do we have to rebalance replicas in a cluster before we run this leader rebalance task?
- Can we finally get a fully balanced cluster in all cases?
If you already have a design document about this issue, can you share it so wen can discuss about the details.
http://gerrit.cloudera.org:8080/#/c/18454/19//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18454/19//COMMIT_MSG@14
PS19, Line 14: Two reasons of load skew:
Besides clarify why we need to balance leader replicas across TServers, it would be better if you can show the skew of leaders in a cluster before and after run the balance task.
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 19
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: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Sun, 03 Jul 2022 16:17:30 +0000
Gerrit-HasComments: Yes
[kudu-CR] [master] KUDU-3061 support auto rebalance tablet leaders across TServers
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Yingchun Lai, Yifan Zhang, Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18454
to look at the new patch set (#21).
Change subject: [master] KUDU-3061 support auto rebalance tablet leaders across TServers
......................................................................
[master] KUDU-3061 support auto rebalance tablet leaders across TServers
The number of leader replicas per tablet server can become imbalanced
over time, which lead to load skew on some nodes.
Adding an auto leader rebalance task to avoid leader replicas skew.
Two reasons of load skew:
1. The main reason. Scan Requests has two modes: LeaderOnly(default) and CLOSEST_REPLICA.
For more accurate results, users will choose the LeaderOnly(default) mode.
Mostly, the scan load is positive correlation with leader numbers.
2. The other reason. Write requests, leaders receive write requests and followers receive
appendEntries(kudu is UpdateConsensus), the flow of processing is a little different, which
is hidden variables, maybe cause imbalanced load. Leader rebalance will make leader and
followers balanced and eliminate hidden variables and make service more stable.
Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
---
M src/kudu/client/flex_partitioning_client-test.cc
M src/kudu/master/CMakeLists.txt
A src/kudu/master/auto_leader_rebalancer-test.cc
A src/kudu/master/auto_leader_rebalancer.cc
A src/kudu/master/auto_leader_rebalancer.h
M src/kudu/master/auto_rebalancer-test.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_path_handlers.cc
11 files changed, 795 insertions(+), 21 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18454/21
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 21
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: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
[kudu-CR] [master] KUDU-3390 support auto rebalance tablet leaders across TServers
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Yingchun Lai, Yifan Zhang, Kudu Jenkins, Andrew Wong, Abhishek Chennaka,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18454
to look at the new patch set (#25).
Change subject: [master] KUDU-3390 support auto rebalance tablet leaders across TServers
......................................................................
[master] KUDU-3390 support auto rebalance tablet leaders across TServers
The number of leader replicas per tablet server can become imbalanced
over time, which lead to load skew on some nodes. This patch adding and
auto leader rebalance task to avoid leader replicas skew.
Two reasons of load skew:
- The main reason. Scan Requests has two modes: LeaderOnly(default) and CLOSEST_REPLICA.
For more accurate results, users will choose the LeaderOnly(default) mode.
Mostly, the scan load is positive correlation with leader numbers.
- The other reason. Write requests, leaders receive write requests and followers receive
UpdateConsensus request(raft called AppendEntries), the flow of processing is a little
different, which is hidden variables, maybe cause imbalanced load. Leader rebalance will
make leader and followers balanced and eliminate hidden variables and make service more stable.
An experiment present leader rebalance's effect(more detail infomations at KUDU-3390):
|leader ratio| scan cost | cpu usage | io
before|40: 0: 0 | 811.586 s | 47%, 18%, 19% | 102MB/s ioutil:55%, 8KB/s 2%, 64KB/s 3%
after |13: 14: 13 | 611.012 s | 39%, 45%, 35% | 53MB/s ioutil:31%, 80MB/s 18%, 45MB/s 24%
Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
---
M src/kudu/master/CMakeLists.txt
A src/kudu/master/auto_leader_rebalancer-test.cc
A src/kudu/master/auto_leader_rebalancer.cc
A src/kudu/master/auto_leader_rebalancer.h
M src/kudu/master/auto_rebalancer-test.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
9 files changed, 794 insertions(+), 12 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18454/25
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 25
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.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: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
[kudu-CR] [master] KUDU-3061 support auto rebalance tablet leaders across TServers
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/18454 )
Change subject: [master] KUDU-3061 support auto rebalance tablet leaders across TServers
......................................................................
Patch Set 19:
> Patch Set 19:
>
> (1 comment)
>
> I didn't really look into the codes because I'd like to understand the design consideration and the use of algorithm first.
> Here are some questions I don't know much about:
> - Is this leader rebalance task only transfer the leadership between replicas of the same tablet? Or it also copy replicas from one server to another?
> - Do we have to rebalance replicas in a cluster before we run this leader rebalance task?
> - Can we finally get a fully balanced cluster in all cases?
>
> If you already have a design document about this issue, can you share it so wen can discuss about the details.
+1
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 19
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: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Tue, 05 Jul 2022 16:45:11 +0000
Gerrit-HasComments: No
[kudu-CR] [master] KUDU-3061 support auto rebalance tablet leaders across TServers
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Yingchun Lai, Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18454
to look at the new patch set (#17).
Change subject: [master] KUDU-3061 support auto rebalance tablet leaders across TServers
......................................................................
[master] KUDU-3061 support auto rebalance tablet leaders across TServers
The number of leader replicas per tablet server can become imbalanced
over time, which lead to load skew on some nodes.
Adding an auto leader rebalance task to avoid leader replicas skew.
Two reasons of load skew:
1. The main reason. Scan Requests has two modes: LeaderOnly(default) and CLOSEST_REPLICA.
For more accurate results, users will choose the LeaderOnly(default) mode.
Mostly, the scan load is positive correlation with leader numbers.
2. The other reason. Write requests, leaders receive write requests and followers receive
appendEntries(kudu is UpdateConsensus), the flow of processing is a little different, which
is hidden variables, maybe cause imbalanced load. Leader rebalance will make leader and
followers balanced and eliminate hidden variables and make service more stable.
Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
---
M src/kudu/client/flex_partitioning_client-test.cc
M src/kudu/master/CMakeLists.txt
A src/kudu/master/auto_leader_rebalancer-test.cc
A src/kudu/master/auto_leader_rebalancer.cc
A src/kudu/master/auto_leader_rebalancer.h
M src/kudu/master/auto_rebalancer-test.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_path_handlers.cc
11 files changed, 792 insertions(+), 23 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18454/17
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 17
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: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
[kudu-CR] [master] KUDU-3390 support auto rebalance tablet leaders across TServers
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Yuqi Du has posted comments on this change. ( http://gerrit.cloudera.org:8080/18454 )
Change subject: [master] KUDU-3390 support auto rebalance tablet leaders across TServers
......................................................................
Patch Set 32:
(12 comments)
Thanks for your crs.
http://gerrit.cloudera.org:8080/#/c/18454/29//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18454/29//COMMIT_MSG@23
PS29, Line 23: detail infomations
> detailed information
Done
http://gerrit.cloudera.org:8080/#/c/18454/30//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18454/30//COMMIT_MSG@20
PS30, Line 20: the queue, pipeline, batc
> Maybe you need to explain what leads to an unbalanced load, I really don't
The reason I called it hidden variables is that I cann't expain it very clearly also.
The imbalance's result based my experience. Some facts can expain partially:
leader's write request conresponding to follower's UpdateConsensus request,
the two process is diffenent, such as theads and queue, pipeline and batch size .
My words may cause confuse, I will fix the words.
http://gerrit.cloudera.org:8080/#/c/18454/32//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18454/32//COMMIT_MSG@7
PS32, Line 7: auto rebalance tablet leaders
> I'm curious why this has started with that background task dong the rebalan
users can use kudu CLI leader_step_down command and write a script program to rebalance the leaders. SREs should make the rebalance script run periodically.
Our company have more than 1500+ kudu clusters and more and more kudu clusters will be deployed, so it's hard that SREs maintenance the rebalance script tasks. The better way is kudu leader rebalance itself.
And kudu has the 'auto-rebalancer' and has no 'auto-leader-rebalancer', so add 'auto-leader-rebalancer' is naturally.
1) Yes, I plan adding a new CLI tool to replace the old one, because it may conflict to the new 'auto-rebalancer', 'auto-rebalancer'. That need another discussion. The purpose of the CLI is for some special situation, eg when user turn off 'auto-leader-rebalancer' and need run it once. Because I did SRE' work before, I understand they have kinds of requirements.
2) auto-rebalancing task would do replicas rebalanced, then leader rebalancing task can reach the best effect.
I think 'auto-rebalancing' and 'auto-lead-rebalancing' both should turn on, but it's no matter who first turn on.
http://gerrit.cloudera.org:8080/#/c/18454/32//COMMIT_MSG@10
PS32, Line 10: adding
> adds
Done
http://gerrit.cloudera.org:8080/#/c/18454/32//COMMIT_MSG@10
PS32, Line 10: and
> Drop 'and'
Done
http://gerrit.cloudera.org:8080/#/c/18454/32//COMMIT_MSG@20
PS32, Line 20: may cause imbalanced load
> It would be nice to quantify this statement, adding measurement similar to
This is qualitative analysis, of course quantitative analysis is more better. But I have no direct evidence for this, no data for this. I can not tell you the proportion of write skew in imbalanced load.
I have found several reviewers referred to this, what do you suggest about this?
In order to avoid confused and not clearly statements, I should remove it, only keep the main reason?
http://gerrit.cloudera.org:8080/#/c/18454/32//COMMIT_MSG@23
PS32, Line 23: detail infomations
> details
Done
http://gerrit.cloudera.org:8080/#/c/18454/32//COMMIT_MSG@23
PS32, Line 23: (
> nit: add a space before this opening parenthesis
Done
http://gerrit.cloudera.org:8080/#/c/18454/30/src/kudu/master/auto_leader_rebalancer.cc
File src/kudu/master/auto_leader_rebalancer.cc:
http://gerrit.cloudera.org:8080/#/c/18454/30/src/kudu/master/auto_leader_rebalancer.cc@287
PS30, Line 287: moves_scheduled_this_round_for_test_ = leader_transfer_tasks.size();
: VLOG(1) << Substitute("leader rebalance tasks, size
> Besides, if the leadership of many tablets is changed, clients will send mo
OK
Add a flag for this.
http://gerrit.cloudera.org:8080/#/c/18454/30/src/kudu/master/auto_leader_rebalancer.cc@350
PS30, Line 350: {
> If there are some temporarily unavailable tservers in a cluster, is it a go
Simply, skip it and next leader rebalance can makeit rebalanced also.
Because the tserver is unsteady just now, delayed leader rebalance is ok.
If a kudu cluster is extreme unsteady, the problem is not the leader rebalance to solve.
http://gerrit.cloudera.org:8080/#/c/18454/30/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:
http://gerrit.cloudera.org:8080/#/c/18454/30/src/kudu/master/catalog_manager.cc@352
PS30, Line 352: auto_leader_rebalancing_enabled
> This flag should be tagged with 'runtime' too.
OK, done
http://gerrit.cloudera.org:8080/#/c/18454/30/src/kudu/master/catalog_manager.cc@1061
PS30, Line 1061:
: // Leader rebalancer depend on a good replicas balance, that means we'd better enable
: // auto_rebalancing. If auto_rebalancing is disabled and leader rebalance is enabled,
:
> I think it is worth pointing out the difference between rebalancing tablet
If auto_rebalancing is disabled and leader rebalance is enabled, the algorithm can also work, becase our algorithm is keeping a propotion of leaders/follows (1 : replication_refactor - 1) for every tserver' every table.
It's no matter to whether replicas balanced.
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 32
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.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: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Thu, 06 Oct 2022 04:03:09 +0000
Gerrit-HasComments: Yes
[kudu-CR] [master] KUDU-3390 support auto rebalance tablet leaders across TServers
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Yingchun Lai, Yifan Zhang, Kudu Jenkins, Andrew Wong, Abhishek Chennaka,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18454
to look at the new patch set (#33).
Change subject: [master] KUDU-3390 support auto rebalance tablet leaders across TServers
......................................................................
[master] KUDU-3390 support auto rebalance tablet leaders across TServers
The number of leader replicas per tablet server can become imbalanced
over time, which lead to load skew on some nodes. This patch adds
auto leader rebalance task to avoid leader replicas skew.
Two reasons of load skew:
- The main reason. Scan Requests has two modes: LeaderOnly(default) and CLOSEST_REPLICA.
For more accurate results, users will choose the LeaderOnly(default) mode.
Mostly, the scan load is positive correlation with leader numbers.
- The other reason. Write requests, leaders receive write requests and followers receive
UpdateConsensus request(raft called AppendEntries), the flow of processing is a little
different: the queue, pipeline, batch size, that may cause imbalanced load. Leader
rebalance will make leader and followers balanced and make service more stable.
An experiment present leader rebalance's effect (more details at KUDU-3390):
|leader ratio| scan cost | cpu usage | io
before|40: 0: 0 | 811.586 s | 47%, 18%, 19% | 102MB/s ioutil:55%, 8KB/s 2%, 64KB/s 3%
after |13: 14: 13 | 611.012 s | 39%, 45%, 35% | 53MB/s ioutil:31%, 80MB/s 18%, 45MB/s 24%
Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
---
M src/kudu/master/CMakeLists.txt
A src/kudu/master/auto_leader_rebalancer-test.cc
A src/kudu/master/auto_leader_rebalancer.cc
A src/kudu/master/auto_leader_rebalancer.h
M src/kudu/master/auto_rebalancer-test.cc
M src/kudu/master/auto_rebalancer.h
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
8 files changed, 835 insertions(+), 15 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18454/33
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 33
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.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: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
[kudu-CR] [master] KUDU-3061 support auto rebalance tablet leaders across TServers
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Yuqi Du has posted comments on this change. ( http://gerrit.cloudera.org:8080/18454 )
Change subject: [master] KUDU-3061 support auto rebalance tablet leaders across TServers
......................................................................
Patch Set 21:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/18454/19//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18454/19//COMMIT_MSG@14
PS19, Line 14: Two reasons of load skew:
> ok, I' ll do this at unit test.
It's not only the results of qualitative analysis, but also actual situation of the online services.
I'll collect some data to show this before the cr commit into repos.
http://gerrit.cloudera.org:8080/#/c/18454/19//COMMIT_MSG@21
PS19, Line 21: maybe cause imbalanced load
> Do you have any data to support this statement? How much imbalance we are
It's very hard to explain the number's difference of UpdateConsensus and writeRequest (UpdateConsensus can be batchWriteReuquest of merged writeRequest). So I say `maybe`, that's based experience.
I cann't show data to support it, so I decide to remove the worlds.
http://gerrit.cloudera.org:8080/#/c/18454/19//COMMIT_MSG@20
PS19, Line 20: flow of processing is a little different, which
: is hidden variables
> So, how much is that it total? Is that just minuscule amount or memory per
Above.
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 21
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: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Tue, 12 Jul 2022 07:04:50 +0000
Gerrit-HasComments: Yes
[kudu-CR] [master] KUDU-3061 support auto rebalance tablet leaders across TServers
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Yuqi Du has posted comments on this change. ( http://gerrit.cloudera.org:8080/18454 )
Change subject: [master] KUDU-3061 support auto rebalance tablet leaders across TServers
......................................................................
Patch Set 14:
(20 comments)
Thank you very much.
http://gerrit.cloudera.org:8080/#/c/18454/13/src/kudu/master/auto_leader_rebalancer.cc
File src/kudu/master/auto_leader_rebalancer.cc:
http://gerrit.cloudera.org:8080/#/c/18454/13/src/kudu/master/auto_leader_rebalancer.cc@92
PS13, Line 92: DCHECK(!thread_) << "AutoleaderRebalancerTask is already initialized";
> thread_ is not initialized after calling Init() when FLAGS_auto_leader_reba
To support enable/disable auto leader rebalancer task by changing the flag, I fix it again.
http://gerrit.cloudera.org:8080/#/c/18454/13/src/kudu/master/auto_leader_rebalancer.cc@93
PS13, Line 93: RETURN_NOT_OK(MessengerBuilder("auto-leader-rebalancer").Build(&messenger_));
> Is it needed to initialize messenger_ when FLAGS_auto_leader_rebalancing_en
fixed as above.
http://gerrit.cloudera.org:8080/#/c/18454/13/src/kudu/master/auto_leader_rebalancer.cc@116
PS13, Line 116: return Status::OK();
> How about rename to RunLeaderRebalanceForTable, or something like that?
Done
http://gerrit.cloudera.org:8080/#/c/18454/13/src/kudu/master/auto_leader_rebalancer.cc@117
PS13, Line 117:
> nit: can be omitted
Done
http://gerrit.cloudera.org:8080/#/c/18454/13/src/kudu/master/auto_leader_rebalancer.cc@117
PS13, Line 117:
> nit: same
Done
http://gerrit.cloudera.org:8080/#/c/18454/13/src/kudu/master/auto_leader_rebalancer.cc@136
PS13, Line 136:
> nit: can be omitted
Done
http://gerrit.cloudera.org:8080/#/c/18454/13/src/kudu/master/auto_leader_rebalancer.cc@165
PS13, Line 165: continue;
: }
:
: auto& uuid_replicas = LookupOrInsert(&uuid_replicas_map, ts_info.permanent_uuid(), {});
: uui
> can be simplified by:
Thanks a lot. You are very kind.
http://gerrit.cloudera.org:8080/#/c/18454/13/src/kudu/master/auto_leader_rebalancer.cc@174
PS13, Line 174: k the servers which number of leaders greater than 1/3 of number of all replicas
: // <uuid, number of replica, number of leader>
: map<string, std::pair<int32_t, int32_t>> tserver_statistics;
: // uuid->leader should transfer count
: map<str
> also can be simplified as above.
Done
http://gerrit.cloudera.org:8080/#/c/18454/13/src/kudu/master/auto_leader_rebalancer.cc@184
PS13, Line 184: (replica_count == 0) {
: // means no replicas (and no leaders), maybe a tserver joined kudu cluster just now, skip it
: continue;
: }
: tserver_statistics.insert({uuid, std::pair<int32_t, int32_t>(replica_count, leader_count)});
: VLOG(1) << Substitute(
:
> same
Done
http://gerrit.cloudera.org:8080/#/c/18454/13/src/kudu/master/auto_leader_rebalancer.cc@203
PS13, Line 203: int32_t expected_count = from_info.second;
: int32_t pick_count = 0;
: vector<string>& uuid_leaders = uuid_leaders_map[leader_uuid];
: std::shuffle(uuid_leaders.begin(), uuid_leaders.end(), random_generator_);
: // pick count leader, and pick dest uuid
: f
> FindWithDefault() in map-util.h may help.
Done
http://gerrit.cloudera.org:8080/#/c/18454/13/src/kudu/master/auto_leader_rebalancer.cc@209
PS13, Line 209: const string& tablet_id = uuid_leaders[i];
: vector<string> uuid_followers = follower_uuid_map[tablet_id];
:
: // TabletId leader transfer, pick a dest follower
: string dest_follower_uuid;
: if (uuid_followers.size() + 1 < table_data.num_replicas()) {
:
> use FindOrNull?
Done
http://gerrit.cloudera.org:8080/#/c/18454/13/src/kudu/master/auto_leader_rebalancer.cc@218
PS13, Line 218: for (int j = 0; j < uuid_followers.size(); j++) {
> I think use VLOG here wouble be more reasonable than depends on mode.
Good.
http://gerrit.cloudera.org:8080/#/c/18454/13/src/kudu/master/auto_leader_rebalancer.cc@234
PS13, Line 234:
> nit: can be omitted.
Done
http://gerrit.cloudera.org:8080/#/c/18454/13/src/kudu/master/auto_leader_rebalancer.cc@247
PS13, Line 247: tserv
> omit
Done
http://gerrit.cloudera.org:8080/#/c/18454/13/src/kudu/master/auto_leader_rebalancer.cc@359
PS13, Line 359: }
> How about add a metric for it?
Learned from auto_rebalance.cc,
The metric may be not useful for kudu users.
I study it later.
http://gerrit.cloudera.org:8080/#/c/18454/13/src/kudu/master/auto_leader_rebalancer.cc@366
PS13, Line 366:
> why use std::set? aren't the tserver uuids unique?
Yes, vector is ok. Not use find().
http://gerrit.cloudera.org:8080/#/c/18454/13/src/kudu/master/auto_leader_rebalancer.cc@390
PS13, Line 390:
> this log is helpless
Done
http://gerrit.cloudera.org:8080/#/c/18454/12/src/kudu/master/auto_rebalancer.cc
File src/kudu/master/auto_rebalancer.cc:
http://gerrit.cloudera.org:8080/#/c/18454/12/src/kudu/master/auto_rebalancer.cc@174
PS12, Line 174: [this]() { this->RunLoop(); }, &thread_);
> You can do it in the next patch, it not needed to modify it now, and you ca
ok
http://gerrit.cloudera.org:8080/#/c/18454/13/src/kudu/master/catalog_manager.h
File src/kudu/master/catalog_manager.h:
http://gerrit.cloudera.org:8080/#/c/18454/13/src/kudu/master/catalog_manager.h@771
PS13, Line 771: >* table
> nit: >* table
Done
http://gerrit.cloudera.org:8080/#/c/18454/13/src/kudu/master/catalog_manager.h@776
PS13, Line 776: >* table
> nit: >* table
Done
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 14
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: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Mon, 27 Jun 2022 07:28:01 +0000
Gerrit-HasComments: Yes
[kudu-CR] [master] KUDU-3061 support auto rebalance tablet leaders across TServers
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Yingchun Lai, Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18454
to look at the new patch set (#15).
Change subject: [master] KUDU-3061 support auto rebalance tablet leaders across TServers
......................................................................
[master] KUDU-3061 support auto rebalance tablet leaders across TServers
The number of leader replicas per tablet server can become imbalanced
over time, which lead to load skew on some nodes.
Adding an auto leader rebalance task to avoid leader replicas skew.
Two reasons of load skew:
1. The main reason. Scan Requests has two modes: LeaderOnly(default) and CLOSEST_REPLICA.
For more accurate results, users will choose the LeaderOnly(default) mode.
Mostly, the scan load is positive correlation with leader numbers.
2. The other reason. Write requests, leaders receive write requests and followers receive
appendEntries(kudu is UpdateConsensus), the flow of processing is a little different, which
is hidden variables, maybe cause imbalanced load. Leader rebalance will make leader and
followers balanced and eliminate hidden variables and make service more stable.
Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
---
M src/kudu/client/flex_partitioning_client-test.cc
M src/kudu/master/CMakeLists.txt
A src/kudu/master/auto_leader_rebalancer-test.cc
A src/kudu/master/auto_leader_rebalancer.cc
A src/kudu/master/auto_leader_rebalancer.h
M src/kudu/master/auto_rebalancer-test.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_path_handlers.cc
11 files changed, 792 insertions(+), 23 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18454/15
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
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: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
[kudu-CR] [master] KUDU-3390 support auto rebalance tablet leaders across TServers
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Yuqi Du has posted comments on this change. ( http://gerrit.cloudera.org:8080/18454 )
Change subject: [master] KUDU-3390 support auto rebalance tablet leaders across TServers
......................................................................
Patch Set 38: Code-Review+1
(2 comments)
Thanks for you advices.
http://gerrit.cloudera.org:8080/#/c/18454/35/src/kudu/master/auto_leader_rebalancer.cc
File src/kudu/master/auto_leader_rebalancer.cc:
http://gerrit.cloudera.org:8080/#/c/18454/35/src/kudu/master/auto_leader_rebalancer.cc@167
PS35, Line 167: {});
> I meant the last parameter, it's similar to line 173.
Thank you for you review. I'll study it and united as one.
My fault, '{}' is ok. Fixed.
http://gerrit.cloudera.org:8080/#/c/18454/35/src/kudu/master/auto_leader_rebalancer.cc@295
PS35, Line 295:
> That's OK, I see the same judgement in line 273.
I am sorry I write '??', In fact I fixed it.
The reply is confusion.
Maybe you say L269 at the base? or base 37 L273 ?
If L273 base 37. L281 and L273 is the same judgement. That's just for break the loop. Of course, use goto label is alternative method, but that's a more vulnerable method for reviewers.
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 38
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.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: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Mon, 14 Nov 2022 06:55:47 +0000
Gerrit-HasComments: Yes
[kudu-CR] [master] KUDU-3061 support auto rebalance tablet leaders across TServers
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Yingchun Lai, Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18454
to look at the new patch set (#12).
Change subject: [master] KUDU-3061 support auto rebalance tablet leaders across TServers
......................................................................
[master] KUDU-3061 support auto rebalance tablet leaders across TServers
The number of leader replicas per tablet server can become imbalanced
over time, which lead to load skew on some nodes.
Adding an auto leader rebalance task to avoid leader replicas skew.
Two reasons of load skew:
1. The main reason. Scan Requests has two modes: LeaderOnly(default) and CLOSEST_REPLICA.
For more accurate results, users will choose the LeaderOnly(default) mode.
Mostly, the scan load is positive correlation with leader numbers.
2. The other reason. Write requests, leaders receive write requests and followers receive
appendEntries(kudu is UpdateConsensus), the flow of processing is a little different, which
is hidden variables, maybe cause imbalanced load. Leader rebalance will make leader and
followers balanced and eliminate hidden variables and make service more stable.
Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
---
M src/kudu/client/flex_partitioning_client-test.cc
M src/kudu/master/CMakeLists.txt
A src/kudu/master/auto_leader_rebalancer-test.cc
A src/kudu/master/auto_leader_rebalancer.cc
A src/kudu/master/auto_leader_rebalancer.h
M src/kudu/master/auto_rebalancer-test.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_path_handlers.cc
11 files changed, 875 insertions(+), 42 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18454/12
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 12
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: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
[kudu-CR] [master] KUDU-3061 support auto rebalance tablet leaders across TServers
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/18454
to look at the new patch set (#5).
Change subject: [master] KUDU-3061 support auto rebalance tablet leaders across TServers
......................................................................
[master] KUDU-3061 support auto rebalance tablet leaders across TServers
The number of leader tablets per tablet server can become imbalanced
over time, putting additional pressure on a few nodes.
A CLI tool or an extension to the existing balancer should be added to
take care of this.
Currently the only option is running leader_step_down manually.
TODO:
1. adding unit test, use TimerThread KUDU-3364 to refact codes.
2. Add a leader rebalance CLI tool.
Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
---
M src/kudu/master/CMakeLists.txt
A src/kudu/master/auto_leader_rebalancer-test.cc
A src/kudu/master/auto_leader_rebalancer.cc
A src/kudu/master/auto_leader_rebalancer.h
M src/kudu/master/auto_rebalancer-test.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
9 files changed, 863 insertions(+), 30 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18454/5
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
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] KUDU-3061 support auto rebalance tablet leaders across TServers
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/18454
to look at the new patch set (#2).
Change subject: [master] KUDU-3061 support auto rebalance tablet leaders across TServers
......................................................................
[master] KUDU-3061 support auto rebalance tablet leaders across TServers
The number of leader tablets per tablet server can become imbalanced
over time, putting additional pressure on a few nodes.
A CLI tool or an extension to the existing balancer should be added to
take care of this.
Currently the only option is running leader_step_down manually.
TODO:
1. adding unit test, use TimerThread KUDU-3364 to refact codes.
2. Add a leader rebalance CLI tool.
Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
---
M src/kudu/master/CMakeLists.txt
A src/kudu/master/auto_leader_rebalancer-test.cc
A src/kudu/master/auto_leader_rebalancer.cc
A src/kudu/master/auto_leader_rebalancer.h
M src/kudu/master/auto_rebalancer-test.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
9 files changed, 944 insertions(+), 30 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18454/2
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 2
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] [master] KUDU-3390 support auto rebalance tablet leaders across TServers
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Yingchun Lai, Yifan Zhang, Kudu Jenkins, Andrew Wong, Abhishek Chennaka,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18454
to look at the new patch set (#24).
Change subject: [master] KUDU-3390 support auto rebalance tablet leaders across TServers
......................................................................
[master] KUDU-3390 support auto rebalance tablet leaders across TServers
The number of leader replicas per tablet server can become imbalanced
over time, which lead to load skew on some nodes. This patch adding and
auto leader rebalance task to avoid leader replicas skew.
Two reasons of load skew:
- The main reason. Scan Requests has two modes: LeaderOnly(default) and CLOSEST_REPLICA.
For more accurate results, users will choose the LeaderOnly(default) mode.
Mostly, the scan load is positive correlation with leader numbers.
- The other reason. Write requests, leaders receive write requests and followers receive
UpdateConsensus request(raft called AppendEntries), the flow of processing is a little
different, which is hidden variables, maybe cause imbalanced load. Leader rebalance will
make leader and followers balanced and eliminate hidden variables and make service more stable.
Leader rebalance's income, an experiment's some result, more detail infos at KUDU-3390"
|leader ratio| scan cost | cpu usage | io
before|40: 0: 0 | 811.586 s | 47%, 18%, 19% | 102MB/s ioutil:55%, 8KB/s 2%, 64KB/s 3%
after |13: 14: 13 | 611.012 s | 39%, 45%, 35% | 53MB/s ioutil:31%, 80MB/s 18%, 45MB/s 24%
Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
---
M src/kudu/master/CMakeLists.txt
A src/kudu/master/auto_leader_rebalancer-test.cc
A src/kudu/master/auto_leader_rebalancer.cc
A src/kudu/master/auto_leader_rebalancer.h
M src/kudu/master/auto_rebalancer-test.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
9 files changed, 794 insertions(+), 12 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18454/24
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 24
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.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: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
[kudu-CR] [master] KUDU-3390 support auto rebalance tablet leaders across TServers
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Yuqi Du has posted comments on this change. ( http://gerrit.cloudera.org:8080/18454 )
Change subject: [master] KUDU-3390 support auto rebalance tablet leaders across TServers
......................................................................
Patch Set 24:
(4 comments)
Sorry for missing some crs.
http://gerrit.cloudera.org:8080/#/c/18454/12/src/kudu/master/auto_leader_rebalancer.cc
File src/kudu/master/auto_leader_rebalancer.cc:
http://gerrit.cloudera.org:8080/#/c/18454/12/src/kudu/master/auto_leader_rebalancer.cc@165
PS12, Line 165:
:
: auto& uuid_replicas = LookupOrInsert(&uuid_replicas_map, ts_info.permanent_uuid(), {});
: uuid_replicas.emplace_back(tablet->id());
: }
: }
> Changed it, and I'll study it later.
Done
http://gerrit.cloudera.org:8080/#/c/18454/12/src/kudu/master/auto_leader_rebalancer.cc@175
PS12, Line 175: ring, std::pair<int32_t, int32_t>> tserver_statistics;
: // uuid->leader should transfer count
: map<string, int32_t> leader_transfer_source;
: for (const auto& uuid : tserver_uuids) {
: const vector<string>& replicas = FindWithDefault(uuid_replicas_map, uuid, {});
: uint3
> Same
Done
http://gerrit.cloudera.org:8080/#/c/18454/12/src/kudu/master/auto_leader_rebalancer.cc@186
PS12, Line 186: nt32_t leader_count = leaders.size();
: tserver_statistics.insert({uuid, std::pair<int32_t, int32_t>(replica_count, leader_count)});
: VLOG(1) << Substitute(
: "uuid: $0, replica_count: $1, leader_count: $2", uuid, replica_count, leader_count);
:
: // Our target is every tserver' replicas, number of leader : number of follower is
: //
> Same
Done
http://gerrit.cloudera.org:8080/#/c/18454/12/src/kudu/master/auto_leader_rebalancer.cc@323
PS12, Line 323: return Status::OK();
> There are several code can be optimized by util functions in src/kudu/gutil
Done
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 24
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.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: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Fri, 19 Aug 2022 10:10:33 +0000
Gerrit-HasComments: Yes
[kudu-CR] [master] KUDU-3061 support auto rebalance tablet leaders across TServers
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Yuqi Du has posted comments on this change. ( http://gerrit.cloudera.org:8080/18454 )
Change subject: [master] KUDU-3061 support auto rebalance tablet leaders across TServers
......................................................................
Patch Set 3:
(9 comments)
http://gerrit.cloudera.org:8080/#/c/18454/1/src/kudu/master/auto_leader_rebalancer-test.cc
File src/kudu/master/auto_leader_rebalancer-test.cc:
http://gerrit.cloudera.org:8080/#/c/18454/1/src/kudu/master/auto_leader_rebalancer-test.cc@57
PS1, Line 57: using std::map;
> warning: using decl 'GetTableLocations' is unused [misc-unused-using-decls]
Done
http://gerrit.cloudera.org:8080/#/c/18454/1/src/kudu/master/auto_leader_rebalancer-test.cc@58
PS1, Line 58: using std::set;
> warning: using decl 'ListTabletServers' is unused [misc-unused-using-decls]
Done
http://gerrit.cloudera.org:8080/#/c/18454/1/src/kudu/master/auto_leader_rebalancer-test.cc@59
PS1, Line 59: using std::string;
> warning: using decl 'GetTableLocationsResponsePB' is unused [misc-unused-us
Done
http://gerrit.cloudera.org:8080/#/c/18454/1/src/kudu/master/auto_leader_rebalancer-test.cc@60
PS1, Line 60: using std::unique_ptr;
> warning: using decl 'TSDescriptor' is unused [misc-unused-using-decls]
Done
http://gerrit.cloudera.org:8080/#/c/18454/1/src/kudu/master/auto_leader_rebalancer-test.cc@65
PS1, Line 65: DECLARE_int32(consensus_inject_latency_ms_in_notifications);
> warning: using decl 'unordered_map' is unused [misc-unused-using-decls]
Done
http://gerrit.cloudera.org:8080/#/c/18454/1/src/kudu/master/auto_leader_rebalancer-test.cc@66
PS1, Line 66: DECLARE_int32(follower_unavailable_considered_failed_sec);
> warning: using decl 'unordered_set' is unused [misc-unused-using-decls]
Done
http://gerrit.cloudera.org:8080/#/c/18454/1/src/kudu/master/auto_leader_rebalancer-test.cc@68
PS1, Line 68: DECLARE_int32(tablet_copy_download_file_inject_latency_ms);
> warning: using decl 'Substitute' is unused [misc-unused-using-decls]
Done
http://gerrit.cloudera.org:8080/#/c/18454/1/src/kudu/master/auto_leader_rebalancer.cc
File src/kudu/master/auto_leader_rebalancer.cc:
http://gerrit.cloudera.org:8080/#/c/18454/1/src/kudu/master/auto_leader_rebalancer.cc@246
PS1, Line 246: vector<string> uuid_followers = follower_uuid_map[tablet_id];
> warning: do not use 'else' after 'continue' [readability-else-after-return]
Done
http://gerrit.cloudera.org:8080/#/c/18454/1/src/kudu/master/auto_leader_rebalancer.cc@274
PS1, Line 274: int32_t leader_count = pair.second;
> warning: do not use 'else' after 'continue' [readability-else-after-return]
Done
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 3
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: Thu, 05 May 2022 11:58:35 +0000
Gerrit-HasComments: Yes
[kudu-CR] [master] KUDU-3061 support auto rebalance tablet leaders across TServers
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/18454
to look at the new patch set (#4).
Change subject: [master] KUDU-3061 support auto rebalance tablet leaders across TServers
......................................................................
[master] KUDU-3061 support auto rebalance tablet leaders across TServers
The number of leader tablets per tablet server can become imbalanced
over time, putting additional pressure on a few nodes.
A CLI tool or an extension to the existing balancer should be added to
take care of this.
Currently the only option is running leader_step_down manually.
TODO:
1. adding unit test, use TimerThread KUDU-3364 to refact codes.
2. Add a leader rebalance CLI tool.
Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
---
M src/kudu/master/CMakeLists.txt
A src/kudu/master/auto_leader_rebalancer-test.cc
A src/kudu/master/auto_leader_rebalancer.cc
A src/kudu/master/auto_leader_rebalancer.h
M src/kudu/master/auto_rebalancer-test.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
9 files changed, 874 insertions(+), 30 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18454/4
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
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>
[kudu-CR] [master] KUDU-3390 support auto rebalance tablet leaders across TServers
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/18454 )
Change subject: [master] KUDU-3390 support auto rebalance tablet leaders across TServers
......................................................................
Patch Set 29:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/18454/29/src/kudu/master/auto_leader_rebalancer.cc
File src/kudu/master/auto_leader_rebalancer.cc:
http://gerrit.cloudera.org:8080/#/c/18454/29/src/kudu/master/auto_leader_rebalancer.cc@107
PS29, Line 107: Status AutoLeaderRebalancerTask::RunLeaderRebalanceForTable(
Logs in this function add the table name to distinguish different tables are operated.
http://gerrit.cloudera.org:8080/#/c/18454/29/src/kudu/master/auto_leader_rebalancer.cc@157
PS29, Line 157: auto& leader_uuids = LookupOrInsert(&uuid_leaders_map, uuid, vector<string>());
: leader_uuids.emplace_back(tablet->id());
nit: InsertOrDie(&uuid_leaders_map, uuid, vector<string>(tablet->id())); ?
http://gerrit.cloudera.org:8080/#/c/18454/29/src/kudu/master/auto_leader_rebalancer.cc@159
PS29, Line 159: leader_uuid_map.insert({tablet->id(), uuid});
: leader_uuid_host_port_map.insert({uuid, HostPortFromPB(ts_info.rpc_addresses(0))});
nit: Recommand to use InsertOrDie
http://gerrit.cloudera.org:8080/#/c/18454/29/src/kudu/master/auto_leader_rebalancer.cc@165
PS29, Line 165: VLOG(0) << Substitute("Not a VOTER, role: $0", RaftPeerPB::Role_Name(r.role()));
Maybe it's a warning or even fatal?
http://gerrit.cloudera.org:8080/#/c/18454/29/src/kudu/master/auto_leader_rebalancer.cc@181
PS29, Line 181: uint32_t replica_count = 0;
: auto iter1 = uuid_replicas_map.find(uuid);
: if (iter1 != uuid_replicas_map.end()) {
: replica_count = iter1->second.size();
: }
nit: FindWithDefault
http://gerrit.cloudera.org:8080/#/c/18454/29/src/kudu/master/auto_leader_rebalancer.cc@190
PS29, Line 190: uint32_t leader_count = 0;
: auto iter2 = uuid_leaders_map.find(uuid);
: if (iter2 != uuid_leaders_map.end()) {
: leader_count = iter2->second.size();
: }
nit: FindWithDefault
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 29
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.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: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Sat, 24 Sep 2022 17:20:57 +0000
Gerrit-HasComments: Yes
[kudu-CR] [master] KUDU-3390 support auto rebalance tablet leaders across TServers
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/18454 )
Change subject: [master] KUDU-3390 support auto rebalance tablet leaders across TServers
......................................................................
[master] KUDU-3390 support auto rebalance tablet leaders across TServers
The number of leader replicas per tablet server can become imbalanced
over time, which lead to load skew on some nodes. This patch adds
auto leader rebalance task to avoid leader replicas skew.
Two reasons of load skew:
- The main reason. Scan Requests has two modes: LeaderOnly(default) and CLOSEST_REPLICA.
For more accurate results, users will choose the LeaderOnly(default) mode.
Mostly, the scan load is positive correlation with leader numbers.
- The other reason. Write requests, leaders receive write requests and followers receive
UpdateConsensus request(raft called AppendEntries), the flow of processing is a little
different: the queue, pipeline, batch size, that may cause imbalanced load. Leader
rebalance will make leader and followers balanced and make service more stable.
An experiment present leader rebalance's effect (more details at KUDU-3390):
|leader ratio| scan cost | cpu usage | io
before|40: 0: 0 | 811.586 s | 47%, 18%, 19% | 102MB/s ioutil:55%, 8KB/s 2%, 64KB/s 3%
after |13: 14: 13 | 611.012 s | 39%, 45%, 35% | 53MB/s ioutil:31%, 80MB/s 18%, 45MB/s 24%
Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Reviewed-on: http://gerrit.cloudera.org:8080/18454
Reviewed-by: Yuqi Du <sh...@gmail.com>
Reviewed-by: Yingchun Lai <ac...@gmail.com>
Tested-by: Kudu Jenkins
---
M src/kudu/master/CMakeLists.txt
A src/kudu/master/auto_leader_rebalancer-test.cc
A src/kudu/master/auto_leader_rebalancer.cc
A src/kudu/master/auto_leader_rebalancer.h
M src/kudu/master/auto_rebalancer-test.cc
M src/kudu/master/auto_rebalancer.h
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
8 files changed, 836 insertions(+), 15 deletions(-)
Approvals:
Yuqi Du: Looks good to me, but someone else must approve
Yingchun Lai: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 40
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.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: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
[kudu-CR] [master] KUDU-3061 support auto rebalance tablet leaders across TServers
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Yuqi Du has posted comments on this change. ( http://gerrit.cloudera.org:8080/18454 )
Change subject: [master] KUDU-3061 support auto rebalance tablet leaders across TServers
......................................................................
Patch Set 15:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18454/14/src/kudu/master/auto_leader_rebalancer-test.cc
File src/kudu/master/auto_leader_rebalancer-test.cc:
http://gerrit.cloudera.org:8080/#/c/18454/14/src/kudu/master/auto_leader_rebalancer-test.cc@54
PS14, Line 54: using std::unique_ptr;
> warning: using decl 'set' is unused [misc-unused-using-decls]
Done
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
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: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Mon, 27 Jun 2022 08:13:12 +0000
Gerrit-HasComments: Yes
[kudu-CR] [master] KUDU-3061 support auto rebalance tablet leaders across TServers
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Yingchun Lai, Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18454
to look at the new patch set (#19).
Change subject: [master] KUDU-3061 support auto rebalance tablet leaders across TServers
......................................................................
[master] KUDU-3061 support auto rebalance tablet leaders across TServers
The number of leader replicas per tablet server can become imbalanced
over time, which lead to load skew on some nodes.
Adding an auto leader rebalance task to avoid leader replicas skew.
Two reasons of load skew:
1. The main reason. Scan Requests has two modes: LeaderOnly(default) and CLOSEST_REPLICA.
For more accurate results, users will choose the LeaderOnly(default) mode.
Mostly, the scan load is positive correlation with leader numbers.
2. The other reason. Write requests, leaders receive write requests and followers receive
appendEntries(kudu is UpdateConsensus), the flow of processing is a little different, which
is hidden variables, maybe cause imbalanced load. Leader rebalance will make leader and
followers balanced and eliminate hidden variables and make service more stable.
Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
---
M src/kudu/client/flex_partitioning_client-test.cc
M src/kudu/master/CMakeLists.txt
A src/kudu/master/auto_leader_rebalancer-test.cc
A src/kudu/master/auto_leader_rebalancer.cc
A src/kudu/master/auto_leader_rebalancer.h
M src/kudu/master/auto_rebalancer-test.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_path_handlers.cc
11 files changed, 792 insertions(+), 23 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18454/19
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 19
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: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
[kudu-CR] [master] KUDU-3061 support auto rebalance tablet leaders across TServers
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Yingchun Lai, Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18454
to look at the new patch set (#10).
Change subject: [master] KUDU-3061 support auto rebalance tablet leaders across TServers
......................................................................
[master] KUDU-3061 support auto rebalance tablet leaders across TServers
The number of leader replicas per tablet server can become imbalanced
over time, which lead to serve pressure skew on some nodes.
Adding an auto leader rebalance task to avoid leader replicas skew.
Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
---
M src/kudu/client/flex_partitioning_client-test.cc
M src/kudu/master/CMakeLists.txt
A src/kudu/master/auto_leader_rebalancer-test.cc
A src/kudu/master/auto_leader_rebalancer.cc
A src/kudu/master/auto_leader_rebalancer.h
M src/kudu/master/auto_rebalancer-test.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_path_handlers.cc
11 files changed, 875 insertions(+), 42 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18454/10
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
Gerrit-PatchSet: 10
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: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
[kudu-CR] [master] KUDU-3061 support auto rebalance tablet leaders across TServers
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Yuqi Du has posted comments on this change. ( http://gerrit.cloudera.org:8080/18454 )
Change subject: [master] KUDU-3061 support auto rebalance tablet leaders across TServers
......................................................................
Patch Set 5:
(9 comments)
http://gerrit.cloudera.org:8080/#/c/18454/3/src/kudu/master/auto_leader_rebalancer-test.cc
File src/kudu/master/auto_leader_rebalancer-test.cc:
http://gerrit.cloudera.org:8080/#/c/18454/3/src/kudu/master/auto_leader_rebalancer-test.cc@56
PS3, Line 56: DECLARE_int32(tablet_copy_download_file_inject_latency_ms);
> warning: using decl 'RaftPeerPB' is unused [misc-unused-using-decls]
Done
http://gerrit.cloudera.org:8080/#/c/18454/3/src/kudu/master/auto_leader_rebalancer-test.cc@57
PS3, Line 57: DECLARE_int32(tserver_unresponsive_timeout_ms);
> warning: using decl 'map' is unused [misc-unused-using-decls]
Done
http://gerrit.cloudera.org:8080/#/c/18454/3/src/kudu/master/auto_leader_rebalancer-test.cc@61
PS3, Line 61: DECLARE_uint32(auto_rebalancing_wait_for_replica_moves_seconds);
> warning: using decl 'vector' is unused [misc-unused-using-decls]
Done
http://gerrit.cloudera.org:8080/#/c/18454/4/src/kudu/master/auto_leader_rebalancer-test.cc
File src/kudu/master/auto_leader_rebalancer-test.cc:
http://gerrit.cloudera.org:8080/#/c/18454/4/src/kudu/master/auto_leader_rebalancer-test.cc@55
PS4, Line 55: DECLARE_int32(raft_heartbeat_interval_ms);
> warning: using decl 'RaftPeerPB' is unused [misc-unused-using-decls]
Done
http://gerrit.cloudera.org:8080/#/c/18454/4/src/kudu/master/auto_leader_rebalancer-test.cc@56
PS4, Line 56: DECLARE_int32(tablet_copy_download_file_inject_latency_ms);
> warning: using decl 'map' is unused [misc-unused-using-decls]
Done
http://gerrit.cloudera.org:8080/#/c/18454/4/src/kudu/master/auto_leader_rebalancer-test.cc@60
PS4, Line 60: DECLARE_uint32(auto_rebalancing_max_moves_per_server);
> warning: using decl 'vector' is unused [misc-unused-using-decls]
Done
http://gerrit.cloudera.org:8080/#/c/18454/3/src/kudu/master/auto_leader_rebalancer.cc
File src/kudu/master/auto_leader_rebalancer.cc:
http://gerrit.cloudera.org:8080/#/c/18454/3/src/kudu/master/auto_leader_rebalancer.cc@293
PS3, Line 293: if (!leader_transfer_tasks.empty()) {
> warning: the 'empty' method should be used to check for emptiness instead o
Done
http://gerrit.cloudera.org:8080/#/c/18454/3/src/kudu/master/auto_leader_rebalancer.cc@293
PS3, Line 293: if (!leader_transfer_tasks.empty()) {
> warning: the 'empty' method should be used to check for emptiness instead o
Done
http://gerrit.cloudera.org:8080/#/c/18454/4/src/kudu/master/auto_leader_rebalancer.cc
File src/kudu/master/auto_leader_rebalancer.cc:
http://gerrit.cloudera.org:8080/#/c/18454/4/src/kudu/master/auto_leader_rebalancer.cc@293
PS4, Line 293: if (!leader_transfer_tasks.empty()) {
> warning: the 'empty' method should be used to check for emptiness instead o
Done
--
To view, visit http://gerrit.cloudera.org:8080/18454
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibfb60d8759a93b6a19238637c27df4f6b1cac918
Gerrit-Change-Number: 18454
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: Fri, 06 May 2022 05:12:50 +0000
Gerrit-HasComments: Yes