Yifan Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/14154 )
Change subject: KUDU-2914: Rebalance tool support moving replicas from some specific tablet servers ...................................................................... Patch Set 2: (8 comments) http://gerrit.cloudera.org:8080/#/c/14154/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/14154/1//COMMIT_MSG@9 PS1, Line 9: por > adds Done http://gerrit.cloudera.org:8080/#/c/14154/1//COMMIT_MSG@9 PS1, Line 9: blet servers, > the rebalancer tool Done http://gerrit.cloudera.org:8080/#/c/14154/1//COMMIT_MSG@12 PS1, Line 12: LI tool. > I think moving this into a separate paragraph would make the commit message Done http://gerrit.cloudera.org:8080/#/c/14154/1//COMMIT_MSG@10 PS1, Line 10: this patch re-uses the '--ignored_tservers' flag and adds a : '--move_replicas_from_ignored_tservers' flag to the : `kudu rebalance cluster` > I think it is easier to read if this sentence is split into a few, for exam Done http://gerrit.cloudera.org:8080/#/c/14154/1/src/kudu/rebalance/rebalancer.h File src/kudu/rebalance/rebalancer.h: http://gerrit.cloudera.org:8080/#/c/14154/1/src/kudu/rebalance/rebalancer.h@208 PS1, Line 208: // involve moving replicas of tablets which are in 'scheduled_moves'. The > Nit: should be "bool* is_healthy_move". Done http://gerrit.cloudera.org:8080/#/c/14154/1/src/kudu/rebalance/rebalancer.cc File src/kudu/rebalance/rebalancer.cc: http://gerrit.cloudera.org:8080/#/c/14154/1/src/kudu/rebalance/rebalancer.cc@110 PS1, Line 110: const auto& table_id = move.table_id; > Why is this necessary? Looks like every path out of the function swaps to t Done http://gerrit.cloudera.org:8080/#/c/14154/1/src/kudu/tools/tool_action_cluster.cc File src/kudu/tools/tool_action_cluster.cc: http://gerrit.cloudera.org:8080/#/c/14154/1/src/kudu/tools/tool_action_cluster.cc@62 PS1, Line 62: DEFINE_string(ignored_tservers, "", > +1 Done http://gerrit.cloudera.org:8080/#/c/14154/1/src/kudu/tools/tool_action_cluster.cc@66 PS1, Line 66: "as a part of the cluster as well as the replicas on them. " > Alternatively, you could simply drop the 'If not specified part' -- it's as Done -- To view, visit http://gerrit.cloudera.org:8080/14154 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I86cfb740030946c13db1a9ca63d241f4907d6c89 Gerrit-Change-Number: 14154 Gerrit-PatchSet: 2 Gerrit-Owner: Yifan Zhang <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yifan Zhang <[email protected]> Gerrit-Comment-Date: Wed, 04 Sep 2019 12:26:51 +0000 Gerrit-HasComments: Yes
