[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4736: Catch exception when checking lead controller resource is enabled

[GitBox <gi...@apache.org>]

jackjlli commented on a change in pull request #4736: Catch exception when checking lead controller resource is enabled
URL: https://github.com/apache/incubator-pinot/pull/4736#discussion_r337792044
 
 

 ##########
 File path: pinot-controller/src/main/java/org/apache/pinot/controller/LeadControllerManager.java
 ##########
 @@ -216,8 +216,6 @@ synchronized void onResourceConfigChange() {
       leadControllerResourceEnabled = LeadControllerUtils.isLeadControllerResourceEnabled(_helixManager);
     } catch (Exception e) {
       LOGGER.error("Exception when checking whether lead controller resource is enabled or not.", e);
-      _isLeadControllerResourceEnabled = false;
 
 Review comment:
   So the idea here is remain the state if any exception happened. 
   Enabling the resource is always one-off. If administrator wants to enable it he will check the log. 
   Plus, I don't think it's rare to have resource config changes because every time there's a Helix task generated, the task will be written to resource config, which will trigger this notification as well.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org