[GitHub] [flink] dmvk commented on a diff in pull request #22678: [FLINK-32204][runtime] Separates the DirectExecutorService from the DirectExecutor again.

["dmvk (via GitHub)" <gi...@apache.org>]

dmvk commented on code in PR #22678:
URL: https://github.com/apache/flink/pull/22678#discussion_r1211260209


##########
flink-core/src/main/java/org/apache/flink/util/concurrent/DirectExecutorService.java:
##########
@@ -39,10 +41,37 @@
  * thread.
  */
 class DirectExecutorService implements ExecutorService {
-    static final DirectExecutorService INSTANCE = new DirectExecutorService();
+
+    private final ThrowingRunnable<? extends RuntimeException> ifShutdownAction;

Review Comment:
   Wouldn't a boolean flag do the trick as well + make this easier to interpret? :)



##########
flink-core/src/main/java/org/apache/flink/util/concurrent/DirectExecutorService.java:
##########
@@ -39,10 +41,37 @@
  * thread.
  */
 class DirectExecutorService implements ExecutorService {
-    static final DirectExecutorService INSTANCE = new DirectExecutorService();
+
+    private final ThrowingRunnable<? extends RuntimeException> ifShutdownAction;
 
     private boolean isShutdown = false;
 
+    /**
+     * Creates a more restrictive {@code DirectExecutorService} implementation that fails with a
+     * {@link RejectedExecutionException} if a task is submitted while the instance is already shut
+     * down which is closer to the {@link ExecutorService} contract. This kind of {@code
+     * DirectExecutorService} might be preferable if you want to
+     */
+    static DirectExecutorService createDirectExecutorService() {
+        return new DirectExecutorService(
+                () -> {
+                    throw new RejectedExecutionException(
+                            "The ExecutorService is shut down already. No Callables can be executed.");
+                });
+    }
+
+    /**
+     * Creates a {@code DirectExecutorService} without proper shutdown contract execution, i.e.
+     * shutting down the executor doesn't have an influence on the execution of tasks.
+     */
+    static DirectExecutorService createDirectExecutorServiceWithoutTaskRejection() {

Review Comment:
   "TaskRejection" is an implementation detail; this should be named along the lines of "missing lifecycle" / "always on" ...
   



-- 
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: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org