Adar Dembo 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 1: (4 comments) Just passing through. Alexey/Andrew, could you guys review this? 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: bool *is_healthy_move); Nit: should be "bool* is_healthy_move". 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: tablet_ids->clear(); Why is this necessary? Looks like every path out of the function swaps to tablet_ids. BTW, this function could be 'void' (or return tablet_ids); doesn't look like it ever fails. 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(blacklist_tservers, "", Nit: how about "blacklisted_tservers" to be consistent with "ignored_tservers"? http://gerrit.cloudera.org:8080/#/c/14154/1/src/kudu/tools/tool_action_cluster.cc@66 PS1, Line 66: "tserver not in blacklist" Should end this sentence with a period and make sure there's a space before "If not specified..." -- 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: 1 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-Comment-Date: Fri, 30 Aug 2019 00:39:00 +0000 Gerrit-HasComments: Yes
