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