LakshSingla commented on code in PR #13706:
URL: https://github.com/apache/druid/pull/13706#discussion_r1093878894
##########
extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/exec/ControllerImpl.java:
##########
@@ -1247,48 +1248,67 @@ private void postResultPartitionBoundariesForStage(
/**
* Publish the list of segments. Additionally, if {@link
DataSourceMSQDestination#isReplaceTimeChunks()},
* also drop all other segments within the replacement intervals.
- * <p>
- * If any existing segments cannot be dropped because their intervals are
not wholly contained within the
- * replacement parameter, throws a {@link MSQException} with {@link
InsertCannotReplaceExistingSegmentFault}.
*/
private void publishAllSegments(final Set<DataSegment> segments) throws
IOException
{
final DataSourceMSQDestination destination =
(DataSourceMSQDestination) task.getQuerySpec().getDestination();
- final Set<DataSegment> segmentsToDrop;
+ Set<DataSegment> segmentsWithTombstones = new HashSet<>(segments);
if (destination.isReplaceTimeChunks()) {
final List<Interval> intervalsToDrop =
findIntervalsToDrop(Preconditions.checkNotNull(segments, "segments"));
- if (intervalsToDrop.isEmpty()) {
- segmentsToDrop = null;
- } else {
- // Determine which segments to drop as part of the replace operation.
This is safe because, in the case where we
- // are doing a replace, the isReady method (which runs prior to the
task starting) acquires an exclusive lock.
- segmentsToDrop =
- ImmutableSet.copyOf(
- context.taskActionClient().submit(
- new RetrieveUsedSegmentsAction(
- task.getDataSource(),
- null,
- intervalsToDrop,
- Segments.ONLY_VISIBLE
- )
- )
+ if (!intervalsToDrop.isEmpty()) {
Review Comment:
There are some minute differences between how the TombstoneHelper expects
the arguments v/s as to how the MSQ is generating the segments:
The TombstoneHelper is using the DataSchema and its granularitySpec to
compute the empty segments, v/s here we have the empty intervals for which we
know that the segments corresponding to it should be empty, due to which I
wasn't able to reconcile the code paths cleanly. One way was to create a dummy
data schema corresponding to the empty intervals.
Also, the `pushedSegments` argument in the helper was of no use since we
know the empty intervals in replace, therefore we would also need to dummy that
to something which would never overlap. Due to these, I decided to drop the
usage of TombstoneHelper
--
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]
