jihoonson commented on a change in pull request #10307:
URL: https://github.com/apache/druid/pull/10307#discussion_r475882549
##########
File path:
indexing-service/src/main/java/org/apache/druid/indexing/common/task/batch/parallel/ParallelIndexSupervisorTask.java
##########
@@ -764,7 +765,28 @@ private PartitionBoundaries
determineRangePartition(Collection<StringDistributio
return Pair.of(start, stop);
}
- private static void publishSegments(TaskToolbox toolbox, Map<String,
PushedSegmentsReport> reportsMap)
+ public static Function<Set<DataSegment>, Set<DataSegment>>
compactionStateAnnotateFunction(
Review comment:
Good idea! Moved this method.
##########
File path:
server/src/main/java/org/apache/druid/client/indexing/ClientCompactionTaskQueryTuningConfig.java
##########
@@ -158,27 +222,90 @@ public IndexSpec getIndexSpec()
return indexSpec;
}
+ @JsonProperty
+ @Nullable
+ public IndexSpec getIndexSpecForIntermediatePersists()
+ {
+ return indexSpecForIntermediatePersists;
+ }
+
@JsonProperty
@Nullable
public Integer getMaxPendingPersists()
{
return maxPendingPersists;
}
+ @JsonProperty
Review comment:
Yes, this should be passed properly when this class is deserialized as
`ParallelIndexTuningConfig`.
##########
File path:
server/src/main/java/org/apache/druid/server/coordinator/duty/NewestSegmentFirstIterator.java
##########
@@ -247,17 +248,30 @@ public boolean hasNext()
}
}
- private boolean needsCompaction(DataSourceCompactionConfig config,
SegmentsToCompact candidates)
+ @VisibleForTesting
+ static PartitionsSpec
findPartitinosSpecFromConfig(ClientCompactionTaskQueryTuningConfig tuningConfig)
+ {
Review comment:
The auto compaction supported only dynamic partitioning before this PR.
The deprecated `maxRowsPerSegment` and `maxTotalRows` in
`DataSourceCompactionConfig` are for dynamic partitioning. Users are
recommended to use `partitionsSpec` instead of them.
> Misread the code. if the maxTotalRows is null in partition spec, should it
be read from tuningConfig instead of using the Long.Max?
My intention is using `partitionsSpec` if it's specified and ignoring
deprecated ones. I will add docs for this behaviour as well as the newly
supported tuning configurations.
----------------------------------------------------------------
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]
