ableegoldman commented on a change in pull request #11686:
URL: https://github.com/apache/kafka/pull/11686#discussion_r788504721



##########
File path: streams/src/main/java/org/apache/kafka/streams/KafkaStreams.java
##########
@@ -523,16 +528,21 @@ private boolean wrappedExceptionIsIn(final Throwable 
throwable, final Set<Class<
     }
 
     private void handleStreamsUncaughtException(final Throwable throwable,
-                                                final 
StreamsUncaughtExceptionHandler streamsUncaughtExceptionHandler) {
+                                                final 
StreamsUncaughtExceptionHandler streamsUncaughtExceptionHandler,
+                                                final boolean 
skipThreadReplacement) {
         final StreamsUncaughtExceptionHandler.StreamThreadExceptionResponse 
action = getActionForThrowable(throwable, streamsUncaughtExceptionHandler);
         if (oldHandler) {
             log.warn("Stream's new uncaught exception handler is set as well 
as the deprecated old handler." +
                     "The old handler will be ignored as long as a new handler 
is set.");
         }
         switch (action) {
             case REPLACE_THREAD:
-                log.error("Replacing thread in the streams uncaught exception 
handler", throwable);
-                replaceStreamThread(throwable);
+                if (!skipThreadReplacement) {
+                    log.error("Replacing thread in the streams uncaught 
exception handler", throwable);
+                    replaceStreamThread(throwable);
+                } else {
+                    log.debug("Skipping thread replacement for recoverable 
error");

Review comment:
       The thing is that the user _has_ to return something from the handler, 
there is no option to "do nothing" -- from the user perspective, the 
`REPLACE_THREAD` option essentially means "continue running the application", 
they don't care if we do so by literally replacing the thread or by just 
swallowing the exception. So imho it doesn't make sense to log as a warning 
because the user _should_ choose the `REPLACE_THREAD` option in this case.
   
   I think the confusion is around the name "REPLACE_THREAD" when we start to 
consider optimizations where we don't need to actually replace the thread 
because we don't need to kill it in the first place, unfortunately we're stuck 
with the name unless we want to deprecate it and choose a better one (eg rename 
REPLACE_THREAD --> CONTINUE_RUNNING -- does that make sense?)




-- 
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: jira-unsubscr...@kafka.apache.org

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


Reply via email to