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



##########
File path: 
streams/src/main/java/org/apache/kafka/streams/processor/internals/GlobalStreamThread.java
##########
@@ -311,6 +314,8 @@ public void run() {
                 "Updating global state failed. You can restart KafkaStreams to 
recover from this error.",
                 recoverableException
             );
+        } catch (final Exception e) {
+            this.streamsUncaughtExceptionHandler.accept(e);

Review comment:
       Oh you're totally right, sorry for letting my paranoia start spreading 
conspiracy theories here 🙂  Given all this I'd still claim that the FSM is in 
need to being cleaned up a bit (or a lot), but if you'd prefer to hold off on 
that until the add thread work then I'm all good here. Thanks for humoring me 
and explaining the state of things. I just wanted/want to make sure we don't 
overlook anything, since there's a lot going on.
   
   For example in the current code, if the global thread dies with the old 
handler still in use then we'll transition to ERROR. However the user still has 
to be responsible for closing the client themselves, and it will ultimately 
transition from ERROR to NOT_RUNNING. Whereas if we transition to ERROR as the 
result of a SHUTDOWN_APPLICATION error code, the user should NOT try to invoke 
close themselves, and the ERROR state will be terminal. That's pretty confusing 
eg for users who use a state listener and wait for the transition to ERROR to 
call close(). We should make sure that ERROR has the same semantics across the 
board by the end of all this work.
   
   Anyways I'm just thinking out loud here, to reiterate I'm perfectly happy to 
merge this as-is. But for reasons like the above, I think it's important to 
tackle the FSM in the next PR and make sure it all gets sorted out by the next 
AK release




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

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


Reply via email to