+1. Thanks Walker. On Wed, Dec 9, 2020 at 10:58 AM Walker Carlson <wcarl...@confluent.io> wrote:
> Sorry I forgot to change the subject line to vote. > > Thanks for the comments. If there are no further concerns I would like to > call for a vote on KIP-696 to clarify and clean up the Streams State > Machine. > > On Wed, Dec 9, 2020 at 10:04 AM Walker Carlson <wcarl...@confluent.io> > wrote: > > > Thanks for the comments. If there are no further concerns I would like to > > call for a vote on KIP-696 to clarify and clean up the Streams State > > Machine. > > > > walker > > > > On Wed, Dec 9, 2020 at 8:50 AM John Roesler <vvcep...@apache.org> wrote: > > > >> Thanks, Walker! > >> > >> Your proposal looks good to me. > >> > >> -John > >> > >> On Tue, 2020-12-08 at 18:29 -0800, Walker Carlson wrote: > >> > 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 > >> > > > >> > >> > >> > -- -- Guozhang
