jihoonson commented on a change in pull request #9905:
URL: https://github.com/apache/druid/pull/9905#discussion_r435641586
##########
File path:
indexing-service/src/main/java/org/apache/druid/indexing/common/task/CompactionTask.java
##########
@@ -710,20 +728,8 @@ private static DimensionsSpec
createDimensionsSpec(List<Pair<QueryableIndex, Dat
// Dimensions are extracted from the recent segments to olders because
recent segments are likely to be queried more
// frequently, and thus the performance should be optimized for recent
ones rather than old ones.
- // timelineSegments are sorted in order of interval, but we do a sanity
check here.
- final Comparator<Interval> intervalComparator =
Comparators.intervalsByStartThenEnd();
- for (int i = 0; i < queryableIndices.size() - 1; i++) {
- final Interval shouldBeSmaller =
queryableIndices.get(i).lhs.getDataInterval();
- final Interval shouldBeLarger = queryableIndices.get(i +
1).lhs.getDataInterval();
- Preconditions.checkState(
- intervalComparator.compare(shouldBeSmaller, shouldBeLarger) <= 0,
- "QueryableIndexes are not sorted! Interval[%s] of segment[%s] is
laster than interval[%s] of segment[%s]",
- shouldBeSmaller,
- queryableIndices.get(i).rhs.getId(),
- shouldBeLarger,
- queryableIndices.get(i + 1).rhs.getId()
- );
- }
+ // sort timelineSegments in order of interval.
+ queryableIndices.sort((o1, o2) ->
Comparators.intervalsByStartThenEnd().compare(o1.rhs.getInterval(),
o2.rhs.getInterval()));
Review comment:
Would you explain why you changed this? The intention here was to make
some unit tests failed when the timeline somehow returns unsorted intervals
which is supposed to be a bug.
##########
File path:
indexing-service/src/test/java/org/apache/druid/indexing/common/task/CompactionTaskTest.java
##########
@@ -191,28 +194,33 @@ public static void setupClass()
MIXED_TYPE_COLUMN_MAP.put(Intervals.of("2017-05-01/2017-06-01"), new
DoubleDimensionSchema(MIXED_TYPE_COLUMN));
MIXED_TYPE_COLUMN_MAP.put(Intervals.of("2017-06-01/2017-07-01"), new
DoubleDimensionSchema(MIXED_TYPE_COLUMN));
+ MIXED_TYPE_COLUMN_MAP.put(Intervals.of("2017-06-01/2017-06-02"), new
DoubleDimensionSchema(MIXED_TYPE_COLUMN));
+ MIXED_TYPE_COLUMN_MAP.put(Intervals.of("2017-06-15/2017-06-16"), new
DoubleDimensionSchema(MIXED_TYPE_COLUMN));
+ MIXED_TYPE_COLUMN_MAP.put(Intervals.of("2017-06-30/2017-07-01"), new
DoubleDimensionSchema(MIXED_TYPE_COLUMN));
+
DIMENSIONS = new HashMap<>();
AGGREGATORS = new ArrayList<>();
DIMENSIONS.put(ColumnHolder.TIME_COLUMN_NAME, new
LongDimensionSchema(ColumnHolder.TIME_COLUMN_NAME));
DIMENSIONS.put(TIMESTAMP_COLUMN, new
LongDimensionSchema(TIMESTAMP_COLUMN));
- for (int i = 0; i < SEGMENT_INTERVALS.size(); i++) {
+ int num = 6;
Review comment:
Could you use a more intuitive name such as `numUmbrellaIntervals`?
----------------------------------------------------------------
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
