Jackie-Jiang commented on a change in pull request #5728:
URL: https://github.com/apache/incubator-pinot/pull/5728#discussion_r461791755



##########
File path: 
pinot-broker/src/main/java/org/apache/pinot/broker/routing/segmentselector/OfflineSegmentSelector.java
##########
@@ -42,7 +49,14 @@ public void onExternalViewChange(ExternalView externalView, 
Set<String> onlineSe
     // TODO: for new added segments, before all replicas are up, consider not 
selecting them to avoid causing
     //       hotspot servers
 
-    _segments = Collections.unmodifiableList(new ArrayList<>(onlineSegments));
+
+    // Update segment lineage based segment selector
+    _segmentLineageBasedSegmentSelector.onExternalViewChange();
+
+    // Compute the intersection of segments to process from both offline and 
segment lineage based segment selectors.
+    List<String> segmentsToProcess =
+        new 
ArrayList<>(_segmentLineageBasedSegmentSelector.computeSegmentsToProcess(onlineSegments));

Review comment:
       @kishoreg Based on the interface definition, I feel it is better to 
model it as SegmentSelector (`The segments selected should cover the whole 
dataset (table) without overlap`). Another benefit of putting it in 
`Selector`/`Preselector` is to avoid the per-query calculation within the 
`Pruner`




----------------------------------------------------------------
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:
us...@infra.apache.org



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

Reply via email to