You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Walker Carlson <wc...@confluent.io> on 2020/12/08 20:15:30 UTC
[DISCUSS] KIP-696: Update Streams FSM to clarify ERROR state meaning
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
Re: [VOTE] KIP-696: Update Streams FSM to clarify ERROR state meaning
Posted by Walker Carlson <wc...@confluent.io>.
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 <so...@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 <vv...@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 <
> wcarlson@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 <
> wcarlson@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 <vv...@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 <wangguoz@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 <
> > >>>> wcarlson@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
> > >>>>>>>>
> > >>>>>>
> > >>>>>>
> > >>>>>>
> > >>>>
> > >>>
> > >>>
> > >>
> > >>
> > >>
> > >
> >
>
Re: [VOTE] KIP-696: Update Streams FSM to clarify ERROR state meaning
Posted by Sophie Blee-Goldman <so...@confluent.io>.
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 <vv...@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 <wc...@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 <wcarlson@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 <vv...@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 <wangguoz@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 <
> >>>> wcarlson@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
> >>>>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>
> >>>
> >>>
> >>
> >>
> >>
> >
>
Re: [VOTE] KIP-696: Update Streams FSM to clarify ERROR state meaning
Posted by Bruno Cadonna <br...@confluent.io>.
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 <vv...@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 <wc...@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 <wc...@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 <vv...@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 <wangguoz@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 <
>>>> wcarlson@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
>>>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>
>>>
>>>
>>
>>
>>
>
Re: [VOTE] KIP-696: Update Streams FSM to clarify ERROR state meaning
Posted by Leah Thomas <lt...@confluent.io>.
Looks good, thanks Walker! +1 (non-binding)
Leah
On Wed, Dec 9, 2020 at 1:04 PM John Roesler <vv...@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 <wc...@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 <wc...@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 <vv...@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 <wangguoz@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 <
> > > wcarlson@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
> > > > > > >
> > > > >
> > > > >
> > > > >
> > >
> >
> >
>
>
>
Re: [VOTE] KIP-696: Update Streams FSM to clarify ERROR state
meaning
Posted by John Roesler <vv...@apache.org>.
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 <wc...@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 <wc...@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 <vv...@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 <wa...@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 <
> > wcarlson@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
> > > > > >
> > > >
> > > >
> > > >
> >
>
>
Re: [VOTE] KIP-696: Update Streams FSM to clarify ERROR state meaning
Posted by Guozhang Wang <wa...@gmail.com>.
+1. Thanks Walker.
On Wed, Dec 9, 2020 at 10:58 AM Walker Carlson <wc...@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 <wc...@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 <vv...@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 <wa...@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 <
> wcarlson@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
Re: [VOTE] KIP-696: Update Streams FSM to clarify ERROR state meaning
Posted by Walker Carlson <wc...@confluent.io>.
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 <wc...@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 <vv...@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 <wa...@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 <wcarlson@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
>> > >
>>
>>
>>
Re: [DISCUSS] KIP-696: Update Streams FSM to clarify ERROR state meaning
Posted by Walker Carlson <wc...@confluent.io>.
Thanks for the comets Matthias.
I respond inline below.
Thanks,
walker
On Sat, Dec 19, 2020 at 11:35 AM Matthias J. Sax <mj...@apache.org> wrote:
> Overall LGTM.
>
> A few minor comments:
>
> > The SHUTDOWN_CLIENT option in the Streams Uncaught Exception Handler
> should leave the client state in ERROR instead of NOT_RUNNING
>
> and
>
> > In order to be consistent, SHUTDOWN_CLIENT will leave the client state
> in ERROR instead of NOT_RUNNING
>
> Both should also apply to SHUTDOWN_APPLICATION? If not, why?
>
I totally agree. This is already the behavior of SHUTDOWN_APPLICATION we
are just bringing SHUTDOWN_CLIENT to match.
>
>
> > Close() called on ERROR or PENDING_ERROR will be idempotent
>
> Should we replae `idempotent` with `a no-op`, because the difference is
> that `close()` would normally transit to NOT_RUNNING?
>
That is fine with me.
>
> The Jira links to 3 tickets, but uses different markup. That is
> confusing. Also, I actually believe that 6520 is unrelated?
>
I'll fix the mark up, I think that this kip changes how that 6520 is
handled but it probably doesn't need to be on the list. I have added a
comment on the ticket to notify anyone that related behavior has changed
>
>
> -Matthias
>
>
> On 12/10/20 1:52 AM, Bruno Cadonna wrote:
> > Thanks for the KIP, Walker!
> >
> > The KIP looks good to me. I have just a minor comment about the KIP
> > document.
> >
> > You talk about SHUTDOWN_CLIENT in the KIP, but never explain that it is
> > a possible action that can be taken in the Streams uncaught exception
> > handler. Could you please clarify that?
> >
> > Best,
> > Bruno
> >
> > On 09.12.20 19:04, Walker Carlson 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 <vv...@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 <wa...@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 <
> wcarlson@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
> >>>>>
> >>>
> >>>
> >>>
> >>
>
Re: [DISCUSS] KIP-696: Update Streams FSM to clarify ERROR state
meaning
Posted by "Matthias J. Sax" <mj...@apache.org>.
Overall LGTM.
A few minor comments:
> The SHUTDOWN_CLIENT option in the Streams Uncaught Exception Handler should leave the client state in ERROR instead of NOT_RUNNING
and
> In order to be consistent, SHUTDOWN_CLIENT will leave the client state in ERROR instead of NOT_RUNNING
Both should also apply to SHUTDOWN_APPLICATION? If not, why?
> Close() called on ERROR or PENDING_ERROR will be idempotent
Should we replae `idempotent` with `a no-op`, because the difference is
that `close()` would normally transit to NOT_RUNNING?
The Jira links to 3 tickets, but uses different markup. That is
confusing. Also, I actually believe that 6520 is unrelated?
-Matthias
On 12/10/20 1:52 AM, Bruno Cadonna wrote:
> Thanks for the KIP, Walker!
>
> The KIP looks good to me. I have just a minor comment about the KIP
> document.
>
> You talk about SHUTDOWN_CLIENT in the KIP, but never explain that it is
> a possible action that can be taken in the Streams uncaught exception
> handler. Could you please clarify that?
>
> Best,
> Bruno
>
> On 09.12.20 19:04, Walker Carlson 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 <vv...@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 <wa...@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 <wc...@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
>>>>>
>>>
>>>
>>>
>>
Re: [DISCUSS] KIP-696: Update Streams FSM to clarify ERROR state
meaning
Posted by Bruno Cadonna <br...@confluent.io>.
Thanks for the KIP, Walker!
The KIP looks good to me. I have just a minor comment about the KIP
document.
You talk about SHUTDOWN_CLIENT in the KIP, but never explain that it is
a possible action that can be taken in the Streams uncaught exception
handler. Could you please clarify that?
Best,
Bruno
On 09.12.20 19:04, Walker Carlson 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 <vv...@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 <wa...@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 <wc...@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
>>>>
>>
>>
>>
>
Re: [DISCUSS] KIP-696: Update Streams FSM to clarify ERROR state meaning
Posted by Walker Carlson <wc...@confluent.io>.
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 <vv...@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 <wa...@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 <wc...@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
> > >
>
>
>
Re: [DISCUSS] KIP-696: Update Streams FSM to clarify ERROR state
meaning
Posted by John Roesler <vv...@apache.org>.
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 <wa...@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 <wc...@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
> >
Re: [DISCUSS] KIP-696: Update Streams FSM to clarify ERROR state meaning
Posted by Walker Carlson <wc...@confluent.io>.
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 <wa...@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 <wc...@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
>
Re: [DISCUSS] KIP-696: Update Streams FSM to clarify ERROR state meaning
Posted by Guozhang Wang <wa...@gmail.com>.
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 <wc...@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