FrankChen021 commented on code in PR #20738:
URL: https://github.com/apache/druid/pull/20738#discussion_r4400450250


##########
extensions-core/kubernetes-overlord-extensions/src/main/java/org/apache/druid/k8s/overlord/KubernetesTaskRunner.java:
##########
@@ -270,10 +271,18 @@ protected TaskStatus doTask(Task task, boolean run)
         }
 
         peonLifecycle = workItem.getPeonLifeycle();
+        isShutdown = workItem.isShutdown();
       }
 
       TaskStatus taskStatus;
-      if (run) {
+      if (isShutdown) {
+        // Shut down while waiting for a free thread.
+        if (!run) {
+          // A restored job already has a pod, so persist its logs and close 
the log watch opened by the shutdown.
+          peonLifecycle.saveLogs();
+        }
+        taskStatus = TaskStatus.failure(task.getId(), "Task was shut down 
before it started");

Review Comment:
   Thanks for the follow-up. I verified the current head across all 5 changed 
files: when shutdown wins before a restored lifecycle starts, 
`KubernetesPeonLifecycle.shutdown()` now moves it to `STOPPED` and completes 
`taskStartedSuccessfullyFuture` with `false`. That resolves the prior P2 and 
removes the startup-timeout path; I found no additional actionable issue.
   
   <!-- mergelens:review -->



-- 
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]

Reply via email to