You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2020/05/21 02:28:04 UTC
[GitHub] [druid] yuanlihan opened a new pull request #9905: Fix compact overlapping segments
yuanlihan opened a new pull request #9905:
URL: https://github.com/apache/druid/pull/9905
### Description
this PR fixes https://github.com/apache/druid/issues/9904 by unifying overlapping intervals to ensure overlapping segments compacting in the same `indexSpec`.
This PR has:
- [X] been self-reviewed.
- [X] added documentation for new or modified features or behaviors.
- [ ] added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
- [ ] added or updated version, license, or notice information in [licenses.yaml](https://github.com/apache/druid/blob/master/licenses.yaml)
- [ ] added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
- [X] added unit tests or modified existing tests to cover new code paths.
- [ ] added integration tests.
- [X] been tested in a test Druid cluster.
<!-- Check the items by putting "x" in the brackets for the done things. Not all of these items apply to every PR. Remove the items which are not done or not relevant to the PR. None of the items from the checklist above are strictly necessary, but it would be very helpful if you at least self-review the PR. -->
<hr>
##### Key changed/added classes in this PR
* `CompactionTask`
----------------------------------------------------------------
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
[GitHub] [druid] jihoonson commented on a change in pull request #9905: Fix compact partially overlapping segments
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] jihoonson commented on pull request #9905: Fix compact partially overlapping segments
Posted by GitBox <gi...@apache.org>.
jihoonson commented on pull request #9905:
URL: https://github.com/apache/druid/pull/9905#issuecomment-632190086
@yuanlihan thanks for the comment. I think now I understand your use case.
> As far as I know Druid will correctly handle this in `DruidInputSource`. Or do I misunderstand something?
Oh, now I got what you are doing here. But, I believe this issue should be already being handled properly by the timeline. I left a comment on https://github.com/apache/druid/issues/9904.
----------------------------------------------------------------
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
[GitHub] [druid] yuanlihan commented on pull request #9905: Fix compact partially overlapping segments
Posted by GitBox <gi...@apache.org>.
yuanlihan commented on pull request #9905:
URL: https://github.com/apache/druid/pull/9905#issuecomment-631867912
Hi @jihoonson, thanks for looking into this.
It should be a kind of partial overlapping not one fully overlapping another one. And sorry for the misleading title.
> Can you share some more details of your use case? Why do you want to compact segments of different versions together?
For example, user first creates a `DAY` granularity segment `inline_data_2020-05-18T00:00:00.000Z_2020-05-19T00:00:00.000Z_version_1`, and then need to fix invalid data of one specific hour and create this `HOUR` granularity segment `inline_data_2020-05-18T20:00:00.000Z_2020-05-18T21:00:00.000Z_version_2`. Then there is a compaction task to compact this two partial overlapping segments.
----------------------------------------------------------------
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
[GitHub] [druid] jihoonson commented on pull request #9905: Fix compact overlapping segments
Posted by GitBox <gi...@apache.org>.
jihoonson commented on pull request #9905:
URL: https://github.com/apache/druid/pull/9905#issuecomment-631854837
If I'm reading this change correctly, the compaction task will read both old and new segments for those overlapping intervals. This doesn't seem right to me since the new segment should overshadow the old segment, which means, the compaction task should read only the new segment for the overlapping intervals.
Can you share some more details of your use case? Why do you want to compact segments of different versions together?
----------------------------------------------------------------
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
[GitHub] [druid] yuanlihan edited a comment on pull request #9905: Fix compact partially overlapping segments
Posted by GitBox <gi...@apache.org>.
yuanlihan edited a comment on pull request #9905:
URL: https://github.com/apache/druid/pull/9905#issuecomment-631867912
Hi @jihoonson, thanks for looking into this.
It should be a kind of partial overlapping not one fully overlapping another one. And sorry for the misleading title.
> If I'm reading this change correctly, the compaction task will read both old and new segments for those overlapping intervals.
As far as I know Druid will correctly handle this in `DruidInputSource`. Or do I misunderstand something?
> Can you share some more details of your use case? Why do you want to compact segments of different versions together?
For example, user first creates a `DAY` granularity segment `inline_data_2020-05-18T00:00:00.000Z_2020-05-19T00:00:00.000Z_version_1`, and then need to fix invalid data of one specific hour and create this `HOUR` granularity segment `inline_data_2020-05-18T20:00:00.000Z_2020-05-18T21:00:00.000Z_version_2`. Then there is a compaction task to compact this two partial overlapping segments.
----------------------------------------------------------------
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
[GitHub] [druid] yuanlihan commented on a change in pull request #9905: Fix compact partially overlapping segments
Posted by GitBox <gi...@apache.org>.
yuanlihan commented on a change in pull request #9905:
URL: https://github.com/apache/druid/pull/9905#discussion_r436249451
##########
File path: indexing-service/src/main/java/org/apache/druid/indexing/common/task/CompactionTask.java
##########
@@ -524,10 +524,28 @@ private String createIndexTaskSpecId(int i)
.add(p)
);
- final List<ParallelIndexIngestionSpec> specs = new ArrayList<>(intervalToSegments.size());
- for (Entry<Interval, List<Pair<QueryableIndex, DataSegment>>> entry : intervalToSegments.entrySet()) {
- final Interval interval = entry.getKey();
- final List<Pair<QueryableIndex, DataSegment>> segmentsToCompact = entry.getValue();
+ // unify overlapping intervals to ensure overlapping segments compacting in the same indexSpec
+ List<Pair<Interval, List<Pair<QueryableIndex, DataSegment>>>> intervalToSegmentsUnified = new ArrayList<>();
+ Iterator<Interval> iterator = intervalToSegments.keySet().iterator();
+ Interval union = iterator.next();
+ List<Pair<QueryableIndex, DataSegment>> segments = new ArrayList<>(intervalToSegments.get(union));
Review comment:
fixed CI failure.
----------------------------------------------------------------
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
[GitHub] [druid] jihoonson merged pull request #9905: Fix compact partially overlapping segments
Posted by GitBox <gi...@apache.org>.
jihoonson merged pull request #9905:
URL: https://github.com/apache/druid/pull/9905
----------------------------------------------------------------
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
[GitHub] [druid] yuanlihan commented on a change in pull request #9905: Fix compact partially overlapping segments
Posted by GitBox <gi...@apache.org>.
yuanlihan commented on a change in pull request #9905:
URL: https://github.com/apache/druid/pull/9905#discussion_r435735651
##########
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:
Hi @jihoonson
Here I intended to fix the error in step 5 as described [here](https://github.com/apache/druid/issues/9904).
When compacting the two segments
- `inline_data_2020-05-18T00:00:00.000Z_2020-05-19T00:00:00.000Z_version_1`
- `inline_data_2020-05-18T20:00:00.000Z_2020-05-18T21:00:00.000Z_version_2`
there will be three timelineObjectHolders
- `2020-05-18T00:00:00.000Z/2020-05-18T20:00:00.000Z` backed by segment `inline_data_2020-05-18T00:00:00.000Z_2020-05-19T00:00:00.000Z_version_1`
- `2020-05-18T20:00:00.000Z/2020-05-18T21:00:00.000Z` backed by segment `inline_data_2020-05-18T20:00:00.000Z_2020-05-18T21:00:00.000Z_version_2`
- `2020-05-18T21:00:00.000Z/2020-05-19T00:00:00.000Z` backed by segment `inline_data_2020-05-18T00:00:00.000Z_2020-05-19T00:00:00.000Z_version_1`
And the last two pairs of `queryableIndices` will fail to pass the previous sanity check. Also I found that now timeline already has unit tests about sorting timelineObjectHolders and the sanity check will pass in most case. So I decided to fix the error by re-sorting them anyway. And I added a link in the comment. Do it make sense?
----------------------------------------------------------------
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
[GitHub] [druid] jihoonson commented on a change in pull request #9905: Fix compact partially overlapping segments
Posted by GitBox <gi...@apache.org>.
jihoonson commented on a change in pull request #9905:
URL: https://github.com/apache/druid/pull/9905#discussion_r436077498
##########
File path: indexing-service/src/main/java/org/apache/druid/indexing/common/task/CompactionTask.java
##########
@@ -524,10 +524,28 @@ private String createIndexTaskSpecId(int i)
.add(p)
);
- final List<ParallelIndexIngestionSpec> specs = new ArrayList<>(intervalToSegments.size());
- for (Entry<Interval, List<Pair<QueryableIndex, DataSegment>>> entry : intervalToSegments.entrySet()) {
- final Interval interval = entry.getKey();
- final List<Pair<QueryableIndex, DataSegment>> segmentsToCompact = entry.getValue();
+ // unify overlapping intervals to ensure overlapping segments compacting in the same indexSpec
+ List<Pair<Interval, List<Pair<QueryableIndex, DataSegment>>>> intervalToSegmentsUnified = new ArrayList<>();
+ Iterator<Interval> iterator = intervalToSegments.keySet().iterator();
+ Interval union = iterator.next();
+ List<Pair<QueryableIndex, DataSegment>> segments = new ArrayList<>(intervalToSegments.get(union));
Review comment:
Oops, sorry I missed the CI failure.
```
[ERROR] org.apache.druid.indexing.common.task.CompactionTask.createIngestionSchema(TaskToolbox, CompactionTask$SegmentProvider, CompactionTask$PartitionConfigurationManager, DimensionsSpec, AggregatorFactory[], Granularity, ObjectMapper, CoordinatorClient, SegmentLoaderFactory, RetryPolicyFactory) makes inefficient use of keySet iterator instead of entrySet iterator [org.apache.druid.indexing.common.task.CompactionTask] At CompactionTask.java:[line 531] WMI_WRONG_MAP_ITERATOR
```
This means you are calling `HashMap.get()` even though you can iterate the `Entry` set. Would you fix this please?
----------------------------------------------------------------
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
[GitHub] [druid] jihoonson commented on a change in pull request #9905: Fix compact partially overlapping segments
Posted by GitBox <gi...@apache.org>.
jihoonson commented on a change in pull request #9905:
URL: https://github.com/apache/druid/pull/9905#discussion_r436075797
##########
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:
@yuanlihan thanks for the explanation! What this method does is finding the most reasonable order of dimensions when input segments have mixed orders. The current algorithm is assuming that more recent segments will likely have the order what you want more (there would be some reason if you have changed the dimension order at some point). Maybe a potential improvement can be considering the segment version as well if some segment intervals are overlapped, but I think this could be done in a separate PR.
> Also I found that now timeline already has unit tests about sorting timelineObjectHolders and the sanity check will pass in most case.
BTW, out of curiosity, can you point me out where this test is?
----------------------------------------------------------------
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
[GitHub] [druid] yuanlihan commented on a change in pull request #9905: Fix compact partially overlapping segments
Posted by GitBox <gi...@apache.org>.
yuanlihan commented on a change in pull request #9905:
URL: https://github.com/apache/druid/pull/9905#discussion_r435733738
##########
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:
Hi @jihoonson
Here I intended to fix the error in step 5 as described [here](https://github.com/apache/druid/issues/9904).
When compacting the two segments
- `inline_data_2020-05-18T00:00:00.000Z_2020-05-19T00:00:00.000Z_version_1`
- `inline_data_2020-05-18T20:00:00.000Z_2020-05-18T21:00:00.000Z_version_2`
there will be three timelineObjectHolders
- `2020-05-18T00:00:00.000Z/2020-05-18T20:00:00.000Z` backed by segment `inline_data_2020-05-18T00:00:00.000Z_2020-05-19T00:00:00.000Z_version_1`
- `2020-05-18T20:00:00.000Z/2020-05-18T21:00:00.000Z` backed by segment `inline_data_2020-05-18T20:00:00.000Z_2020-05-18T21:00:00.000Z_version_2`
- `2020-05-18T21:00:00.000Z/2020-05-19T00:00:00.000Z` backed by segment `inline_data_2020-05-18T00:00:00.000Z_2020-05-19T00:00:00.000Z_version_1`
And the last two pairs of `queryableIndices` will fail to pass the previous sanity check. Also I found that now timeline already has unit tests about sorting timelineObjectHolders and the sanity check will pass in most case. So I decided to fix the error by re-sorting them anyway. And I added a link in the comment. Does it make sense?
##########
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:
Naming updated
----------------------------------------------------------------
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
[GitHub] [druid] yuanlihan commented on a change in pull request #9905: Fix compact partially overlapping segments
Posted by GitBox <gi...@apache.org>.
yuanlihan commented on a change in pull request #9905:
URL: https://github.com/apache/druid/pull/9905#discussion_r436249175
##########
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:
Hi @jihoonson
> BTW, out of curiosity, can you point me out where this test is?
I found that the expected timelines in test cases of `VersionedIntervalTimelineTest` are all intentionally provided in well-ordered. Maybe I should said that there are implicit tests that cover sorting timelineObjectHolders.
----------------------------------------------------------------
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