Thanks for the feedback Guozhang! I clarified some of the points in the Proposed Changes section so hopefully it will be more clear what is going on now. I also agree with your suggestion about the possible call to close() on ERROR so I added this line. "Close() called on ERROR will be idempotent and not throw an exception, but we will log a warning."
I have linked those tickets and I will leave a comment trying to explain how these changes will affect their issue. walker On Tue, Dec 8, 2020 at 4:57 PM Guozhang Wang <wangg...@gmail.com> wrote: > Hello Walker, > > Thanks for the KIP! Overall it looks reasonable to me. Just a few minor > comments for the wiki page itself: > > 1) Could you clarify the conditions when RUNNING / REBALANCING -> > PENDING_ERROR will happen; and when PENDING_ERROR -> ERROR will happen. > E.g. when I read "Streams will only reach ERROR state in the event of an > exceptional failure in which the `StreamsUncaughtExceptionHandler` chose to > either shutdown the application or the client." I thought the first > transition would happen before the handler, and the second transition would > happen immediately after the handler returns "shutdown client" or "shutdown > application", until I read the last statement regarding "SHUTDOWN_CLIENT". > > 2) A compatibility issue: today it is possible that users would call > Streams APIs like shutdown in the global state transition listener. And > it's common to try shutting down the application automatically when > transiting to ERROR (assuming it was not a terminating state). I think we > could consider making this call a no-op and log a warning. > > 3) Could you link the following JIRAs in the "JIRA" field? > > https://issues.apache.org/jira/browse/KAFKA-10555 > https://issues.apache.org/jira/browse/KAFKA-9638 > https://issues.apache.org/jira/browse/KAFKA-6520 > > And maybe we can also left a comment on those tickets explaining what would > happen to tackle the issues after this KIP. > > > Guozhang > > > On Tue, Dec 8, 2020 at 12:16 PM Walker Carlson <wcarl...@confluent.io> > wrote: > > > Hello all, > > > > I'd like to propose KIP-696 to clarify the meaning of ERROR state in the > > KafkaStreams Client State Machine. This will update the States to be > > consistent with changes in KIP-671 and KIP-663. > > > > Here are the details: https://cwiki.apache.org/confluence/x/lCvZCQ > > > > Thanks, > > Walker > > > > > -- > -- Guozhang >
