capistrant commented on code in PR #18587:
URL: https://github.com/apache/druid/pull/18587#discussion_r2399033413


##########
extensions-core/kubernetes-overlord-extensions/src/main/java/org/apache/druid/k8s/overlord/KubernetesPeonLifecycle.java:
##########
@@ -343,52 +343,71 @@ protected void startWatchingLogs()
       log.debug("There is already a log watcher for %s", 
taskId.getOriginalTaskId());
       return;
     }
+    ExecutorService executor = 
Executors.newSingleThreadExecutor(Execs.makeThreadFactory("k8s-tasklog-init-watch-%d"));
     try {
-      Optional<LogWatch> maybeLogWatch = 
kubernetesClient.getPeonLogWatcher(taskId);
+      Future<Optional<LogWatch>> future = executor.submit(() -> 
kubernetesClient.getPeonLogWatcher(taskId));
+      Optional<LogWatch> maybeLogWatch = 
future.get(logWatchOperationTimeoutMs, TimeUnit.MILLISECONDS);
       if (maybeLogWatch.isPresent()) {
         logWatch = maybeLogWatch.get();
       }
     }
-    catch (Exception e) {
-      log.error(e, "Error watching logs from task: %s", taskId);
-    }
-  }
-
-  protected void saveLogs()
-  {
-    ExecutorService executor = 
Executors.newSingleThreadExecutor(Execs.makeThreadFactory("k8s-tasklog-persist-%d"));
-    try {
-      Future<?> future = executor.submit(this::doSaveLogs);
-      future.get(logSaveTimeoutMs, TimeUnit.MILLISECONDS);
-    }
     catch (TimeoutException e) {
-      log.warn("Persisting task logs timed out after %d ms for task [%s]. This 
does not have any impact on the"
-               + " work done by the task, but the logs may be innaccessible. 
If this continues to happen, check"
-               + " Kubernetes server logs for potential errors.", 
logSaveTimeoutMs, taskId.getOriginalTaskId());
+      log.warn("Initializing log watcher timed out after %d ms for task [%s]. 
LogWatch not initialized.",
+               logWatchOperationTimeoutMs, taskId.getOriginalTaskId());
+    }
+    catch (InterruptedException e) {
+      Thread.currentThread().interrupt();

Review Comment:
   hmmm. I guess this may just be force of habit to be defensive and do this 
whenever I catch an interrupted exception. Is there negatives to it even if the 
status isn't read?



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