You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Hannah Nguyen (Code Review)" <ge...@cloudera.org> on 2019/08/21 01:08:01 UTC
[kudu-CR] Move some rebalancing logic from tools to master
Hannah Nguyen has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14110
Change subject: Move some rebalancing logic from tools to master
......................................................................
Move some rebalancing logic from tools to master
Moved rebalance_algo.h, rebalance_algo.cc, and
rebalance_algo-test.cc from tools into master.
Pulled structures from tools/ksck_results into
master/cluster_status.
TODO: move tools/placement_policy_util and tools/rebalancer
into master too, so future auto-rebalancer task does not
rely on tools.
Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
---
M src/kudu/master/CMakeLists.txt
A src/kudu/master/cluster_status.cc
A src/kudu/master/cluster_status.h
R src/kudu/master/rebalance_algo-test.cc
R src/kudu/master/rebalance_algo.cc
R src/kudu/master/rebalance_algo.h
M src/kudu/tools/CMakeLists.txt
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck.h
M src/kudu/tools/ksck_remote.cc
M src/kudu/tools/ksck_remote.h
M src/kudu/tools/ksck_results.cc
M src/kudu/tools/ksck_results.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/rebalancer.cc
M src/kudu/tools/rebalancer.h
M src/kudu/tools/tool.proto
M src/kudu/tools/tool_action_cluster.cc
22 files changed, 782 insertions(+), 588 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/10/14110/1
--
To view, visit http://gerrit.cloudera.org:8080/14110
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
Gerrit-Change-Number: 14110
Gerrit-PatchSet: 1
Gerrit-Owner: Hannah Nguyen <ha...@cloudera.com>
[kudu-CR] Move some rebalancing logic from tools to master
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14110 )
Change subject: Move some rebalancing logic from tools to master
......................................................................
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/14110/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/14110/2//COMMIT_MSG@15
PS2, Line 15: TODO: move tools/placement_policy_util and tools/rebalancer
: into master too, so future auto-rebalancer task does not
: rely on tools.
> Do you think it should be done in parts? ie. pt1 and pt2?
If the patch is about moving things around and renaming, it should not be hard to review even if such patch looks big in number of lines. The important thing is not to mix in any semantic changes or other changes in the logic of the code.
BTW, is there any other rationale behind the idea of splitting the patch into two pieces except for the concern its size?
http://gerrit.cloudera.org:8080/#/c/14110/2/src/kudu/master/CMakeLists.txt
File src/kudu/master/CMakeLists.txt:
http://gerrit.cloudera.org:8080/#/c/14110/2/src/kudu/master/CMakeLists.txt@48
PS2, Line 48: rebalance_algo.cc
> Do you mean adding another directory like kudu/rebalance and moving all thi
Eventually, only master is going to use that code (and the kudu CLI will simply make RPC calls to master to perform rebalancing on-demand), I think. But it would not hurt to put it under kudu/rebalance or kudu/rebalancer and use corresponding namespace kudu::rebalance (or kudu::rebalancer), as recommended by Adar.
--
To view, visit http://gerrit.cloudera.org:8080/14110
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
Gerrit-Change-Number: 14110
Gerrit-PatchSet: 2
Gerrit-Owner: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 21 Aug 2019 18:34:31 +0000
Gerrit-HasComments: Yes
[kudu-CR] Create kudu/rebalance subdirectory
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has removed a vote on this change.
Change subject: Create kudu/rebalance subdirectory
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/14110
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
Gerrit-Change-Number: 14110
Gerrit-PatchSet: 13
Gerrit-Owner: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] Create kudu/rebalance subdirectory
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14110 )
Change subject: Create kudu/rebalance subdirectory
......................................................................
Patch Set 11: Code-Review+1
LGTM, though there's an IWYU complaint. Would also be good to get another look from Alexey.
--
To view, visit http://gerrit.cloudera.org:8080/14110
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
Gerrit-Change-Number: 14110
Gerrit-PatchSet: 11
Gerrit-Owner: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Tue, 27 Aug 2019 03:40:10 +0000
Gerrit-HasComments: No
[kudu-CR] Create kudu/rebalance subdirectory
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14110 )
Change subject: Create kudu/rebalance subdirectory
......................................................................
Patch Set 9:
(8 comments)
http://gerrit.cloudera.org:8080/#/c/14110/9/src/kudu/rebalance/cluster_status.h
File src/kudu/rebalance/cluster_status.h:
http://gerrit.cloudera.org:8080/#/c/14110/9/src/kudu/rebalance/cluster_status.h@36
PS9, Line 36: // The result of health check on a tablet.
: // Also used to indicate the health of a table, since the health of a table is
: // the health of its least healthy tablet.
: enum class ClusterCheckResult {
: // The tablet is healthy.
: HEALTHY,
:
: // The tablet has on-going tablet copies.
: RECOVERING,
:
: // The tablet has fewer replicas than its table's replication factor and
: // has no on-going tablet copies.
: UNDER_REPLICATED,
:
: // The tablet is missing a majority of its replicas and is unavailable for
: // writes. If a majority cannot be brought back online, then the tablet
: // requires manual intervention to recover.
: UNAVAILABLE,
:
: // There was a discrepancy among the tablets' consensus configs and the master's.
: CONSENSUS_MISMATCH,
: };
I'm not sure this fits exactly under 'rebalance' namespace. The same concern for other former ksck stuff in this file.
Maybe, create a separate namespace 'cluster' and put that stuff there?
There are might be other options around. In the worst case, I'm not against keeping it here for a while, but the placing it under 'rebalance' namespace is a bit odd since this is used by the ksck CLI tool in the context far from rebalancing.
Once put under 'cluster' namespace or alike, it would be natural to drop 'Cluster' prefix in the names of corresponding structures.
http://gerrit.cloudera.org:8080/#/c/14110/9/src/kudu/rebalance/rebalance-test.cc
File src/kudu/rebalance/rebalance-test.cc:
http://gerrit.cloudera.org:8080/#/c/14110/9/src/kudu/rebalance/rebalance-test.cc@182
PS9, Line 182: return HasSameContents(lhs.servers_by_replica_count,
: rhs.servers_by_replica_count);
nit: indentation
http://gerrit.cloudera.org:8080/#/c/14110/9/src/kudu/rebalance/rebalancer.h
File src/kudu/rebalance/rebalancer.h:
http://gerrit.cloudera.org:8080/#/c/14110/9/src/kudu/rebalance/rebalancer.h@19
PS9, Line 19: #include <stddef.h>
: #include <stdint.h>
nit here and elsewhere when using IWYU suggestions: please use C++-style include headers instead, like
#include <cstddef>
#include <cstdint>
IWYU recommends the universal C-style ones, and we haven't fixed that in IWYU yet.
Once you switch to C++-style ones, re-order the includes to make sure the sorted alphabetically.
http://gerrit.cloudera.org:8080/#/c/14110/9/src/kudu/rebalance/rebalancer.h@47
PS9, Line 47: class Rebalancer {
Add short documentation for the class. The important point is to explain what are responsibilities and functionality of this class.
Also, what is behind separation of this class out of the RebalancerTool ? Does it serve any other logical functionality in addition to pure namespacing? If not, maybe you could simply extract the classes/structs and those static methods directly under kudu::rebalance namespace.
http://gerrit.cloudera.org:8080/#/c/14110/9/src/kudu/rebalance/rebalancer.cc
File src/kudu/rebalance/rebalancer.cc:
http://gerrit.cloudera.org:8080/#/c/14110/9/src/kudu/rebalance/rebalancer.cc@57
PS9, Line 57: std::vector<std::string> ignored_tservers_param,
: std::vector<std::string> master_addresses,
: std::vector<std::string> table_filters,
nit: remove std:: namespace prefix since there are corresponding 'using std::xxx' directives above.
http://gerrit.cloudera.org:8080/#/c/14110/9/src/kudu/tools/ksck_results.cc
File src/kudu/tools/ksck_results.cc:
http://gerrit.cloudera.org:8080/#/c/14110/9/src/kudu/tools/ksck_results.cc@714
PS9, Line 714:
nit: indent/spacing
http://gerrit.cloudera.org:8080/#/c/14110/9/src/kudu/tools/ksck_results.cc@746
PS9, Line 746:
nit: indent/spacing
http://gerrit.cloudera.org:8080/#/c/14110/9/src/kudu/tools/ksck_results.cc@781
PS9, Line 781:
nit: indent/spacing
--
To view, visit http://gerrit.cloudera.org:8080/14110
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
Gerrit-Change-Number: 14110
Gerrit-PatchSet: 9
Gerrit-Owner: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Fri, 23 Aug 2019 19:15:26 +0000
Gerrit-HasComments: Yes
[kudu-CR] Create kudu/rebalance subdirectory
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14110 )
Change subject: Create kudu/rebalance subdirectory
......................................................................
Patch Set 13: Verified+1
The test failures are all due to NTP issues.
--
To view, visit http://gerrit.cloudera.org:8080/14110
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
Gerrit-Change-Number: 14110
Gerrit-PatchSet: 13
Gerrit-Owner: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Tue, 27 Aug 2019 18:15:05 +0000
Gerrit-HasComments: No
[kudu-CR] Create kudu/rebalance subdirectory
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14110 )
Change subject: Create kudu/rebalance subdirectory
......................................................................
Patch Set 10:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/14110/10/src/kudu/rebalance/cluster_status.cc
File src/kudu/rebalance/cluster_status.cc:
PS10:
We chatted about this offline: there is already a cluster namespace; maybe we should consider using something else? Maybe cluster_summary? cluster_health?
http://gerrit.cloudera.org:8080/#/c/14110/10/src/kudu/rebalance/rebalance_algo-test.cc
File src/kudu/rebalance/rebalance_algo-test.cc:
http://gerrit.cloudera.org:8080/#/c/14110/10/src/kudu/rebalance/rebalance_algo-test.cc@44
PS10, Line 44:
: namespace kudu {
: namespace rebalance {
: struct TestClusterConfig;
: } // namespace rebalance
: } // namespace kudu
Can this be moved into the below rebalance namespace?
http://gerrit.cloudera.org:8080/#/c/14110/10/src/kudu/tools/ksck_results.h
File src/kudu/tools/ksck_results.h:
http://gerrit.cloudera.org:8080/#/c/14110/10/src/kudu/tools/ksck_results.h@148
PS10, Line 148: const std::vector<cluster::ServerHealthSummary>&
: summaries,
nit: could you keep these on the same line?
http://gerrit.cloudera.org:8080/#/c/14110/10/src/kudu/tools/ksck_results.cc
File src/kudu/tools/ksck_results.cc:
http://gerrit.cloudera.org:8080/#/c/14110/10/src/kudu/tools/ksck_results.cc@714
PS10, Line 714:
nit: spacing here and below
--
To view, visit http://gerrit.cloudera.org:8080/14110
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
Gerrit-Change-Number: 14110
Gerrit-PatchSet: 10
Gerrit-Owner: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Mon, 26 Aug 2019 23:52:51 +0000
Gerrit-HasComments: Yes
[kudu-CR] Move some rebalancing logic from tools to master
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/14110 )
Change subject: Move some rebalancing logic from tools to master
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14110/2/src/kudu/master/CMakeLists.txt
File src/kudu/master/CMakeLists.txt:
http://gerrit.cloudera.org:8080/#/c/14110/2/src/kudu/master/CMakeLists.txt@48
PS2, Line 48: rebalance_algo.cc
> You might separate the rebalancing and replica placement stuff (i.e. placem
Agreed with Alexey. The rebalancing policy stuff is enough code that it probably merits being a first class module, in its own subdirectory, with its own CMake library target.
The namespacing should follow too (i.e. if you put this stuff in a new module called 'rebalancer', use kudu::rebalancer as the C++ namespace).
--
To view, visit http://gerrit.cloudera.org:8080/14110
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
Gerrit-Change-Number: 14110
Gerrit-PatchSet: 2
Gerrit-Owner: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 21 Aug 2019 18:01:29 +0000
Gerrit-HasComments: Yes
[kudu-CR] WIP: Create kudu/rebalance subdirectory
Posted by "Hannah Nguyen (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14110
to look at the new patch set (#4).
Change subject: WIP: Create kudu/rebalance subdirectory
......................................................................
WIP: Create kudu/rebalance subdirectory
Moved structs used for rebalancing from tools/ksck_results
into rebalance/cluster_status. Moved contents of
tools/rebalance_algo into rebalance subdirectory too.
TODO: move tools/placement_policy_util and tools/rebalancer.
Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
---
M CMakeLists.txt
A src/kudu/rebalance/CMakeLists.txt
A src/kudu/rebalance/cluster_status.cc
A src/kudu/rebalance/cluster_status.h
R src/kudu/rebalance/rebalance_algo-test.cc
R src/kudu/rebalance/rebalance_algo.cc
R src/kudu/rebalance/rebalance_algo.h
M src/kudu/tools/CMakeLists.txt
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck.h
M src/kudu/tools/ksck_remote.cc
M src/kudu/tools/ksck_remote.h
M src/kudu/tools/ksck_results.cc
M src/kudu/tools/ksck_results.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/rebalancer.cc
M src/kudu/tools/rebalancer.h
M src/kudu/tools/tool.proto
M src/kudu/tools/tool_action_cluster.cc
23 files changed, 859 insertions(+), 658 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/10/14110/4
--
To view, visit http://gerrit.cloudera.org:8080/14110
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
Gerrit-Change-Number: 14110
Gerrit-PatchSet: 4
Gerrit-Owner: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] Create kudu/rebalance subdirectory
Posted by "Hannah Nguyen (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14110
to look at the new patch set (#8).
Change subject: Create kudu/rebalance subdirectory
......................................................................
Create kudu/rebalance subdirectory
Moved and renamed structs used for rebalancing from
tools/ksck_results into rebalance/cluster_status.
Moved tools/rebalance_algo to rebalance/rebalance_algo.
Moved class Rebalancer from tools/rebalancer to rebalance/rebalancer.
Renamed tools/rebalancer to tools/rebalancer_tool.
Created RebalancerTool in tools/rebalancer_tool that inherits
from Rebalancer.
Moved corresponding tests into kudu/rebalance subdirectory.
Movement of code done to prepare for autorebalancer task in master.
No functional changes made.
Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
---
M CMakeLists.txt
A src/kudu/rebalance/CMakeLists.txt
A src/kudu/rebalance/cluster_status.cc
A src/kudu/rebalance/cluster_status.h
R src/kudu/rebalance/placement_policy_util-test.cc
R src/kudu/rebalance/placement_policy_util.cc
R src/kudu/rebalance/placement_policy_util.h
R src/kudu/rebalance/rebalance-test.cc
R src/kudu/rebalance/rebalance_algo-test.cc
R src/kudu/rebalance/rebalance_algo.cc
R src/kudu/rebalance/rebalance_algo.h
A src/kudu/rebalance/rebalancer.cc
A src/kudu/rebalance/rebalancer.h
M src/kudu/tools/CMakeLists.txt
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck.h
M src/kudu/tools/ksck_remote.cc
M src/kudu/tools/ksck_remote.h
M src/kudu/tools/ksck_results.cc
M src/kudu/tools/ksck_results.h
D src/kudu/tools/rebalancer.h
R src/kudu/tools/rebalancer_tool.cc
A src/kudu/tools/rebalancer_tool.h
M src/kudu/tools/tool.proto
M src/kudu/tools/tool_action_cluster.cc
26 files changed, 1,882 insertions(+), 1,606 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/10/14110/8
--
To view, visit http://gerrit.cloudera.org:8080/14110
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
Gerrit-Change-Number: 14110
Gerrit-PatchSet: 8
Gerrit-Owner: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] WIP: Create kudu/rebalance subdirectory
Posted by "Hannah Nguyen (Code Review)" <ge...@cloudera.org>.
Hannah Nguyen has posted comments on this change. ( http://gerrit.cloudera.org:8080/14110 )
Change subject: WIP: Create kudu/rebalance subdirectory
......................................................................
Patch Set 3:
> (12 comments)
>
> I've uploaded PS3 as a WIP patch to address everything here so far,
> but will put up the remaining code to move in this patch later.
--
To view, visit http://gerrit.cloudera.org:8080/14110
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
Gerrit-Change-Number: 14110
Gerrit-PatchSet: 3
Gerrit-Owner: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 22 Aug 2019 01:09:59 +0000
Gerrit-HasComments: No
[kudu-CR] Create kudu/rebalance subdirectory
Posted by "Hannah Nguyen (Code Review)" <ge...@cloudera.org>.
Hannah Nguyen has posted comments on this change. ( http://gerrit.cloudera.org:8080/14110 )
Change subject: Create kudu/rebalance subdirectory
......................................................................
Patch Set 10:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/14110/10/src/kudu/rebalance/cluster_status.cc
File src/kudu/rebalance/cluster_status.cc:
PS10:
> We chatted about this offline: there is already a cluster namespace; maybe
Done
http://gerrit.cloudera.org:8080/#/c/14110/10/src/kudu/rebalance/rebalance_algo-test.cc
File src/kudu/rebalance/rebalance_algo-test.cc:
http://gerrit.cloudera.org:8080/#/c/14110/10/src/kudu/rebalance/rebalance_algo-test.cc@44
PS10, Line 44:
: namespace kudu {
: namespace rebalance {
: struct TestClusterConfig;
: } // namespace rebalance
: } // namespace kudu
> Can this be moved into the below rebalance namespace?
Done
http://gerrit.cloudera.org:8080/#/c/14110/10/src/kudu/tools/ksck_results.h
File src/kudu/tools/ksck_results.h:
http://gerrit.cloudera.org:8080/#/c/14110/10/src/kudu/tools/ksck_results.h@148
PS10, Line 148: const std::vector<cluster::ServerHealthSummary>&
: summaries,
> nit: could you keep these on the same line?
Done
http://gerrit.cloudera.org:8080/#/c/14110/10/src/kudu/tools/ksck_results.cc
File src/kudu/tools/ksck_results.cc:
http://gerrit.cloudera.org:8080/#/c/14110/10/src/kudu/tools/ksck_results.cc@714
PS10, Line 714:
> nit: spacing here and below
Done
--
To view, visit http://gerrit.cloudera.org:8080/14110
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
Gerrit-Change-Number: 14110
Gerrit-PatchSet: 10
Gerrit-Owner: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Tue, 27 Aug 2019 01:21:42 +0000
Gerrit-HasComments: Yes
[kudu-CR] Create kudu/rebalance subdirectory
Posted by "Hannah Nguyen (Code Review)" <ge...@cloudera.org>.
Hannah Nguyen has posted comments on this change. ( http://gerrit.cloudera.org:8080/14110 )
Change subject: Create kudu/rebalance subdirectory
......................................................................
Patch Set 7:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/14110/7/src/kudu/rebalance/placement_policy_util-test.cc
File src/kudu/rebalance/placement_policy_util-test.cc:
http://gerrit.cloudera.org:8080/#/c/14110/7/src/kudu/rebalance/placement_policy_util-test.cc@33
PS7, Line 33: #include "kudu/consensus/quorum_util.h"
> Hrm.. I might be missing something; why do we need this now?
oops
http://gerrit.cloudera.org:8080/#/c/14110/7/src/kudu/rebalance/placement_policy_util-test.cc@199
PS7, Line 199: Re
> nit: spacing
Done
http://gerrit.cloudera.org:8080/#/c/14110/7/src/kudu/rebalance/rebalancer.cc
File src/kudu/rebalance/rebalancer.cc:
http://gerrit.cloudera.org:8080/#/c/14110/7/src/kudu/rebalance/rebalancer.cc@397
PS7, Line 397: ksck
> nit: maybe just "health report"? Same below. Or omit details of "ksck" in g
ah, these were copied comments from the original tools/rebalancer.cc
http://gerrit.cloudera.org:8080/#/c/14110/8/src/kudu/rebalance/rebalancer.cc
File src/kudu/rebalance/rebalancer.cc:
http://gerrit.cloudera.org:8080/#/c/14110/8/src/kudu/rebalance/rebalancer.cc@86
PS8, Line 86: }
> To leverage move semantics, pass 'config' to by copy, ie
Done
http://gerrit.cloudera.org:8080/#/c/14110/7/src/kudu/tools/rebalancer_tool.cc
File src/kudu/tools/rebalancer_tool.cc:
http://gerrit.cloudera.org:8080/#/c/14110/7/src/kudu/tools/rebalancer_tool.cc@339
PS7, Line 339: const ClusterRawInfo& raw_info,
: const ClusterInfo& ci,
: ostream& out) const {
> nit: spacing
Done
http://gerrit.cloudera.org:8080/#/c/14110/7/src/kudu/tools/rebalancer_tool.cc@750
PS7, Line 750: bool* timed_out) {
> nit: spacing
Done
--
To view, visit http://gerrit.cloudera.org:8080/14110
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
Gerrit-Change-Number: 14110
Gerrit-PatchSet: 7
Gerrit-Owner: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 22 Aug 2019 23:37:06 +0000
Gerrit-HasComments: Yes
[kudu-CR] Create kudu/rebalance subdirectory
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14110 )
Change subject: Create kudu/rebalance subdirectory
......................................................................
Patch Set 13: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/14110
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
Gerrit-Change-Number: 14110
Gerrit-PatchSet: 13
Gerrit-Owner: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Tue, 27 Aug 2019 18:15:31 +0000
Gerrit-HasComments: No
[kudu-CR] Create kudu/rebalance subdirectory
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/14110 )
Change subject: Create kudu/rebalance subdirectory
......................................................................
Create kudu/rebalance subdirectory
Moved and renamed structs used for rebalancing from
tools/ksck_results into rebalance/cluster_status.
Moved tools/rebalance_algo to rebalance/rebalance_algo.
Moved class Rebalancer from tools/rebalancer to rebalance/rebalancer.
Renamed tools/rebalancer to tools/rebalancer_tool.
Created RebalancerTool in tools/rebalancer_tool that inherits
from Rebalancer.
Moved corresponding tests into kudu/rebalance subdirectory.
Movement of code done to prepare for autorebalancer task in master.
No functional changes made.
Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
Reviewed-on: http://gerrit.cloudera.org:8080/14110
Tested-by: Andrew Wong <aw...@cloudera.com>
Reviewed-by: Andrew Wong <aw...@cloudera.com>
Reviewed-by: Alexey Serbin <as...@cloudera.com>
---
M CMakeLists.txt
A src/kudu/rebalance/CMakeLists.txt
A src/kudu/rebalance/cluster_status.cc
A src/kudu/rebalance/cluster_status.h
R src/kudu/rebalance/placement_policy_util-test.cc
R src/kudu/rebalance/placement_policy_util.cc
R src/kudu/rebalance/placement_policy_util.h
R src/kudu/rebalance/rebalance-test.cc
R src/kudu/rebalance/rebalance_algo-test.cc
R src/kudu/rebalance/rebalance_algo.cc
R src/kudu/rebalance/rebalance_algo.h
A src/kudu/rebalance/rebalancer.cc
A src/kudu/rebalance/rebalancer.h
M src/kudu/tools/CMakeLists.txt
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck.h
M src/kudu/tools/ksck_remote.cc
M src/kudu/tools/ksck_remote.h
M src/kudu/tools/ksck_results.cc
M src/kudu/tools/ksck_results.h
D src/kudu/tools/rebalancer.h
R src/kudu/tools/rebalancer_tool.cc
A src/kudu/tools/rebalancer_tool.h
M src/kudu/tools/tool.proto
M src/kudu/tools/tool_action_cluster.cc
26 files changed, 1,918 insertions(+), 1,627 deletions(-)
Approvals:
Andrew Wong: Looks good to me, but someone else must approve; Verified
Alexey Serbin: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/14110
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
Gerrit-Change-Number: 14110
Gerrit-PatchSet: 14
Gerrit-Owner: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] Create kudu/rebalance subdirectory
Posted by "Hannah Nguyen (Code Review)" <ge...@cloudera.org>.
Hannah Nguyen has posted comments on this change. ( http://gerrit.cloudera.org:8080/14110 )
Change subject: Create kudu/rebalance subdirectory
......................................................................
Patch Set 7:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/14110/7/src/kudu/rebalance/placement_policy_util.cc
File src/kudu/rebalance/placement_policy_util.cc:
http://gerrit.cloudera.org:8080/#/c/14110/7/src/kudu/rebalance/placement_policy_util.cc@458
PS7, Line 458: } // namespace tools
> warning: namespace 'rebalance' ends with a comment that refers to a wrong n
Done
http://gerrit.cloudera.org:8080/#/c/14110/7/src/kudu/tools/rebalancer_tool.cc
File src/kudu/tools/rebalancer_tool.cc:
http://gerrit.cloudera.org:8080/#/c/14110/7/src/kudu/tools/rebalancer_tool.cc@79
PS7, Line 79: using std::multimap;
> warning: using decl 'multimap' is unused [misc-unused-using-decls]
Done
http://gerrit.cloudera.org:8080/#/c/14110/7/src/kudu/tools/rebalancer_tool.cc@80
PS7, Line 80: using std::numeric_limits;
> warning: using decl 'numeric_limits' is unused [misc-unused-using-decls]
Done
http://gerrit.cloudera.org:8080/#/c/14110/7/src/kudu/tools/rebalancer_tool.cc@82
PS7, Line 82: using std::set_difference;
> warning: using decl 'set_difference' is unused [misc-unused-using-decls]
Done
http://gerrit.cloudera.org:8080/#/c/14110/7/src/kudu/tools/rebalancer_tool.cc@99
PS7, Line 99:
Fixed
--
To view, visit http://gerrit.cloudera.org:8080/14110
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
Gerrit-Change-Number: 14110
Gerrit-PatchSet: 7
Gerrit-Owner: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 22 Aug 2019 22:06:20 +0000
Gerrit-HasComments: Yes
[kudu-CR] Move some rebalancing logic from tools to master
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14110 )
Change subject: Move some rebalancing logic from tools to master
......................................................................
Patch Set 2:
(4 comments)
I took a quick look and I think it's a good first step in the right direction. It would be great to move the rest of the rebalancer- and policy-placement related stuff as well.
http://gerrit.cloudera.org:8080/#/c/14110/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/14110/2//COMMIT_MSG@15
PS2, Line 15: TODO: move tools/placement_policy_util and tools/rebalancer
: into master too, so future auto-rebalancer task does not
: rely on tools.
Yep, it makes sense to do so. Probably, it's best to do so in this changelist, otherwise the changelist doesn't seem to be logically complete.
http://gerrit.cloudera.org:8080/#/c/14110/2/src/kudu/master/CMakeLists.txt
File src/kudu/master/CMakeLists.txt:
http://gerrit.cloudera.org:8080/#/c/14110/2/src/kudu/master/CMakeLists.txt@48
PS2, Line 48: rebalance_algo.cc
You might separate the rebalancing and replica placement stuff (i.e. placement_policy.cc) into a separate library. Link it into the tools instead of linking in libmaster. That might with resolving circular dependencies (if any) while trying to move the rest of rebalancer stuff from tools into the master.
http://gerrit.cloudera.org:8080/#/c/14110/2/src/kudu/master/cluster_status.h
File src/kudu/master/cluster_status.h:
http://gerrit.cloudera.org:8080/#/c/14110/2/src/kudu/master/cluster_status.h@33
PS2, Line 33: namespace master {
nit: I'm not sure that 'master' is the best namespace to host cluster-related stuff. Maybe, leave it just under 'kudu' namespace? What do you think?
http://gerrit.cloudera.org:8080/#/c/14110/2/src/kudu/master/cluster_status.cc
File src/kudu/master/cluster_status.cc:
http://gerrit.cloudera.org:8080/#/c/14110/2/src/kudu/master/cluster_status.cc@72
PS2, Line 72: int ServerHealthScore(ClusterServerHealth sh) {
: switch (sh) {
: case ClusterServerHealth::HEALTHY:
: return 0;
: case ClusterServerHealth::UNAUTHORIZED:
: return 1;
: case ClusterServerHealth::UNAVAILABLE:
: return 2;
: case ClusterServerHealth::WRONG_SERVER_UUID:
: return 3;
: default:
: LOG(FATAL) << "Unknown ClusterServerHealth";
I know this just came from the prior revision, but I'm not sure this is needed. The ClusterServerHealth is backed by int anyways, so I would expect its elements are comparable without any additional tricks.
I think you can remove this safely.
--
To view, visit http://gerrit.cloudera.org:8080/14110
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
Gerrit-Change-Number: 14110
Gerrit-PatchSet: 2
Gerrit-Owner: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 21 Aug 2019 17:50:14 +0000
Gerrit-HasComments: Yes
[kudu-CR] Move some rebalancing logic from tools to master
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14110 )
Change subject: Move some rebalancing logic from tools to master
......................................................................
Patch Set 2:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/14110/2/src/kudu/master/cluster_status.h
File src/kudu/master/cluster_status.h:
http://gerrit.cloudera.org:8080/#/c/14110/2/src/kudu/master/cluster_status.h@79
PS2, Line 79: boost::optional<int64_t> term,
: boost::optional<int64_t> opid_index,
: boost::optional<std::string> leader_uuid,
: const std::vector<std::string>& voters,
: const std::vector<std::string>& non_voters)
nit: given the renames, be sure to fix any spacing issues this may have caused.
http://gerrit.cloudera.org:8080/#/c/14110/2/src/kudu/master/cluster_status.h@119
PS2, Line 119: // Originally named KsckServerHealth.
nit: remove and maybe just mention the rename in the commit message. Same with other renamed classes.
http://gerrit.cloudera.org:8080/#/c/14110/2/src/kudu/tools/ksck-test.cc
File src/kudu/tools/ksck-test.cc:
http://gerrit.cloudera.org:8080/#/c/14110/2/src/kudu/tools/ksck-test.cc@67
PS2, Line 67: using kudu::master::ClusterConsensusConfigType;
: using kudu::master::ClusterConsensusState;
: using kudu::master::ClusterConsensusStateMap;
: using kudu::master::ClusterReplicaSummary;
: using kudu::master::ClusterServerHealth;
: using kudu::master::ClusterServerHealthSummary;
: using kudu::master::ClusterTableSummary;
: using kudu::master::ClusterTabletSummary;
:
: using server::GetFlagsResponsePB;
: using std::ostringstream;
: using std::shared_ptr;
: using std::static_pointer_cast;
: using std::string;
: using std::unordered_map;
: using std::vector;
: using strings::Substitute;
: using tablet::TabletDataState;
:
nit: you can drop the 'kudu::' prefix, or move all of these out of the namespaces, eg:
using kudu::master::Cluster...
...
using kudu::server::GetFlagsResponsePB;
using kudu::tablet::TabletDataState;
using std::ostringstream;
using std::...
using strings::Substitute;
which is nice because it makes the namespaces even more explicit.
http://gerrit.cloudera.org:8080/#/c/14110/2/src/kudu/tools/ksck_results.h
File src/kudu/tools/ksck_results.h:
http://gerrit.cloudera.org:8080/#/c/14110/2/src/kudu/tools/ksck_results.h@114
PS2, Line 114: // master_summaries
: // tserver_summaries
: // master_uuids
: // master_consensus_conflict
: // master_consensus_state_map
: // tablet_summaries
: // table_summaries
nit: remove this?
--
To view, visit http://gerrit.cloudera.org:8080/14110
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
Gerrit-Change-Number: 14110
Gerrit-PatchSet: 2
Gerrit-Owner: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 21 Aug 2019 20:07:04 +0000
Gerrit-HasComments: Yes
[kudu-CR] Create kudu/rebalance subdirectory
Posted by "Hannah Nguyen (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14110
to look at the new patch set (#12).
Change subject: Create kudu/rebalance subdirectory
......................................................................
Create kudu/rebalance subdirectory
Moved and renamed structs used for rebalancing from
tools/ksck_results into rebalance/cluster_status.
Moved tools/rebalance_algo to rebalance/rebalance_algo.
Moved class Rebalancer from tools/rebalancer to rebalance/rebalancer.
Renamed tools/rebalancer to tools/rebalancer_tool.
Created RebalancerTool in tools/rebalancer_tool that inherits
from Rebalancer.
Moved corresponding tests into kudu/rebalance subdirectory.
Movement of code done to prepare for autorebalancer task in master.
No functional changes made.
Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
---
M CMakeLists.txt
A src/kudu/rebalance/CMakeLists.txt
A src/kudu/rebalance/cluster_status.cc
A src/kudu/rebalance/cluster_status.h
R src/kudu/rebalance/placement_policy_util-test.cc
R src/kudu/rebalance/placement_policy_util.cc
R src/kudu/rebalance/placement_policy_util.h
R src/kudu/rebalance/rebalance-test.cc
R src/kudu/rebalance/rebalance_algo-test.cc
R src/kudu/rebalance/rebalance_algo.cc
R src/kudu/rebalance/rebalance_algo.h
A src/kudu/rebalance/rebalancer.cc
A src/kudu/rebalance/rebalancer.h
M src/kudu/tools/CMakeLists.txt
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck.h
M src/kudu/tools/ksck_remote.cc
M src/kudu/tools/ksck_remote.h
M src/kudu/tools/ksck_results.cc
M src/kudu/tools/ksck_results.h
D src/kudu/tools/rebalancer.h
R src/kudu/tools/rebalancer_tool.cc
A src/kudu/tools/rebalancer_tool.h
M src/kudu/tools/tool.proto
M src/kudu/tools/tool_action_cluster.cc
26 files changed, 1,918 insertions(+), 1,627 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/10/14110/12
--
To view, visit http://gerrit.cloudera.org:8080/14110
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
Gerrit-Change-Number: 14110
Gerrit-PatchSet: 12
Gerrit-Owner: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] Create kudu/rebalance subdirectory
Posted by "Hannah Nguyen (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14110
to look at the new patch set (#13).
Change subject: Create kudu/rebalance subdirectory
......................................................................
Create kudu/rebalance subdirectory
Moved and renamed structs used for rebalancing from
tools/ksck_results into rebalance/cluster_status.
Moved tools/rebalance_algo to rebalance/rebalance_algo.
Moved class Rebalancer from tools/rebalancer to rebalance/rebalancer.
Renamed tools/rebalancer to tools/rebalancer_tool.
Created RebalancerTool in tools/rebalancer_tool that inherits
from Rebalancer.
Moved corresponding tests into kudu/rebalance subdirectory.
Movement of code done to prepare for autorebalancer task in master.
No functional changes made.
Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
---
M CMakeLists.txt
A src/kudu/rebalance/CMakeLists.txt
A src/kudu/rebalance/cluster_status.cc
A src/kudu/rebalance/cluster_status.h
R src/kudu/rebalance/placement_policy_util-test.cc
R src/kudu/rebalance/placement_policy_util.cc
R src/kudu/rebalance/placement_policy_util.h
R src/kudu/rebalance/rebalance-test.cc
R src/kudu/rebalance/rebalance_algo-test.cc
R src/kudu/rebalance/rebalance_algo.cc
R src/kudu/rebalance/rebalance_algo.h
A src/kudu/rebalance/rebalancer.cc
A src/kudu/rebalance/rebalancer.h
M src/kudu/tools/CMakeLists.txt
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck.h
M src/kudu/tools/ksck_remote.cc
M src/kudu/tools/ksck_remote.h
M src/kudu/tools/ksck_results.cc
M src/kudu/tools/ksck_results.h
D src/kudu/tools/rebalancer.h
R src/kudu/tools/rebalancer_tool.cc
A src/kudu/tools/rebalancer_tool.h
M src/kudu/tools/tool.proto
M src/kudu/tools/tool_action_cluster.cc
26 files changed, 1,918 insertions(+), 1,627 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/10/14110/13
--
To view, visit http://gerrit.cloudera.org:8080/14110
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
Gerrit-Change-Number: 14110
Gerrit-PatchSet: 13
Gerrit-Owner: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] Create kudu/rebalance subdirectory
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14110 )
Change subject: Create kudu/rebalance subdirectory
......................................................................
Patch Set 9:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/14110/9/src/kudu/rebalance/cluster_status.h
File src/kudu/rebalance/cluster_status.h:
http://gerrit.cloudera.org:8080/#/c/14110/9/src/kudu/rebalance/cluster_status.h@36
PS9, Line 36: // The result of health check on a tablet.
: // Also used to indicate the health of a table, since the health of a table is
: // the health of its least healthy tablet.
: enum class ClusterCheckResult {
: // The tablet is healthy.
: HEALTHY,
:
: // The tablet has on-going tablet copies.
: RECOVERING,
:
: // The tablet has fewer replicas than its table's replication factor and
: // has no on-going tablet copies.
: UNDER_REPLICATED,
:
: // The tablet is missing a majority of its replicas and is unavailable for
: // writes. If a majority cannot be brought back online, then the tablet
: // requires manual intervention to recover.
: UNAVAILABLE,
:
: // There was a discrepancy among the tablets' consensus configs and the master's.
: CONSENSUS_MISMATCH,
: };
> I agree, putting these under a namespace 'cluster' makes sense.
I'm OK with leaving it currently under src/kudu/rebalancer. You can shop around for other opinions as well.
http://gerrit.cloudera.org:8080/#/c/14110/9/src/kudu/rebalance/rebalancer.h
File src/kudu/rebalance/rebalancer.h:
http://gerrit.cloudera.org:8080/#/c/14110/9/src/kudu/rebalance/rebalancer.h@47
PS9, Line 47: class Rebalancer {
> A Rebalancer object is a member variable of the class master::AutoRebalance
Great! Could you add the gist of it this in the form of a short comments for corresponding classes?
--
To view, visit http://gerrit.cloudera.org:8080/14110
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
Gerrit-Change-Number: 14110
Gerrit-PatchSet: 9
Gerrit-Owner: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Sun, 25 Aug 2019 23:50:28 +0000
Gerrit-HasComments: Yes
[kudu-CR] Move some rebalancing logic from tools to master
Posted by "Hannah Nguyen (Code Review)" <ge...@cloudera.org>.
Hannah Nguyen has posted comments on this change. ( http://gerrit.cloudera.org:8080/14110 )
Change subject: Move some rebalancing logic from tools to master
......................................................................
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/14110/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/14110/2//COMMIT_MSG@15
PS2, Line 15: TODO: move tools/placement_policy_util and tools/rebalancer
: into master too, so future auto-rebalancer task does not
: rely on tools.
> Yep, it makes sense to do so. Probably, it's best to do so in this changel
Do you think it should be done in parts? ie. pt1 and pt2?
I was concerned the patch would be too big if I submitted it all at once.
http://gerrit.cloudera.org:8080/#/c/14110/2/src/kudu/master/CMakeLists.txt
File src/kudu/master/CMakeLists.txt:
http://gerrit.cloudera.org:8080/#/c/14110/2/src/kudu/master/CMakeLists.txt@48
PS2, Line 48: rebalance_algo.cc
> Agreed with Alexey. The rebalancing policy stuff is enough code that it pro
Do you mean adding another directory like kudu/rebalance and moving all this code there?
Or leaving this code in kudu/master but not in the master namespace, adding a target and library to kudu/master's CMakelists.txt?
--
To view, visit http://gerrit.cloudera.org:8080/14110
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
Gerrit-Change-Number: 14110
Gerrit-PatchSet: 2
Gerrit-Owner: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 21 Aug 2019 18:10:06 +0000
Gerrit-HasComments: Yes
[kudu-CR] Create kudu/rebalance subdirectory
Posted by "Hannah Nguyen (Code Review)" <ge...@cloudera.org>.
Hannah Nguyen has posted comments on this change. ( http://gerrit.cloudera.org:8080/14110 )
Change subject: Create kudu/rebalance subdirectory
......................................................................
Patch Set 9:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/14110/9/src/kudu/rebalance/cluster_status.h
File src/kudu/rebalance/cluster_status.h:
http://gerrit.cloudera.org:8080/#/c/14110/9/src/kudu/rebalance/cluster_status.h@36
PS9, Line 36: // The result of health check on a tablet.
: // Also used to indicate the health of a table, since the health of a table is
: // the health of its least healthy tablet.
: enum class ClusterCheckResult {
: // The tablet is healthy.
: HEALTHY,
:
: // The tablet has on-going tablet copies.
: RECOVERING,
:
: // The tablet has fewer replicas than its table's replication factor and
: // has no on-going tablet copies.
: UNDER_REPLICATED,
:
: // The tablet is missing a majority of its replicas and is unavailable for
: // writes. If a majority cannot be brought back online, then the tablet
: // requires manual intervention to recover.
: UNAVAILABLE,
:
: // There was a discrepancy among the tablets' consensus configs and the master's.
: CONSENSUS_MISMATCH,
: };
> I'm OK with leaving it currently under src/kudu/rebalancer. You can shop a
Done
http://gerrit.cloudera.org:8080/#/c/14110/9/src/kudu/rebalance/rebalancer.h
File src/kudu/rebalance/rebalancer.h:
http://gerrit.cloudera.org:8080/#/c/14110/9/src/kudu/rebalance/rebalancer.h@47
PS9, Line 47: class Rebalancer {
> Great! Could you add the gist of it this in the form of a short comments f
Done
--
To view, visit http://gerrit.cloudera.org:8080/14110
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
Gerrit-Change-Number: 14110
Gerrit-PatchSet: 9
Gerrit-Owner: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Mon, 26 Aug 2019 15:57:56 +0000
Gerrit-HasComments: Yes
[kudu-CR] WIP: Create kudu/rebalance subdirectory
Posted by "Hannah Nguyen (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14110
to look at the new patch set (#5).
Change subject: WIP: Create kudu/rebalance subdirectory
......................................................................
WIP: Create kudu/rebalance subdirectory
Moved structs used for rebalancing from tools/ksck_results
into rebalance/cluster_status. Moved contents of
tools/rebalance_algo into rebalance subdirectory too.
TODO: move tools/placement_policy_util and tools/rebalancer.
Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
---
M CMakeLists.txt
A src/kudu/rebalance/CMakeLists.txt
A src/kudu/rebalance/cluster_status.cc
A src/kudu/rebalance/cluster_status.h
R src/kudu/rebalance/rebalance_algo-test.cc
R src/kudu/rebalance/rebalance_algo.cc
R src/kudu/rebalance/rebalance_algo.h
M src/kudu/tools/CMakeLists.txt
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck.h
M src/kudu/tools/ksck_remote.cc
M src/kudu/tools/ksck_remote.h
M src/kudu/tools/ksck_results.cc
M src/kudu/tools/ksck_results.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/rebalancer.cc
M src/kudu/tools/rebalancer.h
M src/kudu/tools/tool.proto
M src/kudu/tools/tool_action_cluster.cc
23 files changed, 859 insertions(+), 658 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/10/14110/5
--
To view, visit http://gerrit.cloudera.org:8080/14110
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
Gerrit-Change-Number: 14110
Gerrit-PatchSet: 5
Gerrit-Owner: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] WIP: Create kudu/rebalance subdirectory
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14110 )
Change subject: WIP: Create kudu/rebalance subdirectory
......................................................................
Patch Set 6:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/14110/6/src/kudu/tools/ksck.h
File src/kudu/tools/ksck.h:
http://gerrit.cloudera.org:8080/#/c/14110/6/src/kudu/tools/ksck.h@301
PS6, Line 301: ClusterServerHealth
Can we forward declare these?
http://gerrit.cloudera.org:8080/#/c/14110/6/src/kudu/tools/ksck.h@613
PS6, Line 613: int table_num_replicas);
nit: spacing
http://gerrit.cloudera.org:8080/#/c/14110/6/src/kudu/tools/ksck_results.cc
File src/kudu/tools/ksck_results.cc:
http://gerrit.cloudera.org:8080/#/c/14110/6/src/kudu/tools/ksck_results.cc@87
PS6, Line 87: const map<string,
: char>& label_by_uuid,
nit: keep these on the same line
http://gerrit.cloudera.org:8080/#/c/14110/6/src/kudu/tools/placement_policy_util.h
File src/kudu/tools/placement_policy_util.h:
http://gerrit.cloudera.org:8080/#/c/14110/6/src/kudu/tools/placement_policy_util.h@30
PS6, Line 30: namespace rebalance {
: struct ClusterLocalityInfo;
: } // namespace rebalance
You can put this in the below kudu namespace.
--
To view, visit http://gerrit.cloudera.org:8080/14110
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
Gerrit-Change-Number: 14110
Gerrit-PatchSet: 6
Gerrit-Owner: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 22 Aug 2019 21:00:04 +0000
Gerrit-HasComments: Yes
[kudu-CR] WIP: Create kudu/rebalance subdirectory
Posted by "Hannah Nguyen (Code Review)" <ge...@cloudera.org>.
Hannah Nguyen has posted comments on this change. ( http://gerrit.cloudera.org:8080/14110 )
Change subject: WIP: Create kudu/rebalance subdirectory
......................................................................
Patch Set 3:
(12 comments)
I've uploaded PS3 as a WIP patch to address everything here so far, but will put up the remaining code to move in PS4.
http://gerrit.cloudera.org:8080/#/c/14110/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/14110/2//COMMIT_MSG@15
PS2, Line 15: Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
:
:
> If the patch is about moving things around and renaming, it should not be h
No, just wanted to make sure it was review-able.
I will add the other changes (tools/placement_policy_util and tools/rebalancer) into PS4.
http://gerrit.cloudera.org:8080/#/c/14110/2/src/kudu/master/CMakeLists.txt
File src/kudu/master/CMakeLists.txt:
http://gerrit.cloudera.org:8080/#/c/14110/2/src/kudu/master/CMakeLists.txt@48
PS2, Line 48: sentry_client_met
> Eventually, only master is going to use that code (and the kudu CLI will si
Done
http://gerrit.cloudera.org:8080/#/c/14110/2/src/kudu/master/cluster_status.h
File src/kudu/master/cluster_status.h:
http://gerrit.cloudera.org:8080/#/c/14110/2/src/kudu/master/cluster_status.h@33
PS2, Line 33:
> nit: I'm not sure that 'master' is the best namespace to host cluster-relat
Done
http://gerrit.cloudera.org:8080/#/c/14110/2/src/kudu/master/cluster_status.h@79
PS2, Line 79:
:
:
:
:
> nit: given the renames, be sure to fix any spacing issues this may have cau
Done
http://gerrit.cloudera.org:8080/#/c/14110/2/src/kudu/master/cluster_status.h@119
PS2, Line 119:
> nit: remove and maybe just mention the rename in the commit message. Same w
Done
http://gerrit.cloudera.org:8080/#/c/14110/2/src/kudu/master/cluster_status.cc
File src/kudu/master/cluster_status.cc:
http://gerrit.cloudera.org:8080/#/c/14110/2/src/kudu/master/cluster_status.cc@72
PS2, Line 72:
:
:
:
:
:
:
:
:
:
:
:
> I know this just came from the prior revision, but I'm not sure this is nee
Done
http://gerrit.cloudera.org:8080/#/c/14110/2/src/kudu/tools/ksck-test.cc
File src/kudu/tools/ksck-test.cc:
http://gerrit.cloudera.org:8080/#/c/14110/2/src/kudu/tools/ksck-test.cc@67
PS2, Line 67: using kudu::rebalance::ClusterReplicaSummary;
: using kudu::rebalance::ClusterServerHealth;
: using kudu::rebalance::ClusterServerHealthSummary;
: using kudu::rebalance::ClusterTableSummary;
: using kudu::rebalance::ClusterTabletSummary;
: using kudu::server::GetFlagsResponsePB;
: using kudu::tablet::TabletDataState;
:
: using std::ostringstream;
: using std::shared_ptr;
: using std::static_pointer_cast;
: using std::string;
: using std::unordered_map;
: using std::vector;
: using strings::Substitute;
:
: namespace kudu {
: namespace tools {
:
> nit: you can drop the 'kudu::' prefix, or move all of these out of the name
Done
http://gerrit.cloudera.org:8080/#/c/14110/2/src/kudu/tools/ksck_results.h
File src/kudu/tools/ksck_results.h:
http://gerrit.cloudera.org:8080/#/c/14110/2/src/kudu/tools/ksck_results.h@114
PS2, Line 114: // Collection of error status for failed checks. Used to print out a final
: // summary of all failed checks.
: // All checks passed if and only if this vector is empty.
: std::vector<Status> error_messages;
:
: // Collection of warnings from checks.
: // These errors are
> nit: remove this?
Done
http://gerrit.cloudera.org:8080/#/c/14110/2/src/kudu/tools/ksck_results.h@190
PS2, Line 190: // summary information will be printed, while in PLAIN_FULL mode, the counts for
> warning: function 'kudu::tools::PrintConsensusMatrix' has a definition with
Done
http://gerrit.cloudera.org:8080/#/c/14110/2/src/kudu/tools/ksck_results.h@191
PS2, Line 191: // every tablet server will be printed as well.
> warning: parameter 'ref_cstate' is const-qualified in the function declarat
Done
http://gerrit.cloudera.org:8080/#/c/14110/2/src/kudu/tools/ksck_results.cc
File src/kudu/tools/ksck_results.cc:
http://gerrit.cloudera.org:8080/#/c/14110/2/src/kudu/tools/ksck_results.cc@127
PS2, Line 127: DataTable* table) {
> warning: the const qualified parameter 'replica_labels' is copied for each
Done
http://gerrit.cloudera.org:8080/#/c/14110/2/src/kudu/tools/ksck_results.cc@307
PS2, Line 307: const ClusterConsensusStateMap& cstates,
> warning: the const qualified parameter 'ref_cstate' is copied for each invo
Done
--
To view, visit http://gerrit.cloudera.org:8080/14110
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
Gerrit-Change-Number: 14110
Gerrit-PatchSet: 3
Gerrit-Owner: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 22 Aug 2019 00:34:53 +0000
Gerrit-HasComments: Yes
[kudu-CR] WIP: Create kudu/rebalance subdirectory
Posted by "Hannah Nguyen (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14110
to look at the new patch set (#3).
Change subject: WIP: Create kudu/rebalance subdirectory
......................................................................
WIP: Create kudu/rebalance subdirectory
Moved structs used for rebalancing from tools/ksck_results
into rebalance/cluster_status. Moved contents of
tools/rebalance_algo into rebalance subdirectory too.
TODO: move tools/placement_policy_util and tools/rebalancer.
Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
---
M CMakeLists.txt
A src/kudu/rebalance/CMakeLists.txt
A src/kudu/rebalance/cluster_status.cc
A src/kudu/rebalance/cluster_status.h
R src/kudu/rebalance/rebalance_algo-test.cc
R src/kudu/rebalance/rebalance_algo.cc
R src/kudu/rebalance/rebalance_algo.h
M src/kudu/tools/CMakeLists.txt
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck.h
M src/kudu/tools/ksck_remote.cc
M src/kudu/tools/ksck_remote.h
M src/kudu/tools/ksck_results.cc
M src/kudu/tools/ksck_results.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/rebalancer.cc
M src/kudu/tools/rebalancer.h
M src/kudu/tools/tool.proto
M src/kudu/tools/tool_action_cluster.cc
23 files changed, 845 insertions(+), 654 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/10/14110/3
--
To view, visit http://gerrit.cloudera.org:8080/14110
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
Gerrit-Change-Number: 14110
Gerrit-PatchSet: 3
Gerrit-Owner: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] WIP: Create kudu/rebalance subdirectory
Posted by "Hannah Nguyen (Code Review)" <ge...@cloudera.org>.
Hannah Nguyen has posted comments on this change. ( http://gerrit.cloudera.org:8080/14110 )
Change subject: WIP: Create kudu/rebalance subdirectory
......................................................................
Patch Set 6:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/14110/6/src/kudu/tools/ksck.h
File src/kudu/tools/ksck.h:
http://gerrit.cloudera.org:8080/#/c/14110/6/src/kudu/tools/ksck.h@301
PS6, Line 301: ClusterServerHealth
> Can we forward declare these?
Done
http://gerrit.cloudera.org:8080/#/c/14110/6/src/kudu/tools/ksck.h@613
PS6, Line 613: int table_num_replicas);
> nit: spacing
Done
http://gerrit.cloudera.org:8080/#/c/14110/6/src/kudu/tools/ksck_results.cc
File src/kudu/tools/ksck_results.cc:
http://gerrit.cloudera.org:8080/#/c/14110/6/src/kudu/tools/ksck_results.cc@87
PS6, Line 87: const map<string,
: char>& label_by_uuid,
> nit: keep these on the same line
Done
http://gerrit.cloudera.org:8080/#/c/14110/6/src/kudu/tools/placement_policy_util.h
File src/kudu/tools/placement_policy_util.h:
http://gerrit.cloudera.org:8080/#/c/14110/6/src/kudu/tools/placement_policy_util.h@30
PS6, Line 30: namespace rebalance {
: struct ClusterLocalityInfo;
: } // namespace rebalance
> You can put this in the below kudu namespace.
Done
--
To view, visit http://gerrit.cloudera.org:8080/14110
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
Gerrit-Change-Number: 14110
Gerrit-PatchSet: 6
Gerrit-Owner: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 22 Aug 2019 21:40:09 +0000
Gerrit-HasComments: Yes
[kudu-CR] Create kudu/rebalance subdirectory
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14110 )
Change subject: Create kudu/rebalance subdirectory
......................................................................
Patch Set 13: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/14110
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
Gerrit-Change-Number: 14110
Gerrit-PatchSet: 13
Gerrit-Owner: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 28 Aug 2019 00:39:28 +0000
Gerrit-HasComments: No
[kudu-CR] Move some rebalancing logic from tools to master
Posted by "Hannah Nguyen (Code Review)" <ge...@cloudera.org>.
Hannah Nguyen has posted comments on this change. ( http://gerrit.cloudera.org:8080/14110 )
Change subject: Move some rebalancing logic from tools to master
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14110/1/src/kudu/tools/ksck_results.cc
File src/kudu/tools/ksck_results.cc:
http://gerrit.cloudera.org:8080/#/c/14110/1/src/kudu/tools/ksck_results.cc@180
PS1, Line 180:
(preemptive) TODO: fix the errant whitespaces in this file
--
To view, visit http://gerrit.cloudera.org:8080/14110
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
Gerrit-Change-Number: 14110
Gerrit-PatchSet: 1
Gerrit-Owner: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 21 Aug 2019 01:16:29 +0000
Gerrit-HasComments: Yes
[kudu-CR] Create kudu/rebalance subdirectory
Posted by "Hannah Nguyen (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14110
to look at the new patch set (#11).
Change subject: Create kudu/rebalance subdirectory
......................................................................
Create kudu/rebalance subdirectory
Moved and renamed structs used for rebalancing from
tools/ksck_results into rebalance/cluster_status.
Moved tools/rebalance_algo to rebalance/rebalance_algo.
Moved class Rebalancer from tools/rebalancer to rebalance/rebalancer.
Renamed tools/rebalancer to tools/rebalancer_tool.
Created RebalancerTool in tools/rebalancer_tool that inherits
from Rebalancer.
Moved corresponding tests into kudu/rebalance subdirectory.
Movement of code done to prepare for autorebalancer task in master.
No functional changes made.
Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
---
M CMakeLists.txt
A src/kudu/rebalance/CMakeLists.txt
A src/kudu/rebalance/cluster_status.cc
A src/kudu/rebalance/cluster_status.h
R src/kudu/rebalance/placement_policy_util-test.cc
R src/kudu/rebalance/placement_policy_util.cc
R src/kudu/rebalance/placement_policy_util.h
R src/kudu/rebalance/rebalance-test.cc
R src/kudu/rebalance/rebalance_algo-test.cc
R src/kudu/rebalance/rebalance_algo.cc
R src/kudu/rebalance/rebalance_algo.h
A src/kudu/rebalance/rebalancer.cc
A src/kudu/rebalance/rebalancer.h
M src/kudu/tools/CMakeLists.txt
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck.h
M src/kudu/tools/ksck_remote.cc
M src/kudu/tools/ksck_remote.h
M src/kudu/tools/ksck_results.cc
M src/kudu/tools/ksck_results.h
D src/kudu/tools/rebalancer.h
R src/kudu/tools/rebalancer_tool.cc
A src/kudu/tools/rebalancer_tool.h
M src/kudu/tools/tool.proto
M src/kudu/tools/tool_action_cluster.cc
26 files changed, 1,914 insertions(+), 1,629 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/10/14110/11
--
To view, visit http://gerrit.cloudera.org:8080/14110
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
Gerrit-Change-Number: 14110
Gerrit-PatchSet: 11
Gerrit-Owner: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] Create kudu/rebalance subdirectory
Posted by "Hannah Nguyen (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14110
to look at the new patch set (#9).
Change subject: Create kudu/rebalance subdirectory
......................................................................
Create kudu/rebalance subdirectory
Moved and renamed structs used for rebalancing from
tools/ksck_results into rebalance/cluster_status.
Moved tools/rebalance_algo to rebalance/rebalance_algo.
Moved class Rebalancer from tools/rebalancer to rebalance/rebalancer.
Renamed tools/rebalancer to tools/rebalancer_tool.
Created RebalancerTool in tools/rebalancer_tool that inherits
from Rebalancer.
Moved corresponding tests into kudu/rebalance subdirectory.
Movement of code done to prepare for autorebalancer task in master.
No functional changes made.
Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
---
M CMakeLists.txt
A src/kudu/rebalance/CMakeLists.txt
A src/kudu/rebalance/cluster_status.cc
A src/kudu/rebalance/cluster_status.h
R src/kudu/rebalance/placement_policy_util-test.cc
R src/kudu/rebalance/placement_policy_util.cc
R src/kudu/rebalance/placement_policy_util.h
R src/kudu/rebalance/rebalance-test.cc
R src/kudu/rebalance/rebalance_algo-test.cc
R src/kudu/rebalance/rebalance_algo.cc
R src/kudu/rebalance/rebalance_algo.h
A src/kudu/rebalance/rebalancer.cc
A src/kudu/rebalance/rebalancer.h
M src/kudu/tools/CMakeLists.txt
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck.h
M src/kudu/tools/ksck_remote.cc
M src/kudu/tools/ksck_remote.h
M src/kudu/tools/ksck_results.cc
M src/kudu/tools/ksck_results.h
D src/kudu/tools/rebalancer.h
R src/kudu/tools/rebalancer_tool.cc
A src/kudu/tools/rebalancer_tool.h
M src/kudu/tools/tool.proto
M src/kudu/tools/tool_action_cluster.cc
26 files changed, 1,884 insertions(+), 1,610 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/10/14110/9
--
To view, visit http://gerrit.cloudera.org:8080/14110
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
Gerrit-Change-Number: 14110
Gerrit-PatchSet: 9
Gerrit-Owner: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] Create kudu/rebalance subdirectory
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14110 )
Change subject: Create kudu/rebalance subdirectory
......................................................................
Patch Set 8:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/14110/7/src/kudu/rebalance/placement_policy_util-test.cc
File src/kudu/rebalance/placement_policy_util-test.cc:
http://gerrit.cloudera.org:8080/#/c/14110/7/src/kudu/rebalance/placement_policy_util-test.cc@33
PS7, Line 33: #include "kudu/consensus/quorum_util.h"
Hrm.. I might be missing something; why do we need this now?
http://gerrit.cloudera.org:8080/#/c/14110/7/src/kudu/rebalance/placement_policy_util-test.cc@199
PS7, Line 199: Re
nit: spacing
http://gerrit.cloudera.org:8080/#/c/14110/7/src/kudu/rebalance/rebalancer.cc
File src/kudu/rebalance/rebalancer.cc:
http://gerrit.cloudera.org:8080/#/c/14110/7/src/kudu/rebalance/rebalancer.cc@397
PS7, Line 397:
nit: maybe just "health report"? Same below. Or omit details of "ksck" in general.
http://gerrit.cloudera.org:8080/#/c/14110/8/src/kudu/rebalance/rebalancer.cc
File src/kudu/rebalance/rebalancer.cc:
http://gerrit.cloudera.org:8080/#/c/14110/8/src/kudu/rebalance/rebalancer.cc@86
PS8, Line 86: : config_(std::move(config)) {
> warning: std::move of the const variable 'config' has no effect; remove std
To leverage move semantics, pass 'config' to by copy, ie
Rebalancer(Config config) {
http://gerrit.cloudera.org:8080/#/c/14110/7/src/kudu/tools/rebalancer_tool.cc
File src/kudu/tools/rebalancer_tool.cc:
http://gerrit.cloudera.org:8080/#/c/14110/7/src/kudu/tools/rebalancer_tool.cc@339
PS7, Line 339: "Location: " << location << endl;
: out << "--------------------------------------------------" << endl;
: }
nit: spacing
http://gerrit.cloudera.org:8080/#/c/14110/7/src/kudu/tools/rebalancer_tool.cc@750
PS7, Line 750:
nit: spacing
--
To view, visit http://gerrit.cloudera.org:8080/14110
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
Gerrit-Change-Number: 14110
Gerrit-PatchSet: 8
Gerrit-Owner: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 22 Aug 2019 23:17:00 +0000
Gerrit-HasComments: Yes
[kudu-CR] Move some rebalancing logic from tools to master
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/14110 )
Change subject: Move some rebalancing logic from tools to master
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14110/2/src/kudu/master/CMakeLists.txt
File src/kudu/master/CMakeLists.txt:
http://gerrit.cloudera.org:8080/#/c/14110/2/src/kudu/master/CMakeLists.txt@48
PS2, Line 48: rebalance_algo.cc
> Do you mean adding another directory like kudu/rebalance and moving all thi
The former.
--
To view, visit http://gerrit.cloudera.org:8080/14110
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
Gerrit-Change-Number: 14110
Gerrit-PatchSet: 2
Gerrit-Owner: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 21 Aug 2019 18:31:34 +0000
Gerrit-HasComments: Yes
[kudu-CR] Create kudu/rebalance subdirectory
Posted by "Hannah Nguyen (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14110
to look at the new patch set (#7).
Change subject: Create kudu/rebalance subdirectory
......................................................................
Create kudu/rebalance subdirectory
Moved and renamed structs used for rebalancing from
tools/ksck_results into rebalance/cluster_status.
Moved tools/rebalance_algo to rebalance/rebalance_algo.
Moved class Rebalancer from tools/rebalancer to rebalance/rebalancer.
Renamed tools/rebalancer to tools/rebalancer_tool.
Created RebalancerTool in tools/rebalancer_tool that inherits
from Rebalancer.
Moved corresponding tests into kudu/rebalance subdirectory.
Movement of code done to prepare for autorebalancer task in master.
No functional changes made.
Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
---
M CMakeLists.txt
A src/kudu/rebalance/CMakeLists.txt
A src/kudu/rebalance/cluster_status.cc
A src/kudu/rebalance/cluster_status.h
R src/kudu/rebalance/placement_policy_util-test.cc
R src/kudu/rebalance/placement_policy_util.cc
R src/kudu/rebalance/placement_policy_util.h
R src/kudu/rebalance/rebalance-test.cc
R src/kudu/rebalance/rebalance_algo-test.cc
R src/kudu/rebalance/rebalance_algo.cc
R src/kudu/rebalance/rebalance_algo.h
A src/kudu/rebalance/rebalancer.cc
A src/kudu/rebalance/rebalancer.h
M src/kudu/tools/CMakeLists.txt
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck.h
M src/kudu/tools/ksck_remote.cc
M src/kudu/tools/ksck_remote.h
M src/kudu/tools/ksck_results.cc
M src/kudu/tools/ksck_results.h
D src/kudu/tools/rebalancer.h
R src/kudu/tools/rebalancer_tool.cc
A src/kudu/tools/rebalancer_tool.h
M src/kudu/tools/tool.proto
M src/kudu/tools/tool_action_cluster.cc
26 files changed, 1,884 insertions(+), 1,602 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/10/14110/7
--
To view, visit http://gerrit.cloudera.org:8080/14110
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
Gerrit-Change-Number: 14110
Gerrit-PatchSet: 7
Gerrit-Owner: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] Move some rebalancing logic from tools to master
Posted by "Hannah Nguyen (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/14110
to look at the new patch set (#2).
Change subject: Move some rebalancing logic from tools to master
......................................................................
Move some rebalancing logic from tools to master
Moved rebalance_algo.h, rebalance_algo.cc, and
rebalance_algo-test.cc from tools into master.
Pulled structures from tools/ksck_results into
master/cluster_status.
TODO: move tools/placement_policy_util and tools/rebalancer
into master too, so future auto-rebalancer task does not
rely on tools.
Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
---
M src/kudu/master/CMakeLists.txt
A src/kudu/master/cluster_status.cc
A src/kudu/master/cluster_status.h
R src/kudu/master/rebalance_algo-test.cc
R src/kudu/master/rebalance_algo.cc
R src/kudu/master/rebalance_algo.h
M src/kudu/tools/CMakeLists.txt
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck.h
M src/kudu/tools/ksck_remote.cc
M src/kudu/tools/ksck_remote.h
M src/kudu/tools/ksck_results.cc
M src/kudu/tools/ksck_results.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/rebalancer.cc
M src/kudu/tools/rebalancer.h
M src/kudu/tools/tool.proto
M src/kudu/tools/tool_action_cluster.cc
22 files changed, 779 insertions(+), 594 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/10/14110/2
--
To view, visit http://gerrit.cloudera.org:8080/14110
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
Gerrit-Change-Number: 14110
Gerrit-PatchSet: 2
Gerrit-Owner: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] Create kudu/rebalance subdirectory
Posted by "Hannah Nguyen (Code Review)" <ge...@cloudera.org>.
Hannah Nguyen has posted comments on this change. ( http://gerrit.cloudera.org:8080/14110 )
Change subject: Create kudu/rebalance subdirectory
......................................................................
Patch Set 9:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/14110/9/src/kudu/rebalance/rebalance-test.cc
File src/kudu/rebalance/rebalance-test.cc:
http://gerrit.cloudera.org:8080/#/c/14110/9/src/kudu/rebalance/rebalance-test.cc@182
PS9, Line 182: return HasSameContents(lhs.servers_by_replica_count,
: rhs.servers_by_replica_count);
> nit: indentation
Done
http://gerrit.cloudera.org:8080/#/c/14110/9/src/kudu/rebalance/rebalancer.h
File src/kudu/rebalance/rebalancer.h:
http://gerrit.cloudera.org:8080/#/c/14110/9/src/kudu/rebalance/rebalancer.h@19
PS9, Line 19: #include <stddef.h>
: #include <stdint.h>
> nit here and elsewhere when using IWYU suggestions: please use C++-style in
Done
http://gerrit.cloudera.org:8080/#/c/14110/9/src/kudu/rebalance/rebalancer.h@47
PS9, Line 47: class Rebalancer {
> Add short documentation for the class. The important point is to explain w
A Rebalancer object is a member variable of the class master::AutoRebalancerTask, but is the base class of tools:RebalancerTool.
RebalancerTool object has some additional functions for printing balance information.
AutoRebalancerTask is a thread that manages the Rebalancer object.
http://gerrit.cloudera.org:8080/#/c/14110/9/src/kudu/rebalance/rebalancer.cc
File src/kudu/rebalance/rebalancer.cc:
http://gerrit.cloudera.org:8080/#/c/14110/9/src/kudu/rebalance/rebalancer.cc@57
PS9, Line 57: std::vector<std::string> ignored_tservers_param,
: std::vector<std::string> master_addresses,
: std::vector<std::string> table_filters,
> nit: remove std:: namespace prefix since there are corresponding 'using std
Done
http://gerrit.cloudera.org:8080/#/c/14110/9/src/kudu/tools/ksck_results.cc
File src/kudu/tools/ksck_results.cc:
http://gerrit.cloudera.org:8080/#/c/14110/9/src/kudu/tools/ksck_results.cc@714
PS9, Line 714:
> nit: indent/spacing
Done
http://gerrit.cloudera.org:8080/#/c/14110/9/src/kudu/tools/ksck_results.cc@746
PS9, Line 746:
> nit: indent/spacing
Done
http://gerrit.cloudera.org:8080/#/c/14110/9/src/kudu/tools/ksck_results.cc@781
PS9, Line 781:
> nit: indent/spacing
Done
--
To view, visit http://gerrit.cloudera.org:8080/14110
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
Gerrit-Change-Number: 14110
Gerrit-PatchSet: 9
Gerrit-Owner: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Sun, 25 Aug 2019 16:57:48 +0000
Gerrit-HasComments: Yes
[kudu-CR] WIP: Create kudu/rebalance subdirectory
Posted by "Hannah Nguyen (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14110
to look at the new patch set (#6).
Change subject: WIP: Create kudu/rebalance subdirectory
......................................................................
WIP: Create kudu/rebalance subdirectory
Moved structs used for rebalancing from tools/ksck_results
into rebalance/cluster_status. Moved contents of
tools/rebalance_algo into rebalance subdirectory too.
TODO: move tools/placement_policy_util and tools/rebalancer.
Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
---
M CMakeLists.txt
A src/kudu/rebalance/CMakeLists.txt
A src/kudu/rebalance/cluster_status.cc
A src/kudu/rebalance/cluster_status.h
R src/kudu/rebalance/rebalance_algo-test.cc
R src/kudu/rebalance/rebalance_algo.cc
R src/kudu/rebalance/rebalance_algo.h
M src/kudu/tools/CMakeLists.txt
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck.h
M src/kudu/tools/ksck_remote.cc
M src/kudu/tools/ksck_remote.h
M src/kudu/tools/ksck_results.cc
M src/kudu/tools/ksck_results.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/rebalancer.cc
M src/kudu/tools/rebalancer.h
M src/kudu/tools/tool.proto
M src/kudu/tools/tool_action_cluster.cc
23 files changed, 857 insertions(+), 661 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/10/14110/6
--
To view, visit http://gerrit.cloudera.org:8080/14110
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
Gerrit-Change-Number: 14110
Gerrit-PatchSet: 6
Gerrit-Owner: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] Move some rebalancing logic from tools to master
Posted by "Hannah Nguyen (Code Review)" <ge...@cloudera.org>.
Hannah Nguyen has posted comments on this change. ( http://gerrit.cloudera.org:8080/14110 )
Change subject: Move some rebalancing logic from tools to master
......................................................................
Patch Set 1:
(14 comments)
http://gerrit.cloudera.org:8080/#/c/14110/1/src/kudu/master/rebalance_algo.cc
File src/kudu/master/rebalance_algo.cc:
http://gerrit.cloudera.org:8080/#/c/14110/1/src/kudu/master/rebalance_algo.cc@198
PS1, Line 198: random_device_(),
> warning: initializer for member 'random_device_' is redundant [readability-
Done
http://gerrit.cloudera.org:8080/#/c/14110/1/src/kudu/tools/ksck-test.cc
File src/kudu/tools/ksck-test.cc:
http://gerrit.cloudera.org:8080/#/c/14110/1/src/kudu/tools/ksck-test.cc@67
PS1, Line 67: using kudu::master::ClusterCheckResult;
> warning: using decl 'ClusterCheckResult' is unused [misc-unused-using-decls
Done
http://gerrit.cloudera.org:8080/#/c/14110/1/src/kudu/tools/ksck-test.cc@542
PS1, Line 542: const string& ref_id,
> warning: parameter 'ref_id' is unused [misc-unused-parameters]
Took out parameter in function signature and in function calls.
http://gerrit.cloudera.org:8080/#/c/14110/1/src/kudu/tools/ksck_results.cc
File src/kudu/tools/ksck_results.cc:
http://gerrit.cloudera.org:8080/#/c/14110/1/src/kudu/tools/ksck_results.cc@61
PS1, Line 61: using kudu::master::ClusterStatus;
> warning: using decl 'ClusterStatus' is unused [misc-unused-using-decls]
Done
http://gerrit.cloudera.org:8080/#/c/14110/1/src/kudu/tools/ksck_results.cc@180
PS1, Line 180:
> (preemptive) TODO: fix the errant whitespaces in this file
Done
http://gerrit.cloudera.org:8080/#/c/14110/1/src/kudu/tools/placement_policy_util.cc
File src/kudu/tools/placement_policy_util.cc:
http://gerrit.cloudera.org:8080/#/c/14110/1/src/kudu/tools/placement_policy_util.cc@47
PS1, Line 47: //TODO: remove
> warning: missing username/bug in TODO [google-readability-todo]
Done
http://gerrit.cloudera.org:8080/#/c/14110/1/src/kudu/tools/rebalance-test.cc
File src/kudu/tools/rebalance-test.cc:
http://gerrit.cloudera.org:8080/#/c/14110/1/src/kudu/tools/rebalance-test.cc@32
PS1, Line 32: #include "kudu/tools/ksck_results.h" //TODO: remove
> warning: missing username/bug in TODO [google-readability-todo]
Done
http://gerrit.cloudera.org:8080/#/c/14110/1/src/kudu/tools/rebalance-test.cc@33
PS1, Line 33: #include "kudu/tools/rebalancer.h" //TODO: remove
> warning: missing username/bug in TODO [google-readability-todo]
Done
http://gerrit.cloudera.org:8080/#/c/14110/1/src/kudu/tools/rebalance-test.cc@46
PS1, Line 46: using kudu::master::ClusterLocalityInfo;
> warning: using decl 'ClusterLocalityInfo' is unused [misc-unused-using-decl
Done
http://gerrit.cloudera.org:8080/#/c/14110/1/src/kudu/tools/rebalance-test.cc@456
PS1, Line 456: //TODO: put this back in before the operator<<
> warning: missing username/bug in TODO [google-readability-todo]
Done
http://gerrit.cloudera.org:8080/#/c/14110/1/src/kudu/tools/rebalancer.cc
File src/kudu/tools/rebalancer.cc:
http://gerrit.cloudera.org:8080/#/c/14110/1/src/kudu/tools/rebalancer.cc@57
PS1, Line 57: using kudu::client::KuduClient;
> warning: using decl 'KuduClient' is unused [misc-unused-using-decls]
Done
http://gerrit.cloudera.org:8080/#/c/14110/1/src/kudu/tools/rebalancer.cc@65
PS1, Line 65: using kudu::master::LocationBalancingAlgo;
> warning: using decl 'LocationBalancingAlgo' is unused [misc-unused-using-de
Done
http://gerrit.cloudera.org:8080/#/c/14110/1/src/kudu/tools/rebalancer.cc@66
PS1, Line 66: using kudu::master::RebalancingAlgo;
> warning: using decl 'RebalancingAlgo' is unused [misc-unused-using-decls]
Done
http://gerrit.cloudera.org:8080/#/c/14110/1/src/kudu/tools/rebalancer.cc@69
PS1, Line 69: using kudu::master::TwoDimensionalGreedyAlgo;
> warning: using decl 'TwoDimensionalGreedyAlgo' is unused [misc-unused-using
Done
--
To view, visit http://gerrit.cloudera.org:8080/14110
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
Gerrit-Change-Number: 14110
Gerrit-PatchSet: 1
Gerrit-Owner: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 21 Aug 2019 15:28:52 +0000
Gerrit-HasComments: Yes
[kudu-CR] Create kudu/rebalance subdirectory
Posted by "Hannah Nguyen (Code Review)" <ge...@cloudera.org>.
Hannah Nguyen has posted comments on this change. ( http://gerrit.cloudera.org:8080/14110 )
Change subject: Create kudu/rebalance subdirectory
......................................................................
Patch Set 9:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14110/9/src/kudu/rebalance/cluster_status.h
File src/kudu/rebalance/cluster_status.h:
http://gerrit.cloudera.org:8080/#/c/14110/9/src/kudu/rebalance/cluster_status.h@36
PS9, Line 36: // The result of health check on a tablet.
: // Also used to indicate the health of a table, since the health of a table is
: // the health of its least healthy tablet.
: enum class ClusterCheckResult {
: // The tablet is healthy.
: HEALTHY,
:
: // The tablet has on-going tablet copies.
: RECOVERING,
:
: // The tablet has fewer replicas than its table's replication factor and
: // has no on-going tablet copies.
: UNDER_REPLICATED,
:
: // The tablet is missing a majority of its replicas and is unavailable for
: // writes. If a majority cannot be brought back online, then the tablet
: // requires manual intervention to recover.
: UNAVAILABLE,
:
: // There was a discrepancy among the tablets' consensus configs and the master's.
: CONSENSUS_MISMATCH,
: };
> I'm not sure this fits exactly under 'rebalance' namespace. The same conce
I agree, putting these under a namespace 'cluster' makes sense.
Where do you think it should go, in terms of subdirectories in src/kudu?
--
To view, visit http://gerrit.cloudera.org:8080/14110
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
Gerrit-Change-Number: 14110
Gerrit-PatchSet: 9
Gerrit-Owner: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Fri, 23 Aug 2019 20:27:06 +0000
Gerrit-HasComments: Yes
[kudu-CR] Create kudu/rebalance subdirectory
Posted by "Hannah Nguyen (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14110
to look at the new patch set (#10).
Change subject: Create kudu/rebalance subdirectory
......................................................................
Create kudu/rebalance subdirectory
Moved and renamed structs used for rebalancing from
tools/ksck_results into rebalance/cluster_status.
Moved tools/rebalance_algo to rebalance/rebalance_algo.
Moved class Rebalancer from tools/rebalancer to rebalance/rebalancer.
Renamed tools/rebalancer to tools/rebalancer_tool.
Created RebalancerTool in tools/rebalancer_tool that inherits
from Rebalancer.
Moved corresponding tests into kudu/rebalance subdirectory.
Movement of code done to prepare for autorebalancer task in master.
No functional changes made.
Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
---
M CMakeLists.txt
A src/kudu/rebalance/CMakeLists.txt
A src/kudu/rebalance/cluster_status.cc
A src/kudu/rebalance/cluster_status.h
R src/kudu/rebalance/placement_policy_util-test.cc
R src/kudu/rebalance/placement_policy_util.cc
R src/kudu/rebalance/placement_policy_util.h
R src/kudu/rebalance/rebalance-test.cc
R src/kudu/rebalance/rebalance_algo-test.cc
R src/kudu/rebalance/rebalance_algo.cc
R src/kudu/rebalance/rebalance_algo.h
A src/kudu/rebalance/rebalancer.cc
A src/kudu/rebalance/rebalancer.h
M src/kudu/tools/CMakeLists.txt
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck.h
M src/kudu/tools/ksck_remote.cc
M src/kudu/tools/ksck_remote.h
M src/kudu/tools/ksck_results.cc
M src/kudu/tools/ksck_results.h
D src/kudu/tools/rebalancer.h
R src/kudu/tools/rebalancer_tool.cc
A src/kudu/tools/rebalancer_tool.h
M src/kudu/tools/tool.proto
M src/kudu/tools/tool_action_cluster.cc
26 files changed, 1,908 insertions(+), 1,620 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/10/14110/10
--
To view, visit http://gerrit.cloudera.org:8080/14110
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie0242a019cb44517539da2878cf889ee0c511964
Gerrit-Change-Number: 14110
Gerrit-PatchSet: 10
Gerrit-Owner: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hannah Nguyen <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)