jtuglu1 commented on code in PR #18612:
URL: https://github.com/apache/druid/pull/18612#discussion_r2415711314
##########
indexing-service/src/main/java/org/apache/druid/indexing/common/task/CompactionTask.java:
##########
@@ -517,6 +520,7 @@ public TaskStatus runTask(TaskToolbox toolbox) throws
Exception
emitMetric(toolbox.getEmitter(), "ingest/count", 1);
final Map<Interval, DataSchema> intervalDataSchemas =
createDataSchemasForIntervals(
Review Comment:
Given we are now ingesting into potentially a new datasource, have we
checked whether all associated tasks for ingestion (e.g. lock acquisition,
segment alloc) are now working?
##########
.idea/inspectionProfiles/Druid.xml:
##########
Review Comment:
Let's avoid writing this if possible
##########
indexing-service/src/main/java/org/apache/druid/indexing/common/task/CompactionInputSpec.java:
##########
@@ -51,4 +51,9 @@ public interface CompactionInputSpec
* @param latestSegments most recent published segments in the
interval returned by {@link #findInterval}
*/
boolean validateSegments(LockGranularity lockGranularityInUse,
List<DataSegment> latestSegments);
+
+ /**
+ * Return the datasource to be used as input to the compaction task.
Review Comment:
```suggestion
* Returns the name of the datasource to ingest the compacted segments to.
```
##########
indexing-service/src/main/java/org/apache/druid/indexing/common/task/CompactionTask.java:
##########
@@ -242,7 +242,10 @@ public CompactionTask(
}
this.projections = projections;
this.tuningConfig = tuningConfig != null ? getTuningConfig(tuningConfig) :
null;
- this.segmentProvider = new SegmentProvider(dataSource,
this.ioConfig.getInputSpec());
+ this.segmentProvider = new SegmentProvider(
+ this.ioConfig.getInputSpec().getDataSource() == null ? dataSource :
this.ioConfig.getInputSpec().getDataSource(),
Review Comment:
```suggestion
Configs.getOrDefault(this.ioConfig.getInputSpec().getDataSource(),
dataSource)
```
##########
indexing-service/src/main/java/org/apache/druid/indexing/common/task/SpecificSegmentsSpec.java:
##########
@@ -61,6 +70,14 @@ public List<String> getSegments()
return segments;
}
+ @Override
+ @Nullable
+ @JsonProperty
+ public String getDataSource()
Review Comment:
nit: let's add some javadoc to this
##########
embedded-tests/src/test/java/org/apache/druid/testing/embedded/compact/CompactionTaskTest.java:
##########
@@ -87,7 +87,7 @@ public class CompactionTaskTest extends CompactionTestBase
() -> TaskBuilder
.ofTypeCompact()
.context("storeCompactionState", true)
- .ioConfig(new
CompactionIntervalSpec(Intervals.of("2013-08-31/2013-09-02"), null), false);
+ .ioConfig(new
CompactionIntervalSpec(Intervals.of("2013-08-31/2013-09-02"), null, null),
false);
Review Comment:
Let's add tests where this isn't null
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
