kfaraz commented on code in PR #17535:
URL: https://github.com/apache/druid/pull/17535#discussion_r1888010080


##########
indexing-service/src/main/java/org/apache/druid/indexing/seekablestream/supervisor/SeekableStreamSupervisor.java:
##########
@@ -1103,6 +1104,18 @@ public void stop(boolean stopGracefully)
     }
   }
 
+  @Override
+  public ListenableFuture<Void> stopAsync(boolean stopGracefully)
+  {
+    ListeningExecutorService shutdownExec = MoreExecutors.listeningDecorator(
+        Execs.singleThreaded(StringUtils.encodeForFormat(supervisorId) + 
"-Shutdown-%d")

Review Comment:
   Nit: maybe use a common prefix for all shutdown threads so that it is easier 
to search for them.
   ```suggestion
           Execs.singleThreaded("supervisor-shutdown-" + 
StringUtils.encodeForFormat(supervisorId) + "--%d")
   ```



##########
server/src/main/java/org/apache/druid/indexing/overlord/supervisor/Supervisor.java:
##########
@@ -44,6 +46,14 @@ public interface Supervisor
    */
   void stop(boolean stopGracefully);
 
+  default ListenableFuture<Void> stopAsync(boolean stopGracefully)

Review Comment:
   Maybe add a javadoc.
   Also, does this need the `stopGracefully` parameter?



##########
indexing-service/src/main/java/org/apache/druid/indexing/seekablestream/supervisor/SeekableStreamSupervisor.java:
##########
@@ -1103,6 +1104,18 @@ public void stop(boolean stopGracefully)
     }
   }
 
+  @Override
+  public ListenableFuture<Void> stopAsync(boolean stopGracefully)
+  {
+    ListeningExecutorService shutdownExec = MoreExecutors.listeningDecorator(
+        Execs.singleThreaded(StringUtils.encodeForFormat(supervisorId) + 
"-Shutdown-%d")
+    );
+    return shutdownExec.submit(() -> {
+      stop(stopGracefully);
+      shutdownExec.shutdown();
+      return null;
+    });
+  }

Review Comment:
   Nit: add newline after this



##########
server/src/main/java/org/apache/druid/indexing/overlord/supervisor/Supervisor.java:
##########
@@ -44,6 +46,14 @@ public interface Supervisor
    */
   void stop(boolean stopGracefully);
 
+  default ListenableFuture<Void> stopAsync(boolean stopGracefully)
+  {
+    SettableFuture<Void> stopFuture = SettableFuture.create();
+    stop(stopGracefully);

Review Comment:
   If this method throws an exception, the future should be completed with the 
exception.



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