Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14177 )
Change subject: WIP KUDU-2780: create thread for auto-rebalancing ...................................................................... Patch Set 3: (2 comments) http://gerrit.cloudera.org:8080/#/c/14177/3/src/kudu/master/auto_rebalancer.cc File src/kudu/master/auto_rebalancer.cc: http://gerrit.cloudera.org:8080/#/c/14177/3/src/kudu/master/auto_rebalancer.cc@195 PS3, Line 195: // If not the leader, don't do anything. : { : CatalogManager::ScopedLeaderSharedLock l(catalog_manager_); : if (!l.first_failed_status().ok()) { : number_of_loop_iterations = 0; : moves_scheduled_this_round = 0; : continue; : } : } > Returning from this function will result in the autorebalancer thread exiti Ah yeah, that would complicate election handling, since we'd have to repeatedly check if we're leader and stop if we're not. Another approach might be to have this check some private bool that only gets set on leadership changes. The only benefit there would be that we wouldn't be taking the lock as much. Let's keep this as is for now though, for simplicity. Sleeping doesn't seem unreasonable; will respond in the other comment thread. http://gerrit.cloudera.org:8080/#/c/14177/3/src/kudu/master/auto_rebalancer.cc@238 PS3, Line 238: SleepFor(MonoDelta::FromSeconds(FLAGS_auto_rebalancing_interval_seconds)); > I intended for this to be the only continue case where the thread sleeps, b I agree sleeping makes sense, otherwise (in case GetMoves() or ExecuteMoves() fails) we'd be caught repeatedly locking in a tight loop. I think it'd be easier to reason about using the same interval. -- 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: 3 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 Sep 2019 18:00:56 +0000 Gerrit-HasComments: Yes
