kfaraz commented on code in PR #18039:
URL: https://github.com/apache/druid/pull/18039#discussion_r2121201587
##########
indexing-service/src/main/java/org/apache/druid/indexing/common/task/batch/parallel/TaskMonitor.java:
##########
@@ -134,6 +136,25 @@ public void start(long taskStatusCheckingPeriod)
final MonitorEntry monitorEntry = entry.getValue();
final String taskId = monitorEntry.runningTask.getId();
+ final long timeout = taskTimeoutMillis;
+ final long elapsed =
monitorEntry.getStopwatch().millisElapsed();
+ if (timeout > 0 && elapsed > timeout) {
+ log.warn("task[%s] timed out after %s ms, cancelling...",
taskId, elapsed);
+ FutureUtils.getUnchecked(overlordClient.cancelTask(taskId),
true);
+ final TaskStatusPlus cancelledTaskStatus =
FutureUtils.getUnchecked(
+ overlordClient.taskStatus(taskId), true).getStatus();
+ reportsMap.remove(taskId);
+ incrementNumFailedTasks();
+
+ if (monitorEntry.numTries() < maxRetry) {
+ retry(specId, monitorEntry, cancelledTaskStatus);
Review Comment:
Do you mean a deadlock in the code?
In that case, I think that failing the tasks is even more important.
Otherwise, we may go on retrying silently and never catch the problem in the
code.
I would advise that we avoid doing the retries in the first iteration of
this config,
and treat the `subTaskTimeoutMillis` as a hard stop for any given sub-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]
