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:
[email protected]
