Re: [VOTE] KIP-696: Update Streams FSM to clarify ERROR state meaning

2020-12-14 Thread Walker Carlson
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 
wrote:

> KIP looks good to me, thanks Walker!
>
> +1 (binding)
>
> -Sophie
>
> On Thu, Dec 10, 2020 at 1:53 AM Bruno Cadonna  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 
> 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 
> > >> 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  > >>>
> > >> 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 

Re: [VOTE] KIP-696: Update Streams FSM to clarify ERROR state meaning

2020-12-10 Thread Sophie Blee-Goldman
KIP looks good to me, thanks Walker!

+1 (binding)

-Sophie

On Thu, Dec 10, 2020 at 1:53 AM Bruno Cadonna  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  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 
> >>> 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  >
>  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 
> >> 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  >>>
> >> 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
> 
> >>
> >>
> >>
> 
> >>>
> >>>
> >>
> >>
> >>
> >
>


Re: [VOTE] KIP-696: Update Streams FSM to clarify ERROR state meaning

2020-12-10 Thread Bruno Cadonna

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  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 
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 
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 

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 


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


















Re: [VOTE] KIP-696: Update Streams FSM to clarify ERROR state meaning

2020-12-09 Thread Leah Thomas
Looks good, thanks Walker! +1 (non-binding)

Leah

On Wed, Dec 9, 2020 at 1:04 PM John Roesler  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 
> > 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 
> > > 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 
> 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  >
> > > > > 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
> > > > > > >
> > > > >
> > > > >
> > > > >
> > >
> >
> >
>
>
>


Re: [VOTE] KIP-696: Update Streams FSM to clarify ERROR state meaning

2020-12-09 Thread John Roesler
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 
> 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 
> > 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  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 
> > > > 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
> > > > > > 
> > > > 
> > > > 
> > > > 
> > 
> 
> 




Re: [VOTE] KIP-696: Update Streams FSM to clarify ERROR state meaning

2020-12-09 Thread Guozhang Wang
+1. Thanks Walker.

On Wed, Dec 9, 2020 at 10:58 AM Walker Carlson 
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 
> 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  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 
> >> 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


Re: [VOTE] KIP-696: Update Streams FSM to clarify ERROR state meaning

2020-12-09 Thread Walker Carlson
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 
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  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 
>> 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 > >
>> > > 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
>> > >
>>
>>
>>