[GitHub] [druid] a2l007 edited a comment on issue #9791: Introduce extensibility support for Datasources

[GitBox <gi...@apache.org>]

a2l007 edited a comment on issue #9791:
URL: https://github.com/apache/druid/issues/9791#issuecomment-622376887


   @clintropolis Thanks for reviewing this.
   In order for the broker to support this, `CachingClusteredClient` will have to start handling a map of Datasource -> Timeline instead of just a Timeline object. Apart from this, the only other significant change would be something like this in the `computeSegmentsToQuery` method:
   
      ```
   private Set<SegmentServerSelector> computeSegmentsToQuery(Map<String,TimelineLookup<String, ServerSelector>> timeline)
       {
         final List<TimelineObjectHolder<String, ServerSelector>> serversLookup;
         if (query.getDataSource() instanceof MultiDataSource) {
           List<TimelineObjectHolder<String, ServerSelector>> multiServersLookup = ((MultiDataSource) query.getDataSource()).getSegmentsFromIntervals(intervals, timeline);
           serversLookup = toolChest.filterSegments(query, multiServersLookup);
         } else {
   // I wouldnt be using toString() as below in the actual PR. This is just to explain my thoughts :)
   
           serversLookup = toolChest.filterSegments(
               query,
               intervals.stream().flatMap(i -> timeline.get(query.getDataSource().toString()).lookup(i).stream()).collect(Collectors.toList())
           );
         }
   ```
   The `computeUncoveredIntervals` method would be modified accordingly as well.
   
   As you said, it is pretty straightforward for Union queries as it only has to deal with table datasources. Its implementation would be something like:
   
   ```
   List<TimelineObjectHolder<String, T>> getSegmentsFromIntervals(
         List<Interval> intervals,
         Map<String, ? extends TimelineLookup<String, T>> timelineMap
     )
     {
       return intervals.stream()
                       .flatMap(i -> getDataSources().stream().filter(d ->timelineMap.containsKey(d.getName()))
                                                     .flatMap(n -> timelineMap.get(n.getName()).lookup(i).stream()))
                       .collect(Collectors.toList());
   ```
   
   > Based on ^, I guess not, but If not, how does the broker distinguish which results belong to which timeline? Or does it matter? I'm having trouble imagining how this works in practice without making caching clustered client know a lot more about the query than it currently does, but maybe I haven't though hard enough about this yet.
   
   For MultiDatasource implementations, the historicals would still return intervals based sequences as it does now, but the only difference would be the sequences would have results from multiple datasources for the same interval. Do you foresee any issue with this impacting the broker merge process for Union queries in some way?


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



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org