Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14177 )
Change subject: KUDU-2780: create thread for auto-rebalancing ...................................................................... Patch Set 25: (5 comments) Test failure looks relevant. Maybe a flaky test? I think it might be related to https://gerrit.cloudera.org/c/14177/25/src/kudu/master/auto_rebalancer.cc#245 http://gerrit.cloudera.org:8080/#/c/14177/25/src/kudu/master/auto_rebalancer.cc File src/kudu/master/auto_rebalancer.cc: http://gerrit.cloudera.org:8080/#/c/14177/25/src/kudu/master/auto_rebalancer.cc@245 PS25, Line 245: moves_scheduled_this_round_for_test_ = replica_moves.size(); nit: maybe move this after we actually schedule the moves? ie after ExecuteMoves()? It might be possible that the execution takes a long time because all the servers are down, and a test might think that we've scheduled moves, even though we're still in the process of sending moves. http://gerrit.cloudera.org:8080/#/c/14177/25/src/kudu/master/auto_rebalancer.cc@247 PS25, Line 247: ExecuteMoves(replica_moves), It's only OK for this to be WARN_NOT_OK because any moves that we failed to schedule, when checked below, would return an IllegalState error by virtue of not having the REPLACE attribute set, right? FWIW that could be alleviated if we removed failed moves from 'replica_moves' in ExecuteMoves(), though we could do that in a later patch if you prefer. http://gerrit.cloudera.org:8080/#/c/14177/25/src/kudu/master/auto_rebalancer.cc@248 PS25, Line 248: "could not schedule auto-rebalancing replica moves"); nit: this might be clearer as "failed to send move request" or somesuch. http://gerrit.cloudera.org:8080/#/c/14177/25/src/kudu/master/auto_rebalancer.cc@256 PS25, Line 256: "could not perform replica move"); nit: this might be clearer as "failed to check if move completed" or somesuch. http://gerrit.cloudera.org:8080/#/c/14177/25/src/kudu/master/auto_rebalancer.cc@656 PS25, Line 656: Status move_completion_status; nit: this is unused -- To view, visit http://gerrit.cloudera.org:8080/14177 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifca25d1063c07047cf2123e6792b3c7395be20e4 Gerrit-Change-Number: 14177 Gerrit-PatchSet: 25 Gerrit-Owner: Hannah Nguyen <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Hannah Nguyen <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Tue, 17 Mar 2020 23:18:50 +0000 Gerrit-HasComments: Yes
