From 2086ff88bcdbfdba507d52d78bd3c7605beab280 Mon Sep 17 00:00:00 2001
From: Rishabh Singh <6513075+findingrish@users.noreply.github.com>
Date: Tue, 30 May 2023 18:50:52 +0530
Subject: [PATCH] Add logging for task stop operations (#14192)

Log more details when task cannot be stopped for various reasons
---
 .../SeekableStreamIndexTaskClientAsyncImpl.java      | 12 +++++++++---
 .../SeekableStreamIndexTaskClientSyncImpl.java       | 11 ++++++++++-
 .../indexing/worker/executor/ExecutorLifecycle.java  |  2 +-
 3 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/indexing-service/src/main/java/org/apache/druid/indexing/seekablestream/SeekableStreamIndexTaskClientAsyncImpl.java b/indexing-service/src/main/java/org/apache/druid/indexing/seekablestream/SeekableStreamIndexTaskClientAsyncImpl.java
index f124ca25ca4..2051d807b11 100644
--- a/indexing-service/src/main/java/org/apache/druid/indexing/seekablestream/SeekableStreamIndexTaskClientAsyncImpl.java
+++ b/indexing-service/src/main/java/org/apache/druid/indexing/seekablestream/SeekableStreamIndexTaskClientAsyncImpl.java
@@ -145,10 +145,16 @@ public abstract class SeekableStreamIndexTaskClientAsyncImpl<PartitionIdType, Se
   {
     return makeRequest(id, new RequestBuilder(HttpMethod.POST, "/stop" + (publish ? "?publish=true" : "")))
         .onSuccess(r -> true)
-        .onHttpError(e -> Either.value(false))
-        .onNotAvailable(e -> Either.value(false))
+        .onHttpError(e -> {
+          log.warn("Task [%s] coundln't be stopped because of http request failure [%s].", id, e.getMessage());
+          return Either.value(false);
+        })
+        .onNotAvailable(e -> {
+          log.warn("Task [%s] coundln't be stopped because it is not available.", id);
+          return Either.value(false);
+        })
         .onClosed(e -> {
-          log.debug("Task [%s] couldn't be stopped because it is no longer running.", id);
+          log.warn("Task [%s] couldn't be stopped because it is no longer running.", id);
           return Either.value(true);
         })
         .go();
diff --git a/indexing-service/src/main/java/org/apache/druid/indexing/seekablestream/SeekableStreamIndexTaskClientSyncImpl.java b/indexing-service/src/main/java/org/apache/druid/indexing/seekablestream/SeekableStreamIndexTaskClientSyncImpl.java
index 9f01a6f8c95..971908ac7dd 100644
--- a/indexing-service/src/main/java/org/apache/druid/indexing/seekablestream/SeekableStreamIndexTaskClientSyncImpl.java
+++ b/indexing-service/src/main/java/org/apache/druid/indexing/seekablestream/SeekableStreamIndexTaskClientSyncImpl.java
@@ -85,7 +85,16 @@ public abstract class SeekableStreamIndexTaskClientSyncImpl<PartitionIdType, Seq
           publish ? "publish=true" : null,
           true
       );
-      return isSuccess(response);
+      boolean isSuccess = isSuccess(response);
+      if (!isSuccess) {
+        log.warn(
+            "Task [%s] coundln't be stopped because of http request failure. Error code [%d], description [%s].",
+            id,
+            response.getStatus().getCode(),
+            response.getStatus().getReasonPhrase()
+        );
+      }
+      return isSuccess;
     }
     catch (NoTaskLocationException e) {
       return false;
diff --git a/indexing-service/src/main/java/org/apache/druid/indexing/worker/executor/ExecutorLifecycle.java b/indexing-service/src/main/java/org/apache/druid/indexing/worker/executor/ExecutorLifecycle.java
index b38ac0e2e89..ad46ee7e666 100644
--- a/indexing-service/src/main/java/org/apache/druid/indexing/worker/executor/ExecutorLifecycle.java
+++ b/indexing-service/src/main/java/org/apache/druid/indexing/worker/executor/ExecutorLifecycle.java
@@ -156,7 +156,7 @@ public class ExecutorLifecycle
             }
 
             // Kind of gross, but best way to kill the JVM as far as I know
-            log.info("Triggering JVM shutdown.");
+            log.info("Triggering JVM shutdown. Check overlord logs to see why the task is being shut down.");
             System.exit(2);
           }
       );