Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14742 )
Change subject: rebalancer: fix on Runner::UpdateMovesInProgressStatus() ...................................................................... Patch Set 1: (6 comments) Thank you for fixing the bug! The patch looks good, just a few nits. http://gerrit.cloudera.org:8080/#/c/14742/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/14742/1//COMMIT_MSG@14 PS1, Line 14: When no pending operation : completed, the rebalancer tool would re-synchronize the state of : cluster nit: I don't think this is what was happening. I think in that case the rebalancer would try to get information on the next move from the algorithm and perform it. Direct re-synchronization of the cluster state happened only if case of error. At least, that's what I can see from the existing code in https://github.com/apache/kudu/blob/master/src/kudu/tools/rebalancer_tool.cc#L499 http://gerrit.cloudera.org:8080/#/c/14742/1/src/kudu/rebalance/rebalancer.h File src/kudu/rebalance/rebalancer.h: http://gerrit.cloudera.org:8080/#/c/14742/1/src/kudu/rebalance/rebalancer.h@180 PS1, Line 180: its their http://gerrit.cloudera.org:8080/#/c/14742/1/src/kudu/rebalance/rebalancer.h@181 PS1, Line 181: operations were operation was http://gerrit.cloudera.org:8080/#/c/14742/1/src/kudu/rebalance/rebalancer.h@181 PS1, Line 181: some nit: at least one This way it's easier to comprehend. http://gerrit.cloudera.org:8080/#/c/14742/1/src/kudu/tools/rebalancer_tool.cc File src/kudu/tools/rebalancer_tool.cc: http://gerrit.cloudera.org:8080/#/c/14742/1/src/kudu/tools/rebalancer_tool.cc@529 PS1, Line 529: auto has_pending_moves = false; Maybe, move this variable into the scope of the 'while' loop below, closer to the place of its usage? It's not referenced anywhere else outside. http://gerrit.cloudera.org:8080/#/c/14742/1/src/kudu/tools/rebalancer_tool.cc@566 PS1, Line 566: Or there was no pending move left, a new set of : // moves is also required nit: Also, do the same if not a single pending move has left. -- To view, visit http://gerrit.cloudera.org:8080/14742 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8a25574d13f36a8f4f98f4340aa2086711b1c741 Gerrit-Change-Number: 14742 Gerrit-PatchSet: 1 Gerrit-Owner: Yifan Zhang <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 20 Nov 2019 00:03:37 +0000 Gerrit-HasComments: Yes
