Address feedback

ankitsultana · ankitsultana · commit 13c4a37d919b · 2022-09-19T12:00:10.000-07:00
diff --git a/pinot-core/src/main/java/org/apache/pinot/core/data/manager/BaseTableDataManager.java b/pinot-core/src/main/java/org/apache/pinot/core/data/manager/BaseTableDataManager.java
@@ -240,7 +240,7 @@ public void addSegment(String segmentName, TableConfig tableConfig, IndexLoading
   @Override
   public void removeSegment(String segmentName) {
     _logger.info("Removing segment: {} from table: {}", segmentName, _tableNameWithType);
-    SegmentDataManager segmentDataManager = deRegisterSegment(segmentName);
+    SegmentDataManager segmentDataManager = unregisterSegment(segmentName);
     if (segmentDataManager != null) {
       releaseSegment(segmentDataManager);
       _logger.info("Removed segment: {} from table: {}", segmentName, _tableNameWithType);
@@ -449,7 +449,7 @@ protected SegmentDataManager registerSegment(String segmentName, SegmentDataMana
    * identify this scenario.
    */
   @Nullable
-  protected SegmentDataManager deRegisterSegment(String segmentName) {
+  protected SegmentDataManager unregisterSegment(String segmentName) {
     _deletedSegments.put(segmentName, segmentName);
     return _segmentDataManagerMap.remove(segmentName);
   }
diff --git a/pinot-core/src/main/java/org/apache/pinot/core/query/executor/ServerQueryExecutorV1Impl.java b/pinot-core/src/main/java/org/apache/pinot/core/query/executor/ServerQueryExecutorV1Impl.java
@@ -195,10 +195,6 @@ private DataTable processQueryInternal(ServerQueryRequest queryRequest, Executor
     List<String> unAcquiredSegments = new ArrayList<>();
     List<SegmentDataManager> segmentDataManagers = tableDataManager.acquireSegments(
         segmentsToQuery, unAcquiredSegments);
-    List<String> missingSegments =
-        unAcquiredSegments.stream()
-            .filter(segmentName -> !tableDataManager.isSegmentDeletedRecently(segmentName))
-            .collect(Collectors.toList());
     int numSegmentsAcquired = segmentDataManagers.size();
     List<IndexSegment> indexSegments = new ArrayList<>(numSegmentsAcquired);
     for (SegmentDataManager segmentDataManager : segmentDataManagers) {
@@ -297,12 +293,18 @@ private DataTable processQueryInternal(ServerQueryRequest queryRequest, Executor
     //
     // After step 2 but before step 4, segment will be missing on server side
     // TODO: Change broker to watch both IdealState and ExternalView to not query the removed segments
-    int numMissingSegments = missingSegments.size();
-    if (numMissingSegments != 0) {
-      dataTable.addException(QueryException.getException(QueryException.SERVER_SEGMENT_MISSING_ERROR,
-          String.format("%d segments %s missing on server: %s", numMissingSegments, missingSegments,
-              _instanceDataManager.getInstanceId())));
-      _serverMetrics.addMeteredTableValue(tableNameWithType, ServerMeter.NUM_MISSING_SEGMENTS, numMissingSegments);
+    if (unAcquiredSegments.size() > 0) {
+      List<String> missingSegments =
+          unAcquiredSegments.stream()
+              .filter(segmentName -> !tableDataManager.isSegmentDeletedRecently(segmentName))
+              .collect(Collectors.toList());
+      int numMissingSegments = missingSegments.size();
+      if (numMissingSegments > 0) {
+        dataTable.addException(QueryException.getException(QueryException.SERVER_SEGMENT_MISSING_ERROR,
+            String.format("%d segments %s missing on server: %s", numMissingSegments, missingSegments,
+                _instanceDataManager.getInstanceId())));
+        _serverMetrics.addMeteredTableValue(tableNameWithType, ServerMeter.NUM_MISSING_SEGMENTS, numMissingSegments);
+      }
     }
 
     if (tableDataManager instanceof RealtimeTableDataManager) {
diff --git a/pinot-server/src/main/java/org/apache/pinot/server/starter/helix/HelixInstanceDataManagerConfig.java b/pinot-server/src/main/java/org/apache/pinot/server/starter/helix/HelixInstanceDataManagerConfig.java
@@ -127,7 +127,7 @@ public class HelixInstanceDataManagerConfig implements InstanceDataManagerConfig
   private final static String[] REQUIRED_KEYS = {INSTANCE_ID, INSTANCE_DATA_DIR, READ_MODE};
   private static final long DEFAULT_ERROR_CACHE_SIZE = 100L;
   private static final int DEFAULT_DELETED_SEGMENTS_CACHE_SIZE = 10_000;
-  private static final int DEFAULT_DELETED_SEGMENTS_CACHE_TTL_MINUTES = 10;
+  private static final int DEFAULT_DELETED_SEGMENTS_CACHE_TTL_MINUTES = 2;
   private PinotConfiguration _instanceDataManagerConfiguration = null;
 
   public HelixInstanceDataManagerConfig(PinotConfiguration serverConfig)
