address comments

apache · Jan 7, 2025 · d49e8ab · d49e8ab
1 parent bbba882
commit d49e8ab
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 54 deletions.
diff --git a/pinot-controller/src/main/java/org/apache/pinot/controller/helix/SegmentStatusChecker.java b/pinot-controller/src/main/java/org/apache/pinot/controller/helix/SegmentStatusChecker.java
@@ -343,8 +343,8 @@ private void updateSegmentMetrics(String tableNameWithType, TableConfig tableCon
           for (Map.Entry<String, String> entry : stateMap.entrySet()) {
             String serverInstanceId = entry.getKey();
             String segmentState = entry.getValue();
-            if (isServerQueryable(serverQueryInfoFetcher.getServerQueryInfo(serverInstanceId))
-              && (segmentState.equals(SegmentStateModel.ONLINE) || segmentState.equals(SegmentStateModel.CONSUMING))) {
+            if ((segmentState.equals(SegmentStateModel.ONLINE) || segmentState.equals(SegmentStateModel.CONSUMING))
+                && isServerQueryable(serverQueryInfoFetcher.getServerQueryInfo(serverInstanceId))) {
               numEVReplicasUp++;
             }
             if (segmentState.equals(SegmentStateModel.ERROR)) {

diff --git a/pinot-controller/src/main/java/org/apache/pinot/controller/util/ServerQueryInfoFetcher.java b/pinot-controller/src/main/java/org/apache/pinot/controller/util/ServerQueryInfoFetcher.java
@@ -21,6 +21,7 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import javax.annotation.Nullable;
 import org.apache.helix.model.InstanceConfig;
 import org.apache.helix.zookeeper.datamodel.ZNRecord;
 import org.apache.pinot.controller.helix.core.PinotHelixResourceManager;
@@ -33,45 +34,44 @@
  * repeated ZK access. This class is NOT thread-safe.
  */
 public class ServerQueryInfoFetcher {
-  private PinotHelixResourceManager _pinotHelixResourceManager;
-  private Map<String, ServerQueryInfo> _cache;
+  private final PinotHelixResourceManager _pinotHelixResourceManager;
+  private final Map<String, ServerQueryInfo> _cache;
 
   public ServerQueryInfoFetcher(PinotHelixResourceManager pinotHelixResourceManager) {
     _pinotHelixResourceManager = pinotHelixResourceManager;
     _cache = new HashMap<>();
   }
 
+  @Nullable
   public ServerQueryInfo getServerQueryInfo(String instanceId) {
     return _cache.computeIfAbsent(instanceId, this::getServerQueryInfoOndemand);
   }
 
+  @Nullable
   private ServerQueryInfo getServerQueryInfoOndemand(String instanceId) {
     InstanceConfig instanceConfig = _pinotHelixResourceManager.getHelixInstanceConfig(instanceId);
     if (instanceConfig == null || !InstanceTypeUtils.isServer(instanceId)) {
       return null;
     }
     List<String> tags = instanceConfig.getTags();
     ZNRecord record = instanceConfig.getRecord();
-    boolean helixEnabled = record.getBooleanField(
-        InstanceConfig.InstanceConfigProperty.HELIX_ENABLED.name(), false);
+    boolean helixEnabled = instanceConfig.getInstanceEnabled();
     boolean queriesDisabled = record.getBooleanField(CommonConstants.Helix.QUERIES_DISABLED, false);
     boolean shutdownInProgress = record.getBooleanField(CommonConstants.Helix.IS_SHUTDOWN_IN_PROGRESS, false);
+
     return new ServerQueryInfo(instanceId, tags, null, helixEnabled, queriesDisabled, shutdownInProgress);
   }
 
   public static class ServerQueryInfo {
-    private String _instanceName;
-    private List<String> _tags;
-    private List<String> _tables;
-    private boolean _helixEnabled;
-    private boolean _queriesDisabled;
-    private boolean _shutdownInProgress;
-    private ServerQueryInfo(String instanceName,
-        List<String> tags,
-        List<String> tables,
-        boolean helixEnabled,
-        boolean queriesDisabled,
-        boolean shutdownInProgress) {
+    private final String _instanceName;
+    private final List<String> _tags;
+    private final List<String> _tables;
+    private final boolean _helixEnabled;
+    private final boolean _queriesDisabled;
+    private final boolean _shutdownInProgress;
+
+    private ServerQueryInfo(String instanceName, List<String> tags, List<String> tables, boolean helixEnabled,
+        boolean queriesDisabled, boolean shutdownInProgress) {
       _instanceName = instanceName;
       _tags = tags;
       _tables = tables;
@@ -80,52 +80,16 @@ private ServerQueryInfo(String instanceName,
       _shutdownInProgress = shutdownInProgress;
     }
 
-    public String getInstanceName() {
-      return _instanceName;
-    }
-
-    public void setInstanceName(String instanceName) {
-      _instanceName = instanceName;
-    }
-
-    public List<String> getTags() {
-      return _tags;
-    }
-
-    public void setTags(List<String> tags) {
-      _tags = tags;
-    }
-
-    public List<String> getTables() {
-      return _tables;
-    }
-
-    public void setTables(List<String> tables) {
-      _tables = tables;
-    }
-
     public boolean isHelixEnabled() {
       return _helixEnabled;
     }
 
-    public void setHelixEnabled(boolean helixEnabled) {
-      _helixEnabled = helixEnabled;
-    }
-
     public boolean isQueriesDisabled() {
       return _queriesDisabled;
     }
 
-    public void setQueriesDisabled(boolean queriesDisabled) {
-      _queriesDisabled = queriesDisabled;
-    }
-
     public boolean isShutdownInProgress() {
       return _shutdownInProgress;
     }
-
-    public void setShutdownInProgress(boolean shutdownInProgress) {
-      _shutdownInProgress = shutdownInProgress;
-    }
   }
 }