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 <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Hannah Nguyen <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Thu, 22 Aug 2019 00:34:53 +0000 Gerrit-HasComments: Yes
