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 5: (13 comments) http://gerrit.cloudera.org:8080/#/c/14154/4/src/kudu/rebalance/rebalancer.h File src/kudu/rebalance/rebalancer.h: http://gerrit.cloudera.org:8080/#/c/14154/4/src/kudu/rebalance/rebalancer.h@72 PS4, Line 72: // If not empty, specified tablet servers (including their health state > nit: missing space Done http://gerrit.cloudera.org:8080/#/c/14154/4/src/kudu/rebalance/rebalancer.h@101 PS4, Line 101: run > running Done http://gerrit.cloudera.org:8080/#/c/14154/4/src/kudu/rebalance/rebalancer.h@202 PS4, Line 202: . > This part is no longer relevant once the return type changed from Status to Done http://gerrit.cloudera.org:8080/#/c/14154/4/src/kudu/tools/rebalancer_tool-test.cc File src/kudu/tools/rebalancer_tool-test.cc: http://gerrit.cloudera.org:8080/#/c/14154/4/src/kudu/tools/rebalancer_tool-test.cc@239 PS4, Line 239: SafetyTe > Safety? Done http://gerrit.cloudera.org:8080/#/c/14154/4/src/kudu/tools/rebalancer_tool-test.cc@245 PS4, Line 245: TEST_P(RebalanceStartSafetyTest, TooManyIgnoredTservers) { > I think it's also necessary to add a scenario when the rebalancer tool actu Done http://gerrit.cloudera.org:8080/#/c/14154/4/src/kudu/tools/rebalancer_tool-test.cc@278 PS4, Line 278: MATCHES(err, "T > nit: this constant is used only once; maybe replace it's usage with the str Done http://gerrit.cloudera.org:8080/#/c/14154/4/src/kudu/tools/rebalancer_tool-test.cc@299 PS4, Line 299: { > nit: an extra line Done http://gerrit.cloudera.org:8080/#/c/14154/4/src/kudu/tools/rebalancer_tool.h File src/kudu/tools/rebalancer_tool.h: http://gerrit.cloudera.org:8080/#/c/14154/4/src/kudu/tools/rebalancer_tool.h@327 PS4, Line 327: ab > nit: about Done http://gerrit.cloudera.org:8080/#/c/14154/4/src/kudu/tools/rebalancer_tool.h@346 PS4, Line 346: the ignored to o > nit: ignored Done http://gerrit.cloudera.org:8080/#/c/14154/4/src/kudu/tools/rebalancer_tool.cc File src/kudu/tools/rebalancer_tool.cc: http://gerrit.cloudera.org:8080/#/c/14154/4/src/kudu/tools/rebalancer_tool.cc@1014 PS4, Line 1014: for (const auto& elem : server_uuids) { : if (elem != move.to && !ContainsKey(cluster_info.healthy_ignored_tservers, elem)) { : LOG(INFO) << Substitute("choose destination server: $0",elem); : move.to = elem; : > How is it guaranteed that the amended move doesn't make the cluster even mo A random 'move.to' server has been chosen here and the move would possibly make the cluster more imbalanced. When we need to move a replica of table T from ignored server A to server B, maybe we can't move any replica because there are replicas of the same tablets on server B, and if we skip this move, next time the scheduled move will be same, so we need to find another 'move.to' server. If the move has been fixed, we will break the loop and get next moves based on the move, so the final state is balanced. http://gerrit.cloudera.org:8080/#/c/14154/4/src/kudu/tools/tool_action_cluster.cc File src/kudu/tools/tool_action_cluster.cc: http://gerrit.cloudera.org:8080/#/c/14154/4/src/kudu/tools/tool_action_cluster.cc@130 PS4, Line 130: move > move Done http://gerrit.cloudera.org:8080/#/c/14154/4/src/kudu/tools/tool_action_cluster.cc@130 PS4, Line 130: ervers to other" > servers Done http://gerrit.cloudera.org:8080/#/c/14154/4/src/kudu/tools/tool_action_cluster.cc@132 PS4, Line 132: This setting is effective only if the '--ignored_tservers' fla > How about: 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: 5 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: Tue, 01 Oct 2019 16:21:02 +0000 Gerrit-HasComments: Yes
