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

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

Jackie-Jiang 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_r337778075
 
 

 ##########
 File path: pinot-core/src/main/java/org/apache/pinot/server/realtime/ControllerLeaderLocator.java
 ##########
 @@ -102,10 +102,20 @@ public static ControllerLeaderLocator getInstance() {
    * Checks whether lead controller resource has been enabled or not.
    * If yes, updates lead controller pairs from the external view of lead controller resource.
    * Otherwise, updates lead controller pairs from Helix cluster leader.
+   * Note: Exception may happen due to Helix/ZK disconnect. If so, we should NOT regress the behavior back to false.
+   * Thus, simply exiting the method should be enough. Retry will be done in the next request.
    */
   private void refreshControllerLeaderMap() {
     // Checks whether lead controller resource has been enabled or not.
-    if (LeadControllerUtils.isLeadControllerResourceEnabled(_helixManager)) {
+    boolean leadControllerResourceEnabled;
 
 Review comment:
   I would recommend you to wrap the whole method into a try-catch because `refreshControllerLeaderMapFromHelixClusterLeader()` can also throw exception.

----------------------------------------------------------------
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