KIP looks good to me, thanks Walker! +1 (binding)
-Sophie On Thu, Dec 10, 2020 at 1:53 AM Bruno Cadonna <br...@confluent.io> wrote: > Thanks, Walker! > > +1 (non-binding) > > Best, > Bruno > > On 09.12.20 20:07, Leah Thomas wrote: > > 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 > >>>>>>>> > >>>>>> > >>>>>> > >>>>>> > >>>> > >>> > >>> > >> > >> > >> > > >
