danprince1 commented on code in PR #13758:
URL: https://github.com/apache/druid/pull/13758#discussion_r1174037386
##########
indexing-service/src/main/java/org/apache/druid/indexing/common/task/IndexTask.java:
##########
@@ -573,7 +580,8 @@ private Map<String, TaskReport> getTaskCompletionReports()
getTaskCompletionRowStats(),
errorMsg,
segmentAvailabilityConfirmationCompleted,
- segmentAvailabilityWaitTimeMs
+ segmentAvailabilityWaitTimeMs,
+ JodaUtils.condenseIntervals(intervals)
Review Comment:
Hi, thanks for taking a look at this PR!
Making `ingestedIntervals` an attribute of the Task doesn't feel right to me
because this is a very transient thing. The instance variable would be empty
all the time except for a vanishingly small period of time after
`generateAndPublishSegments()` completes and before the task is torn down. We
only pass intervals to `getTaskCompletionReports()` in one place - we would
call the setter and then immediately call `getTaskCompletionReports()`, which
would call the getter. And no other entity has any interest in these intervals
- they are not really a property of the Task.
Let me know what you think.
--
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]
