minor: Updated query to matches the index for retriving unused intervals.

[cryptoe]

We create an index like this :

https://github.com/implydata/druid/blob/528e8079cdc92a1c98bc46ea8e0af30032018b3a/server/src/main/java/org/apache/druid/metadata/SQLMetadataConnector.java#L397-L406

Updating query to match the index above.

Real world results on mysql 8.0.0:

Before

mysql> explain SELECT start, end FROM druid_segments WHERE dataSource = 'xxxx' AND used = false GROUP BY start , end limit 100 \G
*************************** 1. row ***************************
           id: 1
  select_type: SIMPLE
        table: druid_segments
   partitions: NULL
         type: ref
possible_keys: idx_druid_segments_used,idx_druid_segments_datasource_used_end_start,idx_druid_segments_datasource_upgraded_from_segment_id
          key: idx_druid_segments_used
      key_len: 1
          ref: const
         rows: xxxx
     filtered: 50.00
        Extra: Using where; Using temporary
1 row in set, 1 warning (0.01 sec)     

As you can see the index used was idx_druid_segments_used

After

mysql> explain SELECT start, end FROM druid_segments WHERE dataSource = 'xxxx' AND used = false GROUP BY end , start limit 100 \G
*************************** 1. row ***************************
          id: 1
 select_type: SIMPLE
       table: druid_segments
  partitions: NULL
        type: range
possible_keys: idx_druid_segments_used,idx_druid_segments_datasource_used_end_start,idx_druid_segments_datasource_upgraded_from_segment_id
         key: idx_druid_segments_datasource_used_end_start
     key_len: xxx
         ref: NULL
        rows: xxx
    filtered: 100.00
       Extra: Using where; Using index
1 row in set, 1 warning (0.00 sec)

As you can see the index used is idx_druid_segments_datasource_used_end_start

Will send in another PR to adjust the limit to the intervals as well.

[jtuglu1]

could this implicitly change any ordering assumptions of the output?

[kfaraz]

Caller typically should not assume any order of the output as the method does not guarantee any order anyway.

[jtuglu1]

The reason I mentioned it was for safety, since in this I attempted doing something similar (on a different query though) and saw perf gains as well.

[cryptoe]

+1 @kfaraz comment.
We should not assume any ordering guarantees.

[capistrant]

im ok with this. method contract explicitly states no ordering is guaranteed and with no order by, the underlying database would also give no ordering contract. so I don't see any reason to have concern with the change given you show a better plan with the revised query

[capistrant]

nit: may want an inline comment that explicitly states that the group by ordering is being done so we select an index to avoid regression in future (unlikely but possible)

[kfaraz]

Looks good.

I wonder if this would really make any difference in practice though.

I tried running the two queries on a table with 15M unused segments for the target datasource and the time taken was the same for both. This was probably because the role of the index here is just to filter out the other datasources. For the chosen datasource, we still have to scan all records and then group them on start and end times.

[cryptoe]

I saw some improvements if I reduce the limit of number of intervals from 10000 to 100 which I feel would also improve this query in general and hit the indexes since the number of rows scans would reduce with a data source filters.
This is all on mysql 8.0 though.

[cryptoe]

Thanks for the review @jtuglu1 , @kfaraz , @capistrant

[kfaraz]

I saw some improvements if I reduce the limit of number of intervals from 10000 to 100 which I feel would also improve this query in general and hit the indexes since the number of rows scans would reduce with a data source filters.
This is all on mysql 8.0 though.

Yeah, please hold off on reducing the limit, as it will drastically reduce the efficiency of the kill duty.

I have been running the queries on MySQL 8 (with GROUP BY columns reversed and changing limits) on a druid_segments table with more than 15M unused segments for a single datasource and the results for that datasource have been consistently the same irrespective of the query (i.e. the GROUP BY order and the limit does not really affect the query perf. In fact, even a SELECT DISTINCT start, end gives the same perf.).

I got the same results as you with the EXPLAIN query, but the actual SELECT query took the same time.
Edit: I retried the queries and this time the EXPLAIN (GROUP BY start, end vs GROUP BY end, start) for both variants chose the correct index. (Claude seems to think this must have been because of stale statistics).

the number of rows scans would reduce with a data source filters

Yes, but changing the limit of the outer SELECT will not affect this.

[kfaraz]

A query like the following would help though:

SELECT `start`, `end`
  FROM (                                                                                                                   
      SELECT `start`, `end`
      FROM druid_segments
      WHERE dataSource = 'transactions'
      AND used = FALSE                                                                                                     
      LIMIT 1000000
  ) AS unused_segments
  GROUP BY `start`, `end`
  LIMIT 10000

Here, we limit the rows scanned for that datasource to 1M (which affects perf)
and limit the final result to 10k rows (which only affects the size of the results streamed back to the client).

This query gives me consistently good perf of ~2s on the same data.

[cryptoe]

This sounds like a even better fix. Thanks @kfaraz for figuring this out.