Alexey Serbin 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: (6 comments) I think I'll take a look at the updated patch once the change suggested by Adar is implemented. Thank you for your contribution! 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: rebalance tool the rebalancer tool or the `kudu cluster rebalance` CLI tool http://gerrit.cloudera.org:8080/#/c/14154/1//COMMIT_MSG@9 PS1, Line 9: add adds http://gerrit.cloudera.org:8080/#/c/14154/1//COMMIT_MSG@12 PS1, Line 12: It will be useful for server decommissioning. I think moving this into a separate paragraph would make the commit message more readable. http://gerrit.cloudera.org:8080/#/c/14154/1//COMMIT_MSG@10 PS1, Line 10: Once specified blacklist_tservers, replicas on these servers would : be moved to other servers first and then run the rebalancing on : servers not in blacklist I think it is easier to read if this sentence is split into a few, for example: Use the '--blacklist_tservers' to specify a comma-separated list of tablet servers' UUIDs to move tablet replicas from. Once the flag is specified, the tool first moves all replicas from the given tablet servers elsewhere before starting the rebalancing process. When running the rebalancing, the specified tablet servers are not considered as a part of the cluster (i.e. they are effectively ignored). UPDATE: once you transform this patch to re-use the '--ignored_tservers' flag, the description should be updated correspondingly, sure :) 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, "", > Perhaps it's a good idea. +1 Yep, I think that's a great idea. Do you also want to add some control over what to do with replicas on tablet servers which are healthy, but it's desirable to ignore replicas at such tablet servers anyways? Probably, it's possible to represent that as extra boolean flag that controls whether to move tablet replicas from the specified tablet servers first. 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 Alternatively, you could simply drop the 'If not specified part' -- it's assumed the tool works with all the tablet servers in the cluster otherwise. -- 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-Reviewer: Yifan Zhang <[email protected]> Gerrit-Comment-Date: Fri, 30 Aug 2019 23:56:01 +0000 Gerrit-HasComments: Yes
