Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14177 )
Change subject: WIP KUDU-2780: create thread for auto-rebalancing ...................................................................... Patch Set 5: (3 comments) http://gerrit.cloudera.org:8080/#/c/14177/3/src/kudu/master/auto_rebalancer-test.cc File src/kudu/master/auto_rebalancer-test.cc: http://gerrit.cloudera.org:8080/#/c/14177/3/src/kudu/master/auto_rebalancer-test.cc@241 PS3, Line 241: int kNumMasters = 1; > Ah, OK. Could you add this comment into the code, please? >From the other side, it sounds too intricate and counter-intuitive: if using >that reasoning, how to ever distinguish the case when some movements were >performed regardless setting the 'auto_rebalancing_stop_flag' to 'true'? >I.e., if the rebalancer performs some movements in spite of the flag being >set, how to tell that if looking at those counters? http://gerrit.cloudera.org:8080/#/c/14177/5/src/kudu/master/auto_rebalancer.cc File src/kudu/master/auto_rebalancer.cc: http://gerrit.cloudera.org:8080/#/c/14177/5/src/kudu/master/auto_rebalancer.cc@101 PS5, Line 101: auto_rebalancing_stop_flag I think the naming and the semantics of this flag should be updated to be in-line with naming of other flags we use in Kudu: * in the code and in the command-line interface, this is seen as a flag, so '_flag' suffix is not needed * even if set to 'false', this flag doesn't stop the activity of the auto-rebalancer thread: the thread would collect a lot of information from the system tablet anyways, and that's a lot of activity Maybe, call this flag 'auto_rebalancing_enabled' and simply re-schedule the auto-rebalancing task up to the next interval, doing nothing? http://gerrit.cloudera.org:8080/#/c/14177/5/src/kudu/master/auto_rebalancer.cc@196 PS5, Line 196: CatalogManager::ScopedLeaderSharedLock l(catalog_manager_); Taking a lock and then sleeping at line 200 doesn't look good. I think Andrew suggested a good alternative. -- 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: 5 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: Mon, 23 Sep 2019 06:31:04 +0000 Gerrit-HasComments: Yes
