jon-wei commented on a change in pull request #7490: Add reload by interval API
URL: https://github.com/apache/incubator-druid/pull/7490#discussion_r277078120
 
 

 ##########
 File path: 
server/src/main/java/org/apache/druid/metadata/SQLMetadataSegmentManager.java
 ##########
 @@ -219,82 +219,112 @@ public void stop()
     }
   }
 
+  private VersionedIntervalTimeline<String, DataSegment> 
getVersionedIntervalTimeline(final String dataSource)
 
 Review comment:
   I think you could consider only the currently enabled segments that overlap 
the target intervals, since only that set would potentially overshadow things.
   
   > For instance if a segment is overshadowed by a disabled segment should it 
still be enabled?
   
   If the timeline is built from only currently enabled segments, then I think 
this case would only arise when a user calls the segmentId-based API, and the 
list they submit contains segments that would overshadow each other. For that, 
I think this sequence would work:
   - Build the timeline from currently enabled segments
   - Add all the target segments to this timeline
   - For each target segment, call isOvershadowed and only enable the ones 
where that returns false
   
   
   

----------------------------------------------------------------
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:
[email protected]


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to