Looks good, thanks Walker! +1 (non-binding) Leah
On Wed, Dec 9, 2020 at 1:04 PM John Roesler <vvcep...@apache.org> wrote: > Thanks, Walker! > > I'm also +1 (binding) > > -John > > On Wed, 2020-12-09 at 11:03 -0800, Guozhang Wang wrote: > > +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 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
