Thank you everyone, KIP-696 has passed with 3 binding votes (Guozhang, John and Sophie) and 2 non-binding votes (Leah and Bruno)
walker On Thu, Dec 10, 2020 at 11:00 AM Sophie Blee-Goldman <sop...@confluent.io> wrote: > 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 > > >>>>>>>> > > >>>>>> > > >>>>>> > > >>>>>> > > >>>> > > >>> > > >>> > > >> > > >> > > >> > > > > > >