fix forking task runner task shutdown to be more graceful #8085
Conversation
| @@ -714,6 +701,21 @@ private void saveRunningTasks() | |||
| } | |||
| } | |||
|
|
|||
| private void shutdownTaskProcess(ForkingTaskRunnerWorkItem taskInfo) | |||
There was a problem hiding this comment.
Would you add a comment like "this method triggers lifecycle.stop()"?
There was a problem hiding this comment.
It sort of seems like a misleading comment, since it doesn't call that directly, it's just closing the stream of the forked task and destroying it if that encounters an exception. Also since I am not entirely sure that lifecycle stop wasn't being called previously...
There was a problem hiding this comment.
That said, I agree it probably needs some sort of javadoc/comment, i'll see what I can come up with
|
Removed "fixes" from description because this isn't really a complete fix for that issue. |
I did a test on HadoopindexTask where I put the following code into The file did get written with the contents, so this does seem to be a logging-only issue. |
|
Thanks for review 👍 |
* fix forking task runner shutdown to be more graceful * javadoc
* fix forking task runner shutdown to be more graceful * javadoc
* fix forking task runner shutdown to be more graceful * javadoc
…#8085 in apache druid)
Description
This PR modifies
ForkingTaskRunner.shutdownto behave in the same manner asForkingTaskRunner.stop, which closes the output stream to the forked task instead of calling destroy directly, which retains the log output on that stream allowing it to be fully written before terminating the task.I am unable to definitively determine if this means the task is not waiting for lifecycle stop to complete, or if it's strictly a logging issue, as I have collected conflicting observations.This appears to be strictly a logging issue.Background
While looking into #8075 and #7962, noticed that the task logs stop abruptly when
ForkingTaskRunner.shutdownis called, with an exception appearing in the middle-manager logs:and the task log output just ends, without the normal lifecycle stopping messaging:
However, terminating the entire middle-manager does retain these logs for the tasks.
middle-manager log during entire middle-manager shutdown:
and the task logs have their own lifecycle stop messaging:
After modifying
ForkingTaskRunner.shutdownto use the same mechanism to stop the tasks asForkingTaskRunner.stop, the IOException is no longer there, and the task logs retain their shutdown messaging.middle-manager log:
and the task log has the same lifecycle stop messaging that appears when the entire middle-manager is terminated, and the tasks still retain the 'failed' status.
This PR has: