You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Alexey Serbin (Code Review)" <ge...@cloudera.org> on 2018/10/11 21:57:54 UTC
[kudu-CR] WIP [rebalancer] location-aware rebalancer (part 2/3)
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11662
Change subject: WIP [rebalancer] location-aware rebalancer (part 2/3)
......................................................................
WIP [rebalancer] location-aware rebalancer (part 2/3)
Added location load balancing algorithm and corresponding unit tests.
WIP: address TODOs and add more tests
Change-Id: Ia4b0ccb3067a42f112e117b31f22d510f96e5234
---
M src/kudu/integration-tests/ts_itest-base.h
M src/kudu/tools/placement_policy_util-test.cc
M src/kudu/tools/placement_policy_util.cc
M src/kudu/tools/placement_policy_util.h
M src/kudu/tools/rebalance-test.cc
M src/kudu/tools/rebalance_algo-test.cc
M src/kudu/tools/rebalance_algo.cc
M src/kudu/tools/rebalance_algo.h
M src/kudu/tools/rebalancer.cc
M src/kudu/tools/rebalancer.h
M src/kudu/tools/rebalancer_tool-test.cc
M src/kudu/tools/tool_action_cluster.cc
M src/kudu/tools/tool_replica_util.cc
M src/kudu/tools/tool_replica_util.h
14 files changed, 1,727 insertions(+), 249 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/62/11662/1
--
To view, visit http://gerrit.cloudera.org:8080/11662
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia4b0ccb3067a42f112e117b31f22d510f96e5234
Gerrit-Change-Number: 11662
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
[kudu-CR] [rebalancer] location-aware rebalancer (part 2/3)
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11662 )
Change subject: [rebalancer] location-aware rebalancer (part 2/3)
......................................................................
Patch Set 3:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/11662/1/src/kudu/tools/rebalancer.cc
File src/kudu/tools/rebalancer.cc:
http://gerrit.cloudera.org:8080/#/c/11662/1/src/kudu/tools/rebalancer.cc@1268
PS1, Line 1268: bool Rebalancer::PolicyFixer::ScheduleNextMove(bool* has_errors,
> warning: both sides of overloaded operator are equivalent [misc-redundant-e
Done
http://gerrit.cloudera.org:8080/#/c/11662/3/src/kudu/tools/rebalancer.cc
File src/kudu/tools/rebalancer.cc:
http://gerrit.cloudera.org:8080/#/c/11662/3/src/kudu/tools/rebalancer.cc@104
PS3, Line 104: Rebalancer::Rebalancer(const Config& config)
> warning: pass by value and use std::move [modernize-pass-by-value]
I'm going to ignore this suggestion from tidy for a while.
--
To view, visit http://gerrit.cloudera.org:8080/11662
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia4b0ccb3067a42f112e117b31f22d510f96e5234
Gerrit-Change-Number: 11662
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Mon, 15 Oct 2018 21:40:22 +0000
Gerrit-HasComments: Yes
[kudu-CR] [rebalancer] location-aware rebalancer (part 2/3)
Posted by "Alexey Serbin (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/11662
to look at the new patch set (#3).
Change subject: [rebalancer] location-aware rebalancer (part 2/3)
......................................................................
[rebalancer] location-aware rebalancer (part 2/3)
Added location load balancing algorithm and corresponding unit tests.
Added one integration test as well, but it's disabled for a now since
it requires a tablet server's location to be reported in
KsckServerHealthSummary. If that piece is in place, the tests passes.
More integration tests will be added in a follow-up commit.
Change-Id: Ia4b0ccb3067a42f112e117b31f22d510f96e5234
---
M src/kudu/tools/rebalance-test.cc
M src/kudu/tools/rebalance_algo-test.cc
M src/kudu/tools/rebalance_algo.cc
M src/kudu/tools/rebalance_algo.h
M src/kudu/tools/rebalancer.cc
M src/kudu/tools/rebalancer.h
M src/kudu/tools/rebalancer_tool-test.cc
M src/kudu/tools/tool_action_cluster.cc
M src/kudu/tools/tool_replica_util.cc
M src/kudu/tools/tool_replica_util.h
10 files changed, 2,039 insertions(+), 519 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/62/11662/3
--
To view, visit http://gerrit.cloudera.org:8080/11662
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia4b0ccb3067a42f112e117b31f22d510f96e5234
Gerrit-Change-Number: 11662
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
[kudu-CR] [rebalancer] location-aware rebalancer (part 2/3)
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/11662 )
Change subject: [rebalancer] location-aware rebalancer (part 2/3)
......................................................................
Patch Set 4:
(15 comments)
It looks pretty good to me but it's hard to thoroughly review this much code at once. I know things are still evolving a bit so I'll take a close look at a later changelist too.
http://gerrit.cloudera.org:8080/#/c/11662/4/src/kudu/tools/rebalance_algo-test.cc
File src/kudu/tools/rebalance_algo-test.cc:
http://gerrit.cloudera.org:8080/#/c/11662/4/src/kudu/tools/rebalance_algo-test.cc@148
PS4, Line 148: balance.servers_by_total_replica_count.emplace(
: count, tcc.tserver_uuids[tserver_idx]);
Use EmplaceOrDie.
http://gerrit.cloudera.org:8080/#/c/11662/4/src/kudu/tools/rebalance_algo-test.cc@282
PS4, Line 282: decltype(TestClusterConfig::servers_by_location){},
I think it'd be clearer to assign this quantity to a local variable like 'empty_location_assignment' and then pass the variable to each test case.
http://gerrit.cloudera.org:8080/#/c/11662/4/src/kudu/tools/rebalance_algo.h
File src/kudu/tools/rebalance_algo.h:
http://gerrit.cloudera.org:8080/#/c/11662/4/src/kudu/tools/rebalance_algo.h@123
PS4, Line 123: ,
and
http://gerrit.cloudera.org:8080/#/c/11662/4/src/kudu/tools/rebalance_algo.h@253
PS4, Line 253: LocationBalancingAlgo() = default;
Is this needed?
http://gerrit.cloudera.org:8080/#/c/11662/4/src/kudu/tools/rebalance_algo.h@264
PS4, Line 264: FindBestMove
This could use a comment since it's hard to know what it does from the signature.
http://gerrit.cloudera.org:8080/#/c/11662/4/src/kudu/tools/rebalance_algo.cc
File src/kudu/tools/rebalance_algo.cc:
http://gerrit.cloudera.org:8080/#/c/11662/4/src/kudu/tools/rebalance_algo.cc@192
PS4, Line 192: std::swap(*balance_info, info);
Should we use move assignment here?
http://gerrit.cloudera.org:8080/#/c/11662/4/src/kudu/tools/rebalance_algo.cc@407
PS4, Line 407: // Work on the most location-wise unbalanced tables first.
This comment feels out of context. Maybe it needs to be expanded, or possible removed?
http://gerrit.cloudera.org:8080/#/c/11662/4/src/kudu/tools/rebalance_algo.cc@414
PS4, Line 414: move some of it into the CluterBalanceInfo structure as is
Even though it's your TODO one day someone else might implement it...could you flesh out what you mean by 'it'?
http://gerrit.cloudera.org:8080/#/c/11662/4/src/kudu/tools/rebalance_algo.cc@430
PS4, Line 430: multimap<double
How does this work if NaN is a key? That ought to never happen, so maybe we ought to CHECK for that when we insert.
http://gerrit.cloudera.org:8080/#/c/11662/4/src/kudu/tools/rebalance_algo.cc@553
PS4, Line 553: avaiable
available
http://gerrit.cloudera.org:8080/#/c/11662/4/src/kudu/tools/rebalance_algo.cc@566
PS4, Line 566: withtin
within
http://gerrit.cloudera.org:8080/#/c/11662/4/src/kudu/tools/rebalance_algo.cc@570
PS4, Line 570: about replica which movement would
: // violate the constraint of not having the majority of
: // replicas at one location
This part of the sentence kind of doesn't make sense.
http://gerrit.cloudera.org:8080/#/c/11662/4/src/kudu/tools/rebalancer.h
File src/kudu/tools/rebalancer.h:
http://gerrit.cloudera.org:8080/#/c/11662/4/src/kudu/tools/rebalancer.h@160
PS4, Line 160: virtual ~Runner() = default;
Is this needed?
http://gerrit.cloudera.org:8080/#/c/11662/4/src/kudu/tools/rebalancer.h@209
PS4, Line 209: protected:
Is there a defined order for public, protected, private? I would expected it to be public, then protected, then private.
http://gerrit.cloudera.org:8080/#/c/11662/4/src/kudu/tools/tool_replica_util.cc
File src/kudu/tools/tool_replica_util.cc:
http://gerrit.cloudera.org:8080/#/c/11662/4/src/kudu/tools/tool_replica_util.cc@417
PS4, Line 417: if (cas_opid_idx) {
: req.set_cas_config_opid_index(*cas_opid_idx);
: }
It'd be nice to combine this with L411.
--
To view, visit http://gerrit.cloudera.org:8080/11662
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia4b0ccb3067a42f112e117b31f22d510f96e5234
Gerrit-Change-Number: 11662
Gerrit-PatchSet: 4
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Thu, 18 Oct 2018 17:52:46 +0000
Gerrit-HasComments: Yes
[kudu-CR] [rebalancer] location-aware rebalancer (part 2/3)
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Will Berkeley, Tidy Bot, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11662
to look at the new patch set (#4).
Change subject: [rebalancer] location-aware rebalancer (part 2/3)
......................................................................
[rebalancer] location-aware rebalancer (part 2/3)
Added location load balancing algorithm and corresponding unit tests.
Added one integration test as well, but it's disabled for a now since
it requires a tablet server's location to be reported in
KsckServerHealthSummary. If that piece is in place, the test passes.
More integration tests will be added in a follow-up commit.
Change-Id: Ia4b0ccb3067a42f112e117b31f22d510f96e5234
---
M src/kudu/tools/rebalance-test.cc
M src/kudu/tools/rebalance_algo-test.cc
M src/kudu/tools/rebalance_algo.cc
M src/kudu/tools/rebalance_algo.h
M src/kudu/tools/rebalancer.cc
M src/kudu/tools/rebalancer.h
M src/kudu/tools/rebalancer_tool-test.cc
M src/kudu/tools/tool_action_cluster.cc
M src/kudu/tools/tool_replica_util.cc
M src/kudu/tools/tool_replica_util.h
10 files changed, 2,069 insertions(+), 521 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/62/11662/4
--
To view, visit http://gerrit.cloudera.org:8080/11662
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia4b0ccb3067a42f112e117b31f22d510f96e5234
Gerrit-Change-Number: 11662
Gerrit-PatchSet: 4
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] [rebalancer] location-aware rebalancer (part 2/3)
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Will Berkeley, Tidy Bot, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11662
to look at the new patch set (#5).
Change subject: [rebalancer] location-aware rebalancer (part 2/3)
......................................................................
[rebalancer] location-aware rebalancer (part 2/3)
Added location load balancing algorithm and corresponding unit tests.
Added one integration test as well, but it's disabled for a now since
it requires a tablet server's location to be reported in
KsckServerHealthSummary. If that piece is in place, the test passes.
More integration tests will be added in a follow-up commit.
Change-Id: Ia4b0ccb3067a42f112e117b31f22d510f96e5234
---
M src/kudu/tools/rebalance-test.cc
M src/kudu/tools/rebalance_algo-test.cc
M src/kudu/tools/rebalance_algo.cc
M src/kudu/tools/rebalance_algo.h
M src/kudu/tools/rebalancer.cc
M src/kudu/tools/rebalancer.h
M src/kudu/tools/rebalancer_tool-test.cc
M src/kudu/tools/tool_action_cluster.cc
M src/kudu/tools/tool_replica_util.cc
M src/kudu/tools/tool_replica_util.h
10 files changed, 2,075 insertions(+), 522 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/62/11662/5
--
To view, visit http://gerrit.cloudera.org:8080/11662
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia4b0ccb3067a42f112e117b31f22d510f96e5234
Gerrit-Change-Number: 11662
Gerrit-PatchSet: 5
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] [rebalancer] location-aware rebalancer (part 2/3)
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Will Berkeley, Tidy Bot, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11662
to look at the new patch set (#7).
Change subject: [rebalancer] location-aware rebalancer (part 2/3)
......................................................................
[rebalancer] location-aware rebalancer (part 2/3)
Added location load balancing algorithm and corresponding unit tests.
Added one integration test as well, but it's disabled for a now since
it requires a tablet server's location to be reported in
KsckServerHealthSummary. If that piece is in place, the test passes.
More integration tests will be added in a follow-up commit.
Change-Id: Ia4b0ccb3067a42f112e117b31f22d510f96e5234
---
M src/kudu/tools/rebalance-test.cc
M src/kudu/tools/rebalance_algo-test.cc
M src/kudu/tools/rebalance_algo.cc
M src/kudu/tools/rebalance_algo.h
M src/kudu/tools/rebalancer.cc
M src/kudu/tools/rebalancer.h
M src/kudu/tools/rebalancer_tool-test.cc
M src/kudu/tools/tool_action_cluster.cc
M src/kudu/tools/tool_replica_util.cc
M src/kudu/tools/tool_replica_util.h
10 files changed, 2,073 insertions(+), 523 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/62/11662/7
--
To view, visit http://gerrit.cloudera.org:8080/11662
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia4b0ccb3067a42f112e117b31f22d510f96e5234
Gerrit-Change-Number: 11662
Gerrit-PatchSet: 7
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] [rebalancer] location-aware rebalancer (part 2/3)
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has abandoned this change. ( http://gerrit.cloudera.org:8080/11662 )
Change subject: [rebalancer] location-aware rebalancer (part 2/3)
......................................................................
Abandoned
Abandoned in favor of 11743-11748 patch series: that's a multi-patch version of the same code.
--
To view, visit http://gerrit.cloudera.org:8080/11662
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: Ia4b0ccb3067a42f112e117b31f22d510f96e5234
Gerrit-Change-Number: 11662
Gerrit-PatchSet: 7
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] [rebalancer] location-aware rebalancer (part 2/3)
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11662 )
Change subject: [rebalancer] location-aware rebalancer (part 2/3)
......................................................................
Patch Set 4:
(15 comments)
Thanks a lot for the review!
I'm working on splitting this patch into few.
http://gerrit.cloudera.org:8080/#/c/11662/4/src/kudu/tools/rebalance_algo-test.cc
File src/kudu/tools/rebalance_algo-test.cc:
http://gerrit.cloudera.org:8080/#/c/11662/4/src/kudu/tools/rebalance_algo-test.cc@148
PS4, Line 148: balance.servers_by_total_replica_count.emplace(
: count, tcc.tserver_uuids[tserver_idx]);
> Use EmplaceOrDie.
IIRC, that's std::multimap, so it cannot be anything but success when inserting any key, even a duplicate one.
http://gerrit.cloudera.org:8080/#/c/11662/4/src/kudu/tools/rebalance_algo-test.cc@282
PS4, Line 282: decltype(TestClusterConfig::servers_by_location){},
> I think it'd be clearer to assign this quantity to a local variable like 'e
That sounds like a good idea.
BTW, that ugliness appeared because of old libstdc++ on CentOS 6 and alike, otherwise that's just an empty map would suffice (i.e. {}).
http://gerrit.cloudera.org:8080/#/c/11662/4/src/kudu/tools/rebalance_algo.h
File src/kudu/tools/rebalance_algo.h:
http://gerrit.cloudera.org:8080/#/c/11662/4/src/kudu/tools/rebalance_algo.h@123
PS4, Line 123: ,
> and
Done
http://gerrit.cloudera.org:8080/#/c/11662/4/src/kudu/tools/rebalance_algo.h@253
PS4, Line 253: LocationBalancingAlgo() = default;
> Is this needed?
Nope, it's not.
http://gerrit.cloudera.org:8080/#/c/11662/4/src/kudu/tools/rebalance_algo.h@264
PS4, Line 264: FindBestMove
> This could use a comment since it's hard to know what it does from the sign
Done
http://gerrit.cloudera.org:8080/#/c/11662/4/src/kudu/tools/rebalance_algo.cc
File src/kudu/tools/rebalance_algo.cc:
http://gerrit.cloudera.org:8080/#/c/11662/4/src/kudu/tools/rebalance_algo.cc@192
PS4, Line 192: std::swap(*balance_info, info);
> Should we use move assignment here?
Done
http://gerrit.cloudera.org:8080/#/c/11662/4/src/kudu/tools/rebalance_algo.cc@407
PS4, Line 407: // Work on the most location-wise unbalanced tables first.
> This comment feels out of context. Maybe it needs to be expanded, or possib
Done
http://gerrit.cloudera.org:8080/#/c/11662/4/src/kudu/tools/rebalance_algo.cc@414
PS4, Line 414: move some of it into the CluterBalanceInfo structure as is
> Even though it's your TODO one day someone else might implement it...could
Done.
http://gerrit.cloudera.org:8080/#/c/11662/4/src/kudu/tools/rebalance_algo.cc@430
PS4, Line 430: multimap<double
> How does this work if NaN is a key? That ought to never happen, so maybe we
Done
http://gerrit.cloudera.org:8080/#/c/11662/4/src/kudu/tools/rebalance_algo.cc@553
PS4, Line 553: avaiable
> available
Done
http://gerrit.cloudera.org:8080/#/c/11662/4/src/kudu/tools/rebalance_algo.cc@566
PS4, Line 566: withtin
> within
Done
http://gerrit.cloudera.org:8080/#/c/11662/4/src/kudu/tools/rebalance_algo.cc@570
PS4, Line 570: about replica which movement would
: // violate the constraint of not having the majority of
: // replicas at one location
> This part of the sentence kind of doesn't make sense.
Done
http://gerrit.cloudera.org:8080/#/c/11662/4/src/kudu/tools/rebalancer.h
File src/kudu/tools/rebalancer.h:
http://gerrit.cloudera.org:8080/#/c/11662/4/src/kudu/tools/rebalancer.h@160
PS4, Line 160: virtual ~Runner() = default;
> Is this needed?
Having virtual destructor in the base class is a must when objects are passed by references/pointers to the base class, otherwise some surprises might happen because correct destructor of the derived class wont be called. Basically, destructors in that sense behave the same as regular virtual functions.
http://gerrit.cloudera.org:8080/#/c/11662/4/src/kudu/tools/rebalancer.h@209
PS4, Line 209: protected:
> Is there a defined order for public, protected, private? I would expected i
Yes, that's the order dictated by the style guide, IIUC. And in this case there is public and protected, but no private section.
http://gerrit.cloudera.org:8080/#/c/11662/4/src/kudu/tools/tool_replica_util.cc
File src/kudu/tools/tool_replica_util.cc:
http://gerrit.cloudera.org:8080/#/c/11662/4/src/kudu/tools/tool_replica_util.cc@417
PS4, Line 417: if (cas_opid_idx) {
: req.set_cas_config_opid_index(*cas_opid_idx);
: }
> It'd be nice to combine this with L411.
Woops. Good catch!
--
To view, visit http://gerrit.cloudera.org:8080/11662
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia4b0ccb3067a42f112e117b31f22d510f96e5234
Gerrit-Change-Number: 11662
Gerrit-PatchSet: 4
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Fri, 19 Oct 2018 14:24:24 +0000
Gerrit-HasComments: Yes
[kudu-CR] [rebalancer] location-aware rebalancer (part 2/3)
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Will Berkeley, Tidy Bot, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11662
to look at the new patch set (#6).
Change subject: [rebalancer] location-aware rebalancer (part 2/3)
......................................................................
[rebalancer] location-aware rebalancer (part 2/3)
Added location load balancing algorithm and corresponding unit tests.
Added one integration test as well, but it's disabled for a now since
it requires a tablet server's location to be reported in
KsckServerHealthSummary. If that piece is in place, the test passes.
More integration tests will be added in a follow-up commit.
Change-Id: Ia4b0ccb3067a42f112e117b31f22d510f96e5234
---
M src/kudu/tools/rebalance-test.cc
M src/kudu/tools/rebalance_algo-test.cc
M src/kudu/tools/rebalance_algo.cc
M src/kudu/tools/rebalance_algo.h
M src/kudu/tools/rebalancer.cc
M src/kudu/tools/rebalancer.h
M src/kudu/tools/rebalancer_tool-test.cc
M src/kudu/tools/tool_action_cluster.cc
M src/kudu/tools/tool_replica_util.cc
M src/kudu/tools/tool_replica_util.h
10 files changed, 2,074 insertions(+), 523 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/62/11662/6
--
To view, visit http://gerrit.cloudera.org:8080/11662
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia4b0ccb3067a42f112e117b31f22d510f96e5234
Gerrit-Change-Number: 11662
Gerrit-PatchSet: 6
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] [rebalancer] location-aware rebalancer (part 2/3)
Posted by "Alexey Serbin (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/11662
to look at the new patch set (#2).
Change subject: [rebalancer] location-aware rebalancer (part 2/3)
......................................................................
[rebalancer] location-aware rebalancer (part 2/3)
Added location load balancing algorithm and corresponding unit tests.
Added one integration test as well, but it's disabled for a now since
it requires a tablet server's location to be reported in
KsckServerHealthSummary. If that piece is in place, the tests passes.
More integration tests will be added in a follow-up commit.
Change-Id: Ia4b0ccb3067a42f112e117b31f22d510f96e5234
---
M src/kudu/tools/rebalance-test.cc
M src/kudu/tools/rebalance_algo-test.cc
M src/kudu/tools/rebalance_algo.cc
M src/kudu/tools/rebalance_algo.h
M src/kudu/tools/rebalancer.cc
M src/kudu/tools/rebalancer.h
M src/kudu/tools/rebalancer_tool-test.cc
M src/kudu/tools/tool_action_cluster.cc
M src/kudu/tools/tool_replica_util.cc
M src/kudu/tools/tool_replica_util.h
10 files changed, 2,005 insertions(+), 495 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/62/11662/2
--
To view, visit http://gerrit.cloudera.org:8080/11662
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia4b0ccb3067a42f112e117b31f22d510f96e5234
Gerrit-Change-Number: 11662
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot