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/10/12 21:32:06 UTC

Re: [VOTE] KIP-671: Add method to Shutdown entire Streams Application

Hello all,

I just wanted to let you know that I had to make 2 minor updates to the KIP.

1) I changed the behavior of the shutdown client to not leave the client in
Error but instead close directly because this aligns better with our
state machine.

2) I added a separate call back for the global thread as it does not have
all the options as a streamThread does. i.e. replace. The default will be
to close the client. that will also be the only option as that is the
current behavior for the global thread.

you can find the diff here:
https://cwiki.apache.org/confluence/pages/diffpagesbyversion.action?pageId=158876566&originalVersion=21&revisedVersion=23

If you have any problems with these changes let me know and we can discuss
them further

Thank you,
Walker

On Wed, Sep 30, 2020 at 7:33 AM Walker Carlson <wc...@confluent.io>
wrote:

>
> Bruno Cadonna <br...@confluent.io>
> 4:51 AM (2 hours ago)
> to dev
> Thank you all for voting!
>
> This KIP is accepted with +3 binding (Guozhang, Bill, Matthias) and +2
> non-binding (Bruno, Leah).
>
> Matthias, we will take care of  the global threads, and for the
> replacement that will depend on Kip-663.
>
> Best,
>
> On Wed, Sep 30, 2020 at 4:59 AM Bruno Cadonna <br...@confluent.io> wrote:
>
>> Thanks a lot Walker!
>>
>> +1 (non-binding)
>>
>> Best,
>> Bruno
>>
>> On 30.09.20 03:10, Matthias J. Sax wrote:
>> > Thanks Walker. The proposed API changes LGTM.
>> >
>> > +1 (binding)
>> >
>> > One minor nit: you should also mention the global-thread that also needs
>> > to be shutdown if requested by the user.
>> >
>> > Minor side question: should we actually terminate a thread and create a
>> > new one, or instead revive the existing thread (reusing its existing
>> ID)?
>> >
>> >
>> > -Matthias
>> >
>> > On 9/29/20 2:39 PM, Bill Bejeck wrote:
>> >> Thanks for the KIP Walker.
>> >>
>> >> +1 (binding)
>> >>
>> >> -Bill
>> >>
>> >> On Tue, Sep 29, 2020 at 4:59 PM Guozhang Wang <wa...@gmail.com>
>> wrote:
>> >>
>> >>> +1 again on the KIP.
>> >>>
>> >>> On Tue, Sep 29, 2020 at 1:51 PM Leah Thomas <lt...@confluent.io>
>> wrote:
>> >>>
>> >>>> Hey Walker,
>> >>>>
>> >>>> Thanks for the KIP! I'm +1, non-binding.
>> >>>>
>> >>>> Cheers,
>> >>>> Leah
>> >>>>
>> >>>> On Tue, Sep 29, 2020 at 1:56 PM Walker Carlson <
>> wcarlson@confluent.io>
>> >>>> wrote:
>> >>>>
>> >>>>> Hello all,
>> >>>>>
>> >>>>> I made some changes to the KIP the descriptions are on the
>> discussion
>> >>>>> thread. If you have already voted I would ask you to confirm your
>> vote.
>> >>>>>
>> >>>>> Otherwise please vote so we can get this feature in.
>> >>>>>
>> >>>>> Thanks,
>> >>>>> Walker
>> >>>>>
>> >>>>> On Thu, Sep 24, 2020 at 4:36 PM John Roesler <vv...@apache.org>
>> >>>> wrote:
>> >>>>>
>> >>>>>> Thanks for the KIP, Walker!
>> >>>>>>
>> >>>>>> I’m +1 (binding)
>> >>>>>>
>> >>>>>> -John
>> >>>>>>
>> >>>>>> On Mon, Sep 21, 2020, at 17:04, Guozhang Wang wrote:
>> >>>>>>> Thanks for finalizing the KIP. +1 (binding)
>> >>>>>>>
>> >>>>>>>
>> >>>>>>> Guozhang
>> >>>>>>>
>> >>>>>>> On Mon, Sep 21, 2020 at 1:38 PM Walker Carlson <
>> >>>> wcarlson@confluent.io>
>> >>>>>>> wrote:
>> >>>>>>>
>> >>>>>>>> Hello all,
>> >>>>>>>>
>> >>>>>>>> I would like to start a thread to vote for KIP-671 to add a
>> >>> method
>> >>>> to
>> >>>>>> close
>> >>>>>>>> all clients in a kafka streams application.
>> >>>>>>>>
>> >>>>>>>> KIP:
>> >>>>>>>>
>> >>>>>>>>
>> >>>>>>
>> >>>>>
>> >>>>
>> >>>
>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-671%3A+Shutdown+Streams+Application+when+appropriate+exception+is+thrown
>> >>>>>>>>
>> >>>>>>>> Discussion thread: *here
>> >>>>>>>> <
>> >>>>>>>>
>> >>>>>>
>> >>>>>
>> >>>>
>> >>>
>> https://mail-archives.apache.org/mod_mbox/kafka-dev/202009.mbox/%3CCAC55fuh3HAGCxz-PzxTJraczy6T-os2oiCV328PBeuJQSVYASg%40mail.gmail.com%3E
>> >>>>>>>>> *
>> >>>>>>>>
>> >>>>>>>> Thanks,
>> >>>>>>>> -Walker
>> >>>>>>>>
>> >>>>>>>
>> >>>>>>>
>> >>>>>>> --
>> >>>>>>> -- Guozhang
>> >>>>>>>
>> >>>>>>
>> >>>>>
>> >>>>
>> >>>
>> >>>
>> >>> --
>> >>> -- Guozhang
>> >>>
>> >>
>>
>

Re: [VOTE] KIP-671: Add method to Shutdown entire Streams Application

Posted by "Matthias J. Sax" <mj...@apache.org>.

+1

On 10/19/20 12:33 PM, John Roesler wrote:
> Thanks, Walker.
> 
> That change looks good to me.
> 
> -John
> 
> On Mon, 2020-10-19 at 12:06 -0700, Walker Carlson wrote:
>> Hello all,
>>
>> Taking into account the feedback about that last change I have removed some
>> of the changes and no longer will we have a separate handler for the global
>> thread. To make it so we can align the handlers there will also be no
>> option to just remove a stream thread.
>>
>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-671%3A+Introduce+Kafka+Streams+Specific+Uncaught+Exception+Handler
>>
>> If you have any concerns please let me know,
>> Walker
>>
>> On Wed, Oct 14, 2020 at 12:51 PM John Roesler <vv...@apache.org> wrote:
>>
>>> Thanks, Sophie,
>>>
>>> That makes sense. Should we add a whole new interface and a
>>> separate kind of listener just because global threads don't
>>> support restarts _yet_, though?
>>>
>>> It seems like that will just widen the API surface area, and
>>> in a few more months, there will be no more difference
>>> between the two handlers. But people will forever afterward
>>> have to register two different handlers to do the same
>>> thing.
>>>
>>> Two alternatives we could consider:
>>> 1. Just don't add the "restart" option until it's possible
>>> for all threads. This KIP is already accepted with a
>>> "restart" option that we know won't be added until KIP-663
>>> is done. Maybe we just wait for KIP-406 as well. But the
>>> _rest_ of this KIP can be implemented in the mean time.
>>> 2. Just log an error and kill the thread anyway if the
>>> handler for the global thread opts to "retry".
>>>
>>> In general, it seems like the problem at hand is better
>>> solved by allowing/disallowing that one option, versus
>>> adding a whole new interface.
>>>
>>> Thanks,
>>> -John
>>>
>>> On Wed, 2020-10-14 at 11:48 -0700, Sophie Blee-Goldman
>>> wrote:
>>>> I don't think the proposal was to *never* add the "replace" functionality
>>>> for
>>>> the global thread, but we didn't want to tie up this KIP with anything
>>> more.
>>>> As I understand it, the goal of Walker's proposal was to set us up for
>>>> success if/when we want to add new functionality for the global thread,
>>>> without necessarily committing to it at this time.
>>>>
>>>> Restarting the global thread will take a bit more work since you need to
>>>> pause any further work that relies on global state until it's back up.
>>>> That's
>>>> starting to sound more in the purview of KIP-406 whose current goal
>>>> is to effectively restart the global thread on a specific type of
>>> exception
>>>> (OffsetOutOfRange). If we want to consider expanding that to allow users
>>>> to choose to restart the thread, then KIP-406 seems like the more
>>>> appropriate place to engage in that discussion.
>>>>
>>>> On Wed, Oct 14, 2020 at 7:13 AM John Roesler <vv...@apache.org>
>>> wrote:
>>>>> Hello Walker,
>>>>>
>>>>> Sorry for the late reply, but I didn’t follow the reasoning for the
>>>>> separate handler. You said that the global thread doesn’t have
>>> “replace”,
>>>>> but as of today, none of the threads have “replace”. Why not add that
>>>>> ability when we add it for the other threads?
>>>>>
>>>>> The nature of an uncaught exception handler is that there is an
>>> exception
>>>>> that will kill the thread. In that case, it seems like replacement is a
>>>>> desirable option.
>>>>>
>>>>> What have I missed?
>>>>>
>>>>> Thanks,
>>>>> John
>>>>>
>>>>> On Tue, Oct 13, 2020, at 15:49, Walker Carlson wrote:
>>>>>> Those are good points Sophie and Matthias. I sepificed the defaults
>>> in
>>>>> the
>>>>>> kip and standardized the names fo the handler to make them a bit more
>>>>>> readable.
>>>>>>
>>>>>> Thanks for the suggestions,
>>>>>> Walker
>>>>>>
>>>>>> On Tue, Oct 13, 2020 at 12:24 PM Sophie Blee-Goldman <
>>>>> sophie@confluent.io>
>>>>>> wrote:
>>>>>>
>>>>>>> Super nit: can we standardize the method & enum names?
>>>>>>>
>>>>>>> Right now we have these enums:
>>>>>>> StreamsUncaughtExceptionHandlerResponse
>>>>>>> StreamsUncaughtExceptionHandlerResponseGlobalThread
>>>>>>>
>>>>>>> and these callbacks:
>>>>>>> handleUncaughtException()
>>>>>>> handleExceptionInGlobalThread()
>>>>>>>
>>>>>>> The method names have different syntax, which is a bit clunky. I
>>> don't
>>>>> have
>>>>>>> any
>>>>>>> strong opinions on what grammar they should follow, just that it
>>>>> should be
>>>>>>> the
>>>>>>> same for each. I also think that we should specify "StreamThread"
>>>>> somewhere
>>>>>>> in the name of the StreadThread-specific callback, now that we
>>> have a
>>>>>>> second
>>>>>>> callback that specifies it's for the GlobalThread. Something like
>>>>>>> "*handleStreamThreadException()*" and
>>> "*handleGlobalThreadException*"
>>>>>>> The enums are ok, although I think we should include "StreamThread"
>>>>>>> somewhere
>>>>>>> like with the callbacks. And we can probably shorten them a bit.
>>> For
>>>>>>> example
>>>>>>> "*StreamThreadExceptionResponse*" and
>>> "*GlobalThreadExceptionResponse*"
>>>>>>>
>>>>>>>
>>>>>>> On Tue, Oct 13, 2020 at 11:48 AM Matthias J. Sax <mjsax@apache.org
>>>>> wrote:
>>>>>>>> Thanks Walker.
>>>>>>>>
>>>>>>>> Overall, LGTM. However, I am wondering if we should have default
>>>>>>>> implementations for both handler methods? Before the latest
>>> change,
>>>>>>>> there was only one method and having a default was not necessary.
>>>>>>>> However, forcing people to implement both methods might not be
>>> the
>>>>> best
>>>>>>>> user experience: for example, if there is no global thread, one
>>>>> should
>>>>>>>> not need to implement the global handler method (and the other
>>> way
>>>>>>> around).
>>>>>>>> Thus, it might be good to add default for both methods. If we add
>>>>>>>> defaults, we should explain the default behavior to the KIP.
>>>>>>>>
>>>>>>>> -Matthias
>>>>>>>>
>>>>>>>> On 10/12/20 2:32 PM, Walker Carlson wrote:
>>>>>>>>> Hello all,
>>>>>>>>>
>>>>>>>>> I just wanted to let you know that I had to make 2 minor
>>> updates
>>>>> to the
>>>>>>>> KIP.
>>>>>>>>> 1) I changed the behavior of the shutdown client to not leave
>>> the
>>>>>>> client
>>>>>>>> in
>>>>>>>>> Error but instead close directly because this aligns better
>>> with
>>>>> our
>>>>>>>>> state machine.
>>>>>>>>>
>>>>>>>>> 2) I added a separate call back for the global thread as it
>>> does
>>>>> not
>>>>>>> have
>>>>>>>>> all the options as a streamThread does. i.e. replace. The
>>> default
>>>>> will
>>>>>>> be
>>>>>>>>> to close the client. that will also be the only option as that
>>> is
>>>>> the
>>>>>>>>> current behavior for the global thread.
>>>>>>>>>
>>>>>>>>> you can find the diff here:
>>>>>>>>>
>>> https://cwiki.apache.org/confluence/pages/diffpagesbyversion.action?pageId=158876566&originalVersion=21&revisedVersion=23
>>>>>>>>> If you have any problems with these changes let me know and we
>>> can
>>>>>>>> discuss
>>>>>>>>> them further
>>>>>>>>>
>>>>>>>>> Thank you,
>>>>>>>>> Walker
>>>>>>>>>
>>>>>>>>> On Wed, Sep 30, 2020 at 7:33 AM Walker Carlson <
>>>>> wcarlson@confluent.io>
>>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>>> Bruno Cadonna <br...@confluent.io>
>>>>>>>>>> 4:51 AM (2 hours ago)
>>>>>>>>>> to dev
>>>>>>>>>> Thank you all for voting!
>>>>>>>>>>
>>>>>>>>>> This KIP is accepted with +3 binding (Guozhang, Bill,
>>> Matthias)
>>>>> and +2
>>>>>>>>>> non-binding (Bruno, Leah).
>>>>>>>>>>
>>>>>>>>>> Matthias, we will take care of  the global threads, and for
>>> the
>>>>>>>>>> replacement that will depend on Kip-663.
>>>>>>>>>>
>>>>>>>>>> Best,
>>>>>>>>>>
>>>>>>>>>> On Wed, Sep 30, 2020 at 4:59 AM Bruno Cadonna <
>>> bruno@confluent.io
>>>>>>>> wrote:
>>>>>>>>>>> Thanks a lot Walker!
>>>>>>>>>>>
>>>>>>>>>>> +1 (non-binding)
>>>>>>>>>>>
>>>>>>>>>>> Best,
>>>>>>>>>>> Bruno
>>>>>>>>>>>
>>>>>>>>>>> On 30.09.20 03:10, Matthias J. Sax wrote:
>>>>>>>>>>>> Thanks Walker. The proposed API changes LGTM.
>>>>>>>>>>>>
>>>>>>>>>>>> +1 (binding)
>>>>>>>>>>>>
>>>>>>>>>>>> One minor nit: you should also mention the global-thread
>>> that
>>>>> also
>>>>>>>> needs
>>>>>>>>>>>> to be shutdown if requested by the user.
>>>>>>>>>>>>
>>>>>>>>>>>> Minor side question: should we actually terminate a
>>> thread and
>>>>>>> create
>>>>>>>> a
>>>>>>>>>>>> new one, or instead revive the existing thread (reusing
>>> its
>>>>> existing
>>>>>>>>>>> ID)?
>>>>>>>>>>>> -Matthias
>>>>>>>>>>>>
>>>>>>>>>>>> On 9/29/20 2:39 PM, Bill Bejeck wrote:
>>>>>>>>>>>>> Thanks for the KIP Walker.
>>>>>>>>>>>>>
>>>>>>>>>>>>> +1 (binding)
>>>>>>>>>>>>>
>>>>>>>>>>>>> -Bill
>>>>>>>>>>>>>
>>>>>>>>>>>>> On Tue, Sep 29, 2020 at 4:59 PM Guozhang Wang <
>>>>> wangguoz@gmail.com>
>>>>>>>>>>> wrote:
>>>>>>>>>>>>>> +1 again on the KIP.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On Tue, Sep 29, 2020 at 1:51 PM Leah Thomas <
>>>>> lthomas@confluent.io
>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>> Hey Walker,
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Thanks for the KIP! I'm +1, non-binding.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Cheers,
>>>>>>>>>>>>>>> Leah
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> On Tue, Sep 29, 2020 at 1:56 PM Walker Carlson <
>>>>>>>>>>> wcarlson@confluent.io>
>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Hello all,
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> I made some changes to the KIP the descriptions
>>> are on the
>>>>>>>>>>> discussion
>>>>>>>>>>>>>>>> thread. If you have already voted I would ask
>>> you to confirm
>>>>>>> your
>>>>>>>>>>> vote.
>>>>>>>>>>>>>>>> Otherwise please vote so we can get this feature
>>> in.
>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>> Walker
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> On Thu, Sep 24, 2020 at 4:36 PM John Roesler <
>>>>>>> vvcephei@apache.org
>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>>> Thanks for the KIP, Walker!
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> I’m +1 (binding)
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> -John
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> On Mon, Sep 21, 2020, at 17:04, Guozhang Wang
>>> wrote:
>>>>>>>>>>>>>>>>>> Thanks for finalizing the KIP. +1 (binding)
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Guozhang
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> On Mon, Sep 21, 2020 at 1:38 PM Walker
>>> Carlson <
>>>>>>>>>>>>>>> wcarlson@confluent.io>
>>>>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> Hello all,
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> I would like to start a thread to vote for
>>> KIP-671 to
>>>>> add a
>>>>>>>>>>>>>> method
>>>>>>>>>>>>>>> to
>>>>>>>>>>>>>>>>> close
>>>>>>>>>>>>>>>>>>> all clients in a kafka streams application.
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> KIP:
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>
>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-671%3A+Shutdown+Streams+Application+when+appropriate+exception+is+thrown
>>>>>>>>>>>>>>>>>>> Discussion thread: *here
>>>>>>>>>>>>>>>>>>> <
>>>>>>>>>>>>>>>>>>>
>>> https://mail-archives.apache.org/mod_mbox/kafka-dev/202009.mbox/%3CCAC55fuh3HAGCxz-PzxTJraczy6T-os2oiCV328PBeuJQSVYASg%40mail.gmail.com%3E
>>>>>>>>>>>>>>>>>>>> *
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>>>>> -Walker
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> --
>>>>>>>>>>>>>>>>>> -- Guozhang
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> --
>>>>>>>>>>>>>> -- Guozhang
>>>>>>>>>>>>>>
>

Re: [VOTE] KIP-671: Add method to Shutdown entire Streams Application

Posted by John Roesler <vv...@apache.org>.

Thanks, Walker.

That change looks good to me.

-John

On Mon, 2020-10-19 at 12:06 -0700, Walker Carlson wrote:
> Hello all,
> 
> Taking into account the feedback about that last change I have removed some
> of the changes and no longer will we have a separate handler for the global
> thread. To make it so we can align the handlers there will also be no
> option to just remove a stream thread.
> 
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-671%3A+Introduce+Kafka+Streams+Specific+Uncaught+Exception+Handler
> 
> If you have any concerns please let me know,
> Walker
> 
> On Wed, Oct 14, 2020 at 12:51 PM John Roesler <vv...@apache.org> wrote:
> 
> > Thanks, Sophie,
> > 
> > That makes sense. Should we add a whole new interface and a
> > separate kind of listener just because global threads don't
> > support restarts _yet_, though?
> > 
> > It seems like that will just widen the API surface area, and
> > in a few more months, there will be no more difference
> > between the two handlers. But people will forever afterward
> > have to register two different handlers to do the same
> > thing.
> > 
> > Two alternatives we could consider:
> > 1. Just don't add the "restart" option until it's possible
> > for all threads. This KIP is already accepted with a
> > "restart" option that we know won't be added until KIP-663
> > is done. Maybe we just wait for KIP-406 as well. But the
> > _rest_ of this KIP can be implemented in the mean time.
> > 2. Just log an error and kill the thread anyway if the
> > handler for the global thread opts to "retry".
> > 
> > In general, it seems like the problem at hand is better
> > solved by allowing/disallowing that one option, versus
> > adding a whole new interface.
> > 
> > Thanks,
> > -John
> > 
> > On Wed, 2020-10-14 at 11:48 -0700, Sophie Blee-Goldman
> > wrote:
> > > I don't think the proposal was to *never* add the "replace" functionality
> > > for
> > > the global thread, but we didn't want to tie up this KIP with anything
> > more.
> > > As I understand it, the goal of Walker's proposal was to set us up for
> > > success if/when we want to add new functionality for the global thread,
> > > without necessarily committing to it at this time.
> > > 
> > > Restarting the global thread will take a bit more work since you need to
> > > pause any further work that relies on global state until it's back up.
> > > That's
> > > starting to sound more in the purview of KIP-406 whose current goal
> > > is to effectively restart the global thread on a specific type of
> > exception
> > > (OffsetOutOfRange). If we want to consider expanding that to allow users
> > > to choose to restart the thread, then KIP-406 seems like the more
> > > appropriate place to engage in that discussion.
> > > 
> > > On Wed, Oct 14, 2020 at 7:13 AM John Roesler <vv...@apache.org>
> > wrote:
> > > > Hello Walker,
> > > > 
> > > > Sorry for the late reply, but I didn’t follow the reasoning for the
> > > > separate handler. You said that the global thread doesn’t have
> > “replace”,
> > > > but as of today, none of the threads have “replace”. Why not add that
> > > > ability when we add it for the other threads?
> > > > 
> > > > The nature of an uncaught exception handler is that there is an
> > exception
> > > > that will kill the thread. In that case, it seems like replacement is a
> > > > desirable option.
> > > > 
> > > > What have I missed?
> > > > 
> > > > Thanks,
> > > > John
> > > > 
> > > > On Tue, Oct 13, 2020, at 15:49, Walker Carlson wrote:
> > > > > Those are good points Sophie and Matthias. I sepificed the defaults
> > in
> > > > the
> > > > > kip and standardized the names fo the handler to make them a bit more
> > > > > readable.
> > > > > 
> > > > > Thanks for the suggestions,
> > > > > Walker
> > > > > 
> > > > > On Tue, Oct 13, 2020 at 12:24 PM Sophie Blee-Goldman <
> > > > sophie@confluent.io>
> > > > > wrote:
> > > > > 
> > > > > > Super nit: can we standardize the method & enum names?
> > > > > > 
> > > > > > Right now we have these enums:
> > > > > > StreamsUncaughtExceptionHandlerResponse
> > > > > > StreamsUncaughtExceptionHandlerResponseGlobalThread
> > > > > > 
> > > > > > and these callbacks:
> > > > > > handleUncaughtException()
> > > > > > handleExceptionInGlobalThread()
> > > > > > 
> > > > > > The method names have different syntax, which is a bit clunky. I
> > don't
> > > > have
> > > > > > any
> > > > > > strong opinions on what grammar they should follow, just that it
> > > > should be
> > > > > > the
> > > > > > same for each. I also think that we should specify "StreamThread"
> > > > somewhere
> > > > > > in the name of the StreadThread-specific callback, now that we
> > have a
> > > > > > second
> > > > > > callback that specifies it's for the GlobalThread. Something like
> > > > > > "*handleStreamThreadException()*" and
> > "*handleGlobalThreadException*"
> > > > > > The enums are ok, although I think we should include "StreamThread"
> > > > > > somewhere
> > > > > > like with the callbacks. And we can probably shorten them a bit.
> > For
> > > > > > example
> > > > > > "*StreamThreadExceptionResponse*" and
> > "*GlobalThreadExceptionResponse*"
> > > > > > 
> > > > > > 
> > > > > > On Tue, Oct 13, 2020 at 11:48 AM Matthias J. Sax <mjsax@apache.org
> > > > wrote:
> > > > > > > Thanks Walker.
> > > > > > > 
> > > > > > > Overall, LGTM. However, I am wondering if we should have default
> > > > > > > implementations for both handler methods? Before the latest
> > change,
> > > > > > > there was only one method and having a default was not necessary.
> > > > > > > However, forcing people to implement both methods might not be
> > the
> > > > best
> > > > > > > user experience: for example, if there is no global thread, one
> > > > should
> > > > > > > not need to implement the global handler method (and the other
> > way
> > > > > > around).
> > > > > > > Thus, it might be good to add default for both methods. If we add
> > > > > > > defaults, we should explain the default behavior to the KIP.
> > > > > > > 
> > > > > > > -Matthias
> > > > > > > 
> > > > > > > On 10/12/20 2:32 PM, Walker Carlson wrote:
> > > > > > > > Hello all,
> > > > > > > > 
> > > > > > > > I just wanted to let you know that I had to make 2 minor
> > updates
> > > > to the
> > > > > > > KIP.
> > > > > > > > 1) I changed the behavior of the shutdown client to not leave
> > the
> > > > > > client
> > > > > > > in
> > > > > > > > Error but instead close directly because this aligns better
> > with
> > > > our
> > > > > > > > state machine.
> > > > > > > > 
> > > > > > > > 2) I added a separate call back for the global thread as it
> > does
> > > > not
> > > > > > have
> > > > > > > > all the options as a streamThread does. i.e. replace. The
> > default
> > > > will
> > > > > > be
> > > > > > > > to close the client. that will also be the only option as that
> > is
> > > > the
> > > > > > > > current behavior for the global thread.
> > > > > > > > 
> > > > > > > > you can find the diff here:
> > > > > > > > 
> > https://cwiki.apache.org/confluence/pages/diffpagesbyversion.action?pageId=158876566&originalVersion=21&revisedVersion=23
> > > > > > > > If you have any problems with these changes let me know and we
> > can
> > > > > > > discuss
> > > > > > > > them further
> > > > > > > > 
> > > > > > > > Thank you,
> > > > > > > > Walker
> > > > > > > > 
> > > > > > > > On Wed, Sep 30, 2020 at 7:33 AM Walker Carlson <
> > > > wcarlson@confluent.io>
> > > > > > > > wrote:
> > > > > > > > 
> > > > > > > > > Bruno Cadonna <br...@confluent.io>
> > > > > > > > > 4:51 AM (2 hours ago)
> > > > > > > > > to dev
> > > > > > > > > Thank you all for voting!
> > > > > > > > > 
> > > > > > > > > This KIP is accepted with +3 binding (Guozhang, Bill,
> > Matthias)
> > > > and +2
> > > > > > > > > non-binding (Bruno, Leah).
> > > > > > > > > 
> > > > > > > > > Matthias, we will take care of  the global threads, and for
> > the
> > > > > > > > > replacement that will depend on Kip-663.
> > > > > > > > > 
> > > > > > > > > Best,
> > > > > > > > > 
> > > > > > > > > On Wed, Sep 30, 2020 at 4:59 AM Bruno Cadonna <
> > bruno@confluent.io
> > > > > > > wrote:
> > > > > > > > > > Thanks a lot Walker!
> > > > > > > > > > 
> > > > > > > > > > +1 (non-binding)
> > > > > > > > > > 
> > > > > > > > > > Best,
> > > > > > > > > > Bruno
> > > > > > > > > > 
> > > > > > > > > > On 30.09.20 03:10, Matthias J. Sax wrote:
> > > > > > > > > > > Thanks Walker. The proposed API changes LGTM.
> > > > > > > > > > > 
> > > > > > > > > > > +1 (binding)
> > > > > > > > > > > 
> > > > > > > > > > > One minor nit: you should also mention the global-thread
> > that
> > > > also
> > > > > > > needs
> > > > > > > > > > > to be shutdown if requested by the user.
> > > > > > > > > > > 
> > > > > > > > > > > Minor side question: should we actually terminate a
> > thread and
> > > > > > create
> > > > > > > a
> > > > > > > > > > > new one, or instead revive the existing thread (reusing
> > its
> > > > existing
> > > > > > > > > > ID)?
> > > > > > > > > > > -Matthias
> > > > > > > > > > > 
> > > > > > > > > > > On 9/29/20 2:39 PM, Bill Bejeck wrote:
> > > > > > > > > > > > Thanks for the KIP Walker.
> > > > > > > > > > > > 
> > > > > > > > > > > > +1 (binding)
> > > > > > > > > > > > 
> > > > > > > > > > > > -Bill
> > > > > > > > > > > > 
> > > > > > > > > > > > On Tue, Sep 29, 2020 at 4:59 PM Guozhang Wang <
> > > > wangguoz@gmail.com>
> > > > > > > > > > wrote:
> > > > > > > > > > > > > +1 again on the KIP.
> > > > > > > > > > > > > 
> > > > > > > > > > > > > On Tue, Sep 29, 2020 at 1:51 PM Leah Thomas <
> > > > lthomas@confluent.io
> > > > > > > > > > wrote:
> > > > > > > > > > > > > > Hey Walker,
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > Thanks for the KIP! I'm +1, non-binding.
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > Cheers,
> > > > > > > > > > > > > > Leah
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > On Tue, Sep 29, 2020 at 1:56 PM Walker Carlson <
> > > > > > > > > > wcarlson@confluent.io>
> > > > > > > > > > > > > > wrote:
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > Hello all,
> > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > I made some changes to the KIP the descriptions
> > are on the
> > > > > > > > > > discussion
> > > > > > > > > > > > > > > thread. If you have already voted I would ask
> > you to confirm
> > > > > > your
> > > > > > > > > > vote.
> > > > > > > > > > > > > > > Otherwise please vote so we can get this feature
> > in.
> > > > > > > > > > > > > > > Thanks,
> > > > > > > > > > > > > > > Walker
> > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > On Thu, Sep 24, 2020 at 4:36 PM John Roesler <
> > > > > > vvcephei@apache.org
> > > > > > > > > > > > > > wrote:
> > > > > > > > > > > > > > > > Thanks for the KIP, Walker!
> > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > I’m +1 (binding)
> > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > -John
> > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > On Mon, Sep 21, 2020, at 17:04, Guozhang Wang
> > wrote:
> > > > > > > > > > > > > > > > > Thanks for finalizing the KIP. +1 (binding)
> > > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > > Guozhang
> > > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > > On Mon, Sep 21, 2020 at 1:38 PM Walker
> > Carlson <
> > > > > > > > > > > > > > wcarlson@confluent.io>
> > > > > > > > > > > > > > > > > wrote:
> > > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > > > Hello all,
> > > > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > > > I would like to start a thread to vote for
> > KIP-671 to
> > > > add a
> > > > > > > > > > > > > method
> > > > > > > > > > > > > > to
> > > > > > > > > > > > > > > > close
> > > > > > > > > > > > > > > > > > all clients in a kafka streams application.
> > > > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > > > KIP:
> > > > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > > > 
> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-671%3A+Shutdown+Streams+Application+when+appropriate+exception+is+thrown
> > > > > > > > > > > > > > > > > > Discussion thread: *here
> > > > > > > > > > > > > > > > > > <
> > > > > > > > > > > > > > > > > > 
> > https://mail-archives.apache.org/mod_mbox/kafka-dev/202009.mbox/%3CCAC55fuh3HAGCxz-PzxTJraczy6T-os2oiCV328PBeuJQSVYASg%40mail.gmail.com%3E
> > > > > > > > > > > > > > > > > > > *
> > > > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > > > Thanks,
> > > > > > > > > > > > > > > > > > -Walker
> > > > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > > --
> > > > > > > > > > > > > > > > > -- Guozhang
> > > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > 
> > > > > > > > > > > > > --
> > > > > > > > > > > > > -- Guozhang
> > > > > > > > > > > > >

Re: [VOTE] KIP-671: Add method to Shutdown entire Streams Application

Posted by Walker Carlson <wc...@confluent.io>.

Hello all,

Taking into account the feedback about that last change I have removed some
of the changes and no longer will we have a separate handler for the global
thread. To make it so we can align the handlers there will also be no
option to just remove a stream thread.

https://cwiki.apache.org/confluence/display/KAFKA/KIP-671%3A+Introduce+Kafka+Streams+Specific+Uncaught+Exception+Handler

If you have any concerns please let me know,
Walker

On Wed, Oct 14, 2020 at 12:51 PM John Roesler <vv...@apache.org> wrote:

> Thanks, Sophie,
>
> That makes sense. Should we add a whole new interface and a
> separate kind of listener just because global threads don't
> support restarts _yet_, though?
>
> It seems like that will just widen the API surface area, and
> in a few more months, there will be no more difference
> between the two handlers. But people will forever afterward
> have to register two different handlers to do the same
> thing.
>
> Two alternatives we could consider:
> 1. Just don't add the "restart" option until it's possible
> for all threads. This KIP is already accepted with a
> "restart" option that we know won't be added until KIP-663
> is done. Maybe we just wait for KIP-406 as well. But the
> _rest_ of this KIP can be implemented in the mean time.
> 2. Just log an error and kill the thread anyway if the
> handler for the global thread opts to "retry".
>
> In general, it seems like the problem at hand is better
> solved by allowing/disallowing that one option, versus
> adding a whole new interface.
>
> Thanks,
> -John
>
> On Wed, 2020-10-14 at 11:48 -0700, Sophie Blee-Goldman
> wrote:
> > I don't think the proposal was to *never* add the "replace" functionality
> > for
> > the global thread, but we didn't want to tie up this KIP with anything
> more.
> > As I understand it, the goal of Walker's proposal was to set us up for
> > success if/when we want to add new functionality for the global thread,
> > without necessarily committing to it at this time.
> >
> > Restarting the global thread will take a bit more work since you need to
> > pause any further work that relies on global state until it's back up.
> > That's
> > starting to sound more in the purview of KIP-406 whose current goal
> > is to effectively restart the global thread on a specific type of
> exception
> > (OffsetOutOfRange). If we want to consider expanding that to allow users
> > to choose to restart the thread, then KIP-406 seems like the more
> > appropriate place to engage in that discussion.
> >
> > On Wed, Oct 14, 2020 at 7:13 AM John Roesler <vv...@apache.org>
> wrote:
> >
> > > Hello Walker,
> > >
> > > Sorry for the late reply, but I didn’t follow the reasoning for the
> > > separate handler. You said that the global thread doesn’t have
> “replace”,
> > > but as of today, none of the threads have “replace”. Why not add that
> > > ability when we add it for the other threads?
> > >
> > > The nature of an uncaught exception handler is that there is an
> exception
> > > that will kill the thread. In that case, it seems like replacement is a
> > > desirable option.
> > >
> > > What have I missed?
> > >
> > > Thanks,
> > > John
> > >
> > > On Tue, Oct 13, 2020, at 15:49, Walker Carlson wrote:
> > > > Those are good points Sophie and Matthias. I sepificed the defaults
> in
> > > the
> > > > kip and standardized the names fo the handler to make them a bit more
> > > > readable.
> > > >
> > > > Thanks for the suggestions,
> > > > Walker
> > > >
> > > > On Tue, Oct 13, 2020 at 12:24 PM Sophie Blee-Goldman <
> > > sophie@confluent.io>
> > > > wrote:
> > > >
> > > > > Super nit: can we standardize the method & enum names?
> > > > >
> > > > > Right now we have these enums:
> > > > > StreamsUncaughtExceptionHandlerResponse
> > > > > StreamsUncaughtExceptionHandlerResponseGlobalThread
> > > > >
> > > > > and these callbacks:
> > > > > handleUncaughtException()
> > > > > handleExceptionInGlobalThread()
> > > > >
> > > > > The method names have different syntax, which is a bit clunky. I
> don't
> > > have
> > > > > any
> > > > > strong opinions on what grammar they should follow, just that it
> > > should be
> > > > > the
> > > > > same for each. I also think that we should specify "StreamThread"
> > > somewhere
> > > > > in the name of the StreadThread-specific callback, now that we
> have a
> > > > > second
> > > > > callback that specifies it's for the GlobalThread. Something like
> > > > > "*handleStreamThreadException()*" and
> "*handleGlobalThreadException*"
> > > > >
> > > > > The enums are ok, although I think we should include "StreamThread"
> > > > > somewhere
> > > > > like with the callbacks. And we can probably shorten them a bit.
> For
> > > > > example
> > > > > "*StreamThreadExceptionResponse*" and
> "*GlobalThreadExceptionResponse*"
> > > > >
> > > > >
> > > > >
> > > > > On Tue, Oct 13, 2020 at 11:48 AM Matthias J. Sax <mjsax@apache.org
> >
> > > wrote:
> > > > > > Thanks Walker.
> > > > > >
> > > > > > Overall, LGTM. However, I am wondering if we should have default
> > > > > > implementations for both handler methods? Before the latest
> change,
> > > > > > there was only one method and having a default was not necessary.
> > > > > > However, forcing people to implement both methods might not be
> the
> > > best
> > > > > > user experience: for example, if there is no global thread, one
> > > should
> > > > > > not need to implement the global handler method (and the other
> way
> > > > > around).
> > > > > > Thus, it might be good to add default for both methods. If we add
> > > > > > defaults, we should explain the default behavior to the KIP.
> > > > > >
> > > > > > -Matthias
> > > > > >
> > > > > > On 10/12/20 2:32 PM, Walker Carlson wrote:
> > > > > > > Hello all,
> > > > > > >
> > > > > > > I just wanted to let you know that I had to make 2 minor
> updates
> > > to the
> > > > > > KIP.
> > > > > > > 1) I changed the behavior of the shutdown client to not leave
> the
> > > > > client
> > > > > > in
> > > > > > > Error but instead close directly because this aligns better
> with
> > > our
> > > > > > > state machine.
> > > > > > >
> > > > > > > 2) I added a separate call back for the global thread as it
> does
> > > not
> > > > > have
> > > > > > > all the options as a streamThread does. i.e. replace. The
> default
> > > will
> > > > > be
> > > > > > > to close the client. that will also be the only option as that
> is
> > > the
> > > > > > > current behavior for the global thread.
> > > > > > >
> > > > > > > you can find the diff here:
> > > > > > >
> > >
> https://cwiki.apache.org/confluence/pages/diffpagesbyversion.action?pageId=158876566&originalVersion=21&revisedVersion=23
> > > > > > > If you have any problems with these changes let me know and we
> can
> > > > > > discuss
> > > > > > > them further
> > > > > > >
> > > > > > > Thank you,
> > > > > > > Walker
> > > > > > >
> > > > > > > On Wed, Sep 30, 2020 at 7:33 AM Walker Carlson <
> > > wcarlson@confluent.io>
> > > > > > > wrote:
> > > > > > >
> > > > > > > > Bruno Cadonna <br...@confluent.io>
> > > > > > > > 4:51 AM (2 hours ago)
> > > > > > > > to dev
> > > > > > > > Thank you all for voting!
> > > > > > > >
> > > > > > > > This KIP is accepted with +3 binding (Guozhang, Bill,
> Matthias)
> > > and +2
> > > > > > > > non-binding (Bruno, Leah).
> > > > > > > >
> > > > > > > > Matthias, we will take care of  the global threads, and for
> the
> > > > > > > > replacement that will depend on Kip-663.
> > > > > > > >
> > > > > > > > Best,
> > > > > > > >
> > > > > > > > On Wed, Sep 30, 2020 at 4:59 AM Bruno Cadonna <
> bruno@confluent.io
> > > > > > wrote:
> > > > > > > > > Thanks a lot Walker!
> > > > > > > > >
> > > > > > > > > +1 (non-binding)
> > > > > > > > >
> > > > > > > > > Best,
> > > > > > > > > Bruno
> > > > > > > > >
> > > > > > > > > On 30.09.20 03:10, Matthias J. Sax wrote:
> > > > > > > > > > Thanks Walker. The proposed API changes LGTM.
> > > > > > > > > >
> > > > > > > > > > +1 (binding)
> > > > > > > > > >
> > > > > > > > > > One minor nit: you should also mention the global-thread
> that
> > > also
> > > > > > needs
> > > > > > > > > > to be shutdown if requested by the user.
> > > > > > > > > >
> > > > > > > > > > Minor side question: should we actually terminate a
> thread and
> > > > > create
> > > > > > a
> > > > > > > > > > new one, or instead revive the existing thread (reusing
> its
> > > existing
> > > > > > > > > ID)?
> > > > > > > > > >
> > > > > > > > > > -Matthias
> > > > > > > > > >
> > > > > > > > > > On 9/29/20 2:39 PM, Bill Bejeck wrote:
> > > > > > > > > > > Thanks for the KIP Walker.
> > > > > > > > > > >
> > > > > > > > > > > +1 (binding)
> > > > > > > > > > >
> > > > > > > > > > > -Bill
> > > > > > > > > > >
> > > > > > > > > > > On Tue, Sep 29, 2020 at 4:59 PM Guozhang Wang <
> > > wangguoz@gmail.com>
> > > > > > > > > wrote:
> > > > > > > > > > > > +1 again on the KIP.
> > > > > > > > > > > >
> > > > > > > > > > > > On Tue, Sep 29, 2020 at 1:51 PM Leah Thomas <
> > > lthomas@confluent.io
> > > > > > > > > wrote:
> > > > > > > > > > > > > Hey Walker,
> > > > > > > > > > > > >
> > > > > > > > > > > > > Thanks for the KIP! I'm +1, non-binding.
> > > > > > > > > > > > >
> > > > > > > > > > > > > Cheers,
> > > > > > > > > > > > > Leah
> > > > > > > > > > > > >
> > > > > > > > > > > > > On Tue, Sep 29, 2020 at 1:56 PM Walker Carlson <
> > > > > > > > > wcarlson@confluent.io>
> > > > > > > > > > > > > wrote:
> > > > > > > > > > > > >
> > > > > > > > > > > > > > Hello all,
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > I made some changes to the KIP the descriptions
> are on the
> > > > > > > > > discussion
> > > > > > > > > > > > > > thread. If you have already voted I would ask
> you to confirm
> > > > > your
> > > > > > > > > vote.
> > > > > > > > > > > > > > Otherwise please vote so we can get this feature
> in.
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > Thanks,
> > > > > > > > > > > > > > Walker
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > On Thu, Sep 24, 2020 at 4:36 PM John Roesler <
> > > > > vvcephei@apache.org
> > > > > > > > > > > > > wrote:
> > > > > > > > > > > > > > > Thanks for the KIP, Walker!
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > I’m +1 (binding)
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > -John
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > On Mon, Sep 21, 2020, at 17:04, Guozhang Wang
> wrote:
> > > > > > > > > > > > > > > > Thanks for finalizing the KIP. +1 (binding)
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > Guozhang
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > On Mon, Sep 21, 2020 at 1:38 PM Walker
> Carlson <
> > > > > > > > > > > > > wcarlson@confluent.io>
> > > > > > > > > > > > > > > > wrote:
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > Hello all,
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > I would like to start a thread to vote for
> KIP-671 to
> > > add a
> > > > > > > > > > > > method
> > > > > > > > > > > > > to
> > > > > > > > > > > > > > > close
> > > > > > > > > > > > > > > > > all clients in a kafka streams application.
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > KIP:
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > >
> > >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-671%3A+Shutdown+Streams+Application+when+appropriate+exception+is+thrown
> > > > > > > > > > > > > > > > > Discussion thread: *here
> > > > > > > > > > > > > > > > > <
> > > > > > > > > > > > > > > > >
> > >
> https://mail-archives.apache.org/mod_mbox/kafka-dev/202009.mbox/%3CCAC55fuh3HAGCxz-PzxTJraczy6T-os2oiCV328PBeuJQSVYASg%40mail.gmail.com%3E
> > > > > > > > > > > > > > > > > > *
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > Thanks,
> > > > > > > > > > > > > > > > > -Walker
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > --
> > > > > > > > > > > > > > > > -- Guozhang
> > > > > > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > > > --
> > > > > > > > > > > > -- Guozhang
> > > > > > > > > > > >
>
>

Re: [VOTE] KIP-671: Add method to Shutdown entire Streams Application

Posted by John Roesler <vv...@apache.org>.

Thanks, Sophie,

That makes sense. Should we add a whole new interface and a
separate kind of listener just because global threads don't
support restarts _yet_, though?

It seems like that will just widen the API surface area, and
in a few more months, there will be no more difference
between the two handlers. But people will forever afterward
have to register two different handlers to do the same
thing.

Two alternatives we could consider:
1. Just don't add the "restart" option until it's possible
for all threads. This KIP is already accepted with a
"restart" option that we know won't be added until KIP-663
is done. Maybe we just wait for KIP-406 as well. But the
_rest_ of this KIP can be implemented in the mean time.
2. Just log an error and kill the thread anyway if the
handler for the global thread opts to "retry".

In general, it seems like the problem at hand is better
solved by allowing/disallowing that one option, versus
adding a whole new interface.

Thanks,
-John

On Wed, 2020-10-14 at 11:48 -0700, Sophie Blee-Goldman
wrote:
> I don't think the proposal was to *never* add the "replace" functionality
> for
> the global thread, but we didn't want to tie up this KIP with anything more.
> As I understand it, the goal of Walker's proposal was to set us up for
> success if/when we want to add new functionality for the global thread,
> without necessarily committing to it at this time.
> 
> Restarting the global thread will take a bit more work since you need to
> pause any further work that relies on global state until it's back up.
> That's
> starting to sound more in the purview of KIP-406 whose current goal
> is to effectively restart the global thread on a specific type of exception
> (OffsetOutOfRange). If we want to consider expanding that to allow users
> to choose to restart the thread, then KIP-406 seems like the more
> appropriate place to engage in that discussion.
> 
> On Wed, Oct 14, 2020 at 7:13 AM John Roesler <vv...@apache.org> wrote:
> 
> > Hello Walker,
> > 
> > Sorry for the late reply, but I didn’t follow the reasoning for the
> > separate handler. You said that the global thread doesn’t have “replace”,
> > but as of today, none of the threads have “replace”. Why not add that
> > ability when we add it for the other threads?
> > 
> > The nature of an uncaught exception handler is that there is an exception
> > that will kill the thread. In that case, it seems like replacement is a
> > desirable option.
> > 
> > What have I missed?
> > 
> > Thanks,
> > John
> > 
> > On Tue, Oct 13, 2020, at 15:49, Walker Carlson wrote:
> > > Those are good points Sophie and Matthias. I sepificed the defaults in
> > the
> > > kip and standardized the names fo the handler to make them a bit more
> > > readable.
> > > 
> > > Thanks for the suggestions,
> > > Walker
> > > 
> > > On Tue, Oct 13, 2020 at 12:24 PM Sophie Blee-Goldman <
> > sophie@confluent.io>
> > > wrote:
> > > 
> > > > Super nit: can we standardize the method & enum names?
> > > > 
> > > > Right now we have these enums:
> > > > StreamsUncaughtExceptionHandlerResponse
> > > > StreamsUncaughtExceptionHandlerResponseGlobalThread
> > > > 
> > > > and these callbacks:
> > > > handleUncaughtException()
> > > > handleExceptionInGlobalThread()
> > > > 
> > > > The method names have different syntax, which is a bit clunky. I don't
> > have
> > > > any
> > > > strong opinions on what grammar they should follow, just that it
> > should be
> > > > the
> > > > same for each. I also think that we should specify "StreamThread"
> > somewhere
> > > > in the name of the StreadThread-specific callback, now that we have a
> > > > second
> > > > callback that specifies it's for the GlobalThread. Something like
> > > > "*handleStreamThreadException()*" and "*handleGlobalThreadException*"
> > > > 
> > > > The enums are ok, although I think we should include "StreamThread"
> > > > somewhere
> > > > like with the callbacks. And we can probably shorten them a bit. For
> > > > example
> > > > "*StreamThreadExceptionResponse*" and "*GlobalThreadExceptionResponse*"
> > > > 
> > > > 
> > > > 
> > > > On Tue, Oct 13, 2020 at 11:48 AM Matthias J. Sax <mj...@apache.org>
> > wrote:
> > > > > Thanks Walker.
> > > > > 
> > > > > Overall, LGTM. However, I am wondering if we should have default
> > > > > implementations for both handler methods? Before the latest change,
> > > > > there was only one method and having a default was not necessary.
> > > > > However, forcing people to implement both methods might not be the
> > best
> > > > > user experience: for example, if there is no global thread, one
> > should
> > > > > not need to implement the global handler method (and the other way
> > > > around).
> > > > > Thus, it might be good to add default for both methods. If we add
> > > > > defaults, we should explain the default behavior to the KIP.
> > > > > 
> > > > > -Matthias
> > > > > 
> > > > > On 10/12/20 2:32 PM, Walker Carlson wrote:
> > > > > > Hello all,
> > > > > > 
> > > > > > I just wanted to let you know that I had to make 2 minor updates
> > to the
> > > > > KIP.
> > > > > > 1) I changed the behavior of the shutdown client to not leave the
> > > > client
> > > > > in
> > > > > > Error but instead close directly because this aligns better with
> > our
> > > > > > state machine.
> > > > > > 
> > > > > > 2) I added a separate call back for the global thread as it does
> > not
> > > > have
> > > > > > all the options as a streamThread does. i.e. replace. The default
> > will
> > > > be
> > > > > > to close the client. that will also be the only option as that is
> > the
> > > > > > current behavior for the global thread.
> > > > > > 
> > > > > > you can find the diff here:
> > > > > > 
> > https://cwiki.apache.org/confluence/pages/diffpagesbyversion.action?pageId=158876566&originalVersion=21&revisedVersion=23
> > > > > > If you have any problems with these changes let me know and we can
> > > > > discuss
> > > > > > them further
> > > > > > 
> > > > > > Thank you,
> > > > > > Walker
> > > > > > 
> > > > > > On Wed, Sep 30, 2020 at 7:33 AM Walker Carlson <
> > wcarlson@confluent.io>
> > > > > > wrote:
> > > > > > 
> > > > > > > Bruno Cadonna <br...@confluent.io>
> > > > > > > 4:51 AM (2 hours ago)
> > > > > > > to dev
> > > > > > > Thank you all for voting!
> > > > > > > 
> > > > > > > This KIP is accepted with +3 binding (Guozhang, Bill, Matthias)
> > and +2
> > > > > > > non-binding (Bruno, Leah).
> > > > > > > 
> > > > > > > Matthias, we will take care of  the global threads, and for the
> > > > > > > replacement that will depend on Kip-663.
> > > > > > > 
> > > > > > > Best,
> > > > > > > 
> > > > > > > On Wed, Sep 30, 2020 at 4:59 AM Bruno Cadonna <bruno@confluent.io
> > > > > wrote:
> > > > > > > > Thanks a lot Walker!
> > > > > > > > 
> > > > > > > > +1 (non-binding)
> > > > > > > > 
> > > > > > > > Best,
> > > > > > > > Bruno
> > > > > > > > 
> > > > > > > > On 30.09.20 03:10, Matthias J. Sax wrote:
> > > > > > > > > Thanks Walker. The proposed API changes LGTM.
> > > > > > > > > 
> > > > > > > > > +1 (binding)
> > > > > > > > > 
> > > > > > > > > One minor nit: you should also mention the global-thread that
> > also
> > > > > needs
> > > > > > > > > to be shutdown if requested by the user.
> > > > > > > > > 
> > > > > > > > > Minor side question: should we actually terminate a thread and
> > > > create
> > > > > a
> > > > > > > > > new one, or instead revive the existing thread (reusing its
> > existing
> > > > > > > > ID)?
> > > > > > > > > 
> > > > > > > > > -Matthias
> > > > > > > > > 
> > > > > > > > > On 9/29/20 2:39 PM, Bill Bejeck wrote:
> > > > > > > > > > Thanks for the KIP Walker.
> > > > > > > > > > 
> > > > > > > > > > +1 (binding)
> > > > > > > > > > 
> > > > > > > > > > -Bill
> > > > > > > > > > 
> > > > > > > > > > On Tue, Sep 29, 2020 at 4:59 PM Guozhang Wang <
> > wangguoz@gmail.com>
> > > > > > > > wrote:
> > > > > > > > > > > +1 again on the KIP.
> > > > > > > > > > > 
> > > > > > > > > > > On Tue, Sep 29, 2020 at 1:51 PM Leah Thomas <
> > lthomas@confluent.io
> > > > > > > > wrote:
> > > > > > > > > > > > Hey Walker,
> > > > > > > > > > > > 
> > > > > > > > > > > > Thanks for the KIP! I'm +1, non-binding.
> > > > > > > > > > > > 
> > > > > > > > > > > > Cheers,
> > > > > > > > > > > > Leah
> > > > > > > > > > > > 
> > > > > > > > > > > > On Tue, Sep 29, 2020 at 1:56 PM Walker Carlson <
> > > > > > > > wcarlson@confluent.io>
> > > > > > > > > > > > wrote:
> > > > > > > > > > > > 
> > > > > > > > > > > > > Hello all,
> > > > > > > > > > > > > 
> > > > > > > > > > > > > I made some changes to the KIP the descriptions are on the
> > > > > > > > discussion
> > > > > > > > > > > > > thread. If you have already voted I would ask you to confirm
> > > > your
> > > > > > > > vote.
> > > > > > > > > > > > > Otherwise please vote so we can get this feature in.
> > > > > > > > > > > > > 
> > > > > > > > > > > > > Thanks,
> > > > > > > > > > > > > Walker
> > > > > > > > > > > > > 
> > > > > > > > > > > > > On Thu, Sep 24, 2020 at 4:36 PM John Roesler <
> > > > vvcephei@apache.org
> > > > > > > > > > > > wrote:
> > > > > > > > > > > > > > Thanks for the KIP, Walker!
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > I’m +1 (binding)
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > -John
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > On Mon, Sep 21, 2020, at 17:04, Guozhang Wang wrote:
> > > > > > > > > > > > > > > Thanks for finalizing the KIP. +1 (binding)
> > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > Guozhang
> > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > On Mon, Sep 21, 2020 at 1:38 PM Walker Carlson <
> > > > > > > > > > > > wcarlson@confluent.io>
> > > > > > > > > > > > > > > wrote:
> > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > Hello all,
> > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > I would like to start a thread to vote for KIP-671 to
> > add a
> > > > > > > > > > > method
> > > > > > > > > > > > to
> > > > > > > > > > > > > > close
> > > > > > > > > > > > > > > > all clients in a kafka streams application.
> > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > KIP:
> > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > 
> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-671%3A+Shutdown+Streams+Application+when+appropriate+exception+is+thrown
> > > > > > > > > > > > > > > > Discussion thread: *here
> > > > > > > > > > > > > > > > <
> > > > > > > > > > > > > > > > 
> > https://mail-archives.apache.org/mod_mbox/kafka-dev/202009.mbox/%3CCAC55fuh3HAGCxz-PzxTJraczy6T-os2oiCV328PBeuJQSVYASg%40mail.gmail.com%3E
> > > > > > > > > > > > > > > > > *
> > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > Thanks,
> > > > > > > > > > > > > > > > -Walker
> > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > --
> > > > > > > > > > > > > > > -- Guozhang
> > > > > > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > --
> > > > > > > > > > > -- Guozhang
> > > > > > > > > > >

Re: [VOTE] KIP-671: Add method to Shutdown entire Streams Application

Posted by Sophie Blee-Goldman <so...@confluent.io>.

I don't think the proposal was to *never* add the "replace" functionality
for
the global thread, but we didn't want to tie up this KIP with anything more.
As I understand it, the goal of Walker's proposal was to set us up for
success if/when we want to add new functionality for the global thread,
without necessarily committing to it at this time.

Restarting the global thread will take a bit more work since you need to
pause any further work that relies on global state until it's back up.
That's
starting to sound more in the purview of KIP-406 whose current goal
is to effectively restart the global thread on a specific type of exception
(OffsetOutOfRange). If we want to consider expanding that to allow users
to choose to restart the thread, then KIP-406 seems like the more
appropriate place to engage in that discussion.

On Wed, Oct 14, 2020 at 7:13 AM John Roesler <vv...@apache.org> wrote:

> Hello Walker,
>
> Sorry for the late reply, but I didn’t follow the reasoning for the
> separate handler. You said that the global thread doesn’t have “replace”,
> but as of today, none of the threads have “replace”. Why not add that
> ability when we add it for the other threads?
>
> The nature of an uncaught exception handler is that there is an exception
> that will kill the thread. In that case, it seems like replacement is a
> desirable option.
>
> What have I missed?
>
> Thanks,
> John
>
> On Tue, Oct 13, 2020, at 15:49, Walker Carlson wrote:
> > Those are good points Sophie and Matthias. I sepificed the defaults in
> the
> > kip and standardized the names fo the handler to make them a bit more
> > readable.
> >
> > Thanks for the suggestions,
> > Walker
> >
> > On Tue, Oct 13, 2020 at 12:24 PM Sophie Blee-Goldman <
> sophie@confluent.io>
> > wrote:
> >
> > > Super nit: can we standardize the method & enum names?
> > >
> > > Right now we have these enums:
> > > StreamsUncaughtExceptionHandlerResponse
> > > StreamsUncaughtExceptionHandlerResponseGlobalThread
> > >
> > > and these callbacks:
> > > handleUncaughtException()
> > > handleExceptionInGlobalThread()
> > >
> > > The method names have different syntax, which is a bit clunky. I don't
> have
> > > any
> > > strong opinions on what grammar they should follow, just that it
> should be
> > > the
> > > same for each. I also think that we should specify "StreamThread"
> somewhere
> > > in the name of the StreadThread-specific callback, now that we have a
> > > second
> > > callback that specifies it's for the GlobalThread. Something like
> > > "*handleStreamThreadException()*" and "*handleGlobalThreadException*"
> > >
> > > The enums are ok, although I think we should include "StreamThread"
> > > somewhere
> > > like with the callbacks. And we can probably shorten them a bit. For
> > > example
> > > "*StreamThreadExceptionResponse*" and "*GlobalThreadExceptionResponse*"
> > >
> > >
> > >
> > > On Tue, Oct 13, 2020 at 11:48 AM Matthias J. Sax <mj...@apache.org>
> wrote:
> > >
> > > > Thanks Walker.
> > > >
> > > > Overall, LGTM. However, I am wondering if we should have default
> > > > implementations for both handler methods? Before the latest change,
> > > > there was only one method and having a default was not necessary.
> > > > However, forcing people to implement both methods might not be the
> best
> > > > user experience: for example, if there is no global thread, one
> should
> > > > not need to implement the global handler method (and the other way
> > > around).
> > > >
> > > > Thus, it might be good to add default for both methods. If we add
> > > > defaults, we should explain the default behavior to the KIP.
> > > >
> > > > -Matthias
> > > >
> > > > On 10/12/20 2:32 PM, Walker Carlson wrote:
> > > > > Hello all,
> > > > >
> > > > > I just wanted to let you know that I had to make 2 minor updates
> to the
> > > > KIP.
> > > > >
> > > > > 1) I changed the behavior of the shutdown client to not leave the
> > > client
> > > > in
> > > > > Error but instead close directly because this aligns better with
> our
> > > > > state machine.
> > > > >
> > > > > 2) I added a separate call back for the global thread as it does
> not
> > > have
> > > > > all the options as a streamThread does. i.e. replace. The default
> will
> > > be
> > > > > to close the client. that will also be the only option as that is
> the
> > > > > current behavior for the global thread.
> > > > >
> > > > > you can find the diff here:
> > > > >
> > > >
> > >
> https://cwiki.apache.org/confluence/pages/diffpagesbyversion.action?pageId=158876566&originalVersion=21&revisedVersion=23
> > > > >
> > > > > If you have any problems with these changes let me know and we can
> > > > discuss
> > > > > them further
> > > > >
> > > > > Thank you,
> > > > > Walker
> > > > >
> > > > > On Wed, Sep 30, 2020 at 7:33 AM Walker Carlson <
> wcarlson@confluent.io>
> > > > > wrote:
> > > > >
> > > > >>
> > > > >> Bruno Cadonna <br...@confluent.io>
> > > > >> 4:51 AM (2 hours ago)
> > > > >> to dev
> > > > >> Thank you all for voting!
> > > > >>
> > > > >> This KIP is accepted with +3 binding (Guozhang, Bill, Matthias)
> and +2
> > > > >> non-binding (Bruno, Leah).
> > > > >>
> > > > >> Matthias, we will take care of  the global threads, and for the
> > > > >> replacement that will depend on Kip-663.
> > > > >>
> > > > >> Best,
> > > > >>
> > > > >> On Wed, Sep 30, 2020 at 4:59 AM Bruno Cadonna <bruno@confluent.io
> >
> > > > wrote:
> > > > >>
> > > > >>> Thanks a lot Walker!
> > > > >>>
> > > > >>> +1 (non-binding)
> > > > >>>
> > > > >>> Best,
> > > > >>> Bruno
> > > > >>>
> > > > >>> On 30.09.20 03:10, Matthias J. Sax wrote:
> > > > >>>> Thanks Walker. The proposed API changes LGTM.
> > > > >>>>
> > > > >>>> +1 (binding)
> > > > >>>>
> > > > >>>> One minor nit: you should also mention the global-thread that
> also
> > > > needs
> > > > >>>> to be shutdown if requested by the user.
> > > > >>>>
> > > > >>>> Minor side question: should we actually terminate a thread and
> > > create
> > > > a
> > > > >>>> new one, or instead revive the existing thread (reusing its
> existing
> > > > >>> ID)?
> > > > >>>>
> > > > >>>>
> > > > >>>> -Matthias
> > > > >>>>
> > > > >>>> On 9/29/20 2:39 PM, Bill Bejeck wrote:
> > > > >>>>> Thanks for the KIP Walker.
> > > > >>>>>
> > > > >>>>> +1 (binding)
> > > > >>>>>
> > > > >>>>> -Bill
> > > > >>>>>
> > > > >>>>> On Tue, Sep 29, 2020 at 4:59 PM Guozhang Wang <
> wangguoz@gmail.com>
> > > > >>> wrote:
> > > > >>>>>
> > > > >>>>>> +1 again on the KIP.
> > > > >>>>>>
> > > > >>>>>> On Tue, Sep 29, 2020 at 1:51 PM Leah Thomas <
> lthomas@confluent.io
> > > >
> > > > >>> wrote:
> > > > >>>>>>
> > > > >>>>>>> Hey Walker,
> > > > >>>>>>>
> > > > >>>>>>> Thanks for the KIP! I'm +1, non-binding.
> > > > >>>>>>>
> > > > >>>>>>> Cheers,
> > > > >>>>>>> Leah
> > > > >>>>>>>
> > > > >>>>>>> On Tue, Sep 29, 2020 at 1:56 PM Walker Carlson <
> > > > >>> wcarlson@confluent.io>
> > > > >>>>>>> wrote:
> > > > >>>>>>>
> > > > >>>>>>>> Hello all,
> > > > >>>>>>>>
> > > > >>>>>>>> I made some changes to the KIP the descriptions are on the
> > > > >>> discussion
> > > > >>>>>>>> thread. If you have already voted I would ask you to confirm
> > > your
> > > > >>> vote.
> > > > >>>>>>>>
> > > > >>>>>>>> Otherwise please vote so we can get this feature in.
> > > > >>>>>>>>
> > > > >>>>>>>> Thanks,
> > > > >>>>>>>> Walker
> > > > >>>>>>>>
> > > > >>>>>>>> On Thu, Sep 24, 2020 at 4:36 PM John Roesler <
> > > vvcephei@apache.org
> > > > >
> > > > >>>>>>> wrote:
> > > > >>>>>>>>
> > > > >>>>>>>>> Thanks for the KIP, Walker!
> > > > >>>>>>>>>
> > > > >>>>>>>>> I’m +1 (binding)
> > > > >>>>>>>>>
> > > > >>>>>>>>> -John
> > > > >>>>>>>>>
> > > > >>>>>>>>> On Mon, Sep 21, 2020, at 17:04, Guozhang Wang wrote:
> > > > >>>>>>>>>> Thanks for finalizing the KIP. +1 (binding)
> > > > >>>>>>>>>>
> > > > >>>>>>>>>>
> > > > >>>>>>>>>> Guozhang
> > > > >>>>>>>>>>
> > > > >>>>>>>>>> On Mon, Sep 21, 2020 at 1:38 PM Walker Carlson <
> > > > >>>>>>> wcarlson@confluent.io>
> > > > >>>>>>>>>> wrote:
> > > > >>>>>>>>>>
> > > > >>>>>>>>>>> Hello all,
> > > > >>>>>>>>>>>
> > > > >>>>>>>>>>> I would like to start a thread to vote for KIP-671 to
> add a
> > > > >>>>>> method
> > > > >>>>>>> to
> > > > >>>>>>>>> close
> > > > >>>>>>>>>>> all clients in a kafka streams application.
> > > > >>>>>>>>>>>
> > > > >>>>>>>>>>> KIP:
> > > > >>>>>>>>>>>
> > > > >>>>>>>>>>>
> > > > >>>>>>>>>
> > > > >>>>>>>>
> > > > >>>>>>>
> > > > >>>>>>
> > > > >>>
> > > >
> > >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-671%3A+Shutdown+Streams+Application+when+appropriate+exception+is+thrown
> > > > >>>>>>>>>>>
> > > > >>>>>>>>>>> Discussion thread: *here
> > > > >>>>>>>>>>> <
> > > > >>>>>>>>>>>
> > > > >>>>>>>>>
> > > > >>>>>>>>
> > > > >>>>>>>
> > > > >>>>>>
> > > > >>>
> > > >
> > >
> https://mail-archives.apache.org/mod_mbox/kafka-dev/202009.mbox/%3CCAC55fuh3HAGCxz-PzxTJraczy6T-os2oiCV328PBeuJQSVYASg%40mail.gmail.com%3E
> > > > >>>>>>>>>>>> *
> > > > >>>>>>>>>>>
> > > > >>>>>>>>>>> Thanks,
> > > > >>>>>>>>>>> -Walker
> > > > >>>>>>>>>>>
> > > > >>>>>>>>>>
> > > > >>>>>>>>>>
> > > > >>>>>>>>>> --
> > > > >>>>>>>>>> -- Guozhang
> > > > >>>>>>>>>>
> > > > >>>>>>>>>
> > > > >>>>>>>>
> > > > >>>>>>>
> > > > >>>>>>
> > > > >>>>>>
> > > > >>>>>> --
> > > > >>>>>> -- Guozhang
> > > > >>>>>>
> > > > >>>>>
> > > > >>>
> > > > >>
> > > > >
> > > >
> > >
> >
>

Re: [VOTE] KIP-671: Add method to Shutdown entire Streams Application

Posted by John Roesler <vv...@apache.org>.

Hello Walker,

Sorry for the late reply, but I didn’t follow the reasoning for the separate handler. You said that the global thread doesn’t have “replace”, but as of today, none of the threads have “replace”. Why not add that ability when we add it for the other threads?

The nature of an uncaught exception handler is that there is an exception that will kill the thread. In that case, it seems like replacement is a desirable option. 

What have I missed?

Thanks,
John

On Tue, Oct 13, 2020, at 15:49, Walker Carlson wrote:
> Those are good points Sophie and Matthias. I sepificed the defaults in the
> kip and standardized the names fo the handler to make them a bit more
> readable.
> 
> Thanks for the suggestions,
> Walker
> 
> On Tue, Oct 13, 2020 at 12:24 PM Sophie Blee-Goldman <so...@confluent.io>
> wrote:
> 
> > Super nit: can we standardize the method & enum names?
> >
> > Right now we have these enums:
> > StreamsUncaughtExceptionHandlerResponse
> > StreamsUncaughtExceptionHandlerResponseGlobalThread
> >
> > and these callbacks:
> > handleUncaughtException()
> > handleExceptionInGlobalThread()
> >
> > The method names have different syntax, which is a bit clunky. I don't have
> > any
> > strong opinions on what grammar they should follow, just that it should be
> > the
> > same for each. I also think that we should specify "StreamThread" somewhere
> > in the name of the StreadThread-specific callback, now that we have a
> > second
> > callback that specifies it's for the GlobalThread. Something like
> > "*handleStreamThreadException()*" and "*handleGlobalThreadException*"
> >
> > The enums are ok, although I think we should include "StreamThread"
> > somewhere
> > like with the callbacks. And we can probably shorten them a bit. For
> > example
> > "*StreamThreadExceptionResponse*" and "*GlobalThreadExceptionResponse*"
> >
> >
> >
> > On Tue, Oct 13, 2020 at 11:48 AM Matthias J. Sax <mj...@apache.org> wrote:
> >
> > > Thanks Walker.
> > >
> > > Overall, LGTM. However, I am wondering if we should have default
> > > implementations for both handler methods? Before the latest change,
> > > there was only one method and having a default was not necessary.
> > > However, forcing people to implement both methods might not be the best
> > > user experience: for example, if there is no global thread, one should
> > > not need to implement the global handler method (and the other way
> > around).
> > >
> > > Thus, it might be good to add default for both methods. If we add
> > > defaults, we should explain the default behavior to the KIP.
> > >
> > > -Matthias
> > >
> > > On 10/12/20 2:32 PM, Walker Carlson wrote:
> > > > Hello all,
> > > >
> > > > I just wanted to let you know that I had to make 2 minor updates to the
> > > KIP.
> > > >
> > > > 1) I changed the behavior of the shutdown client to not leave the
> > client
> > > in
> > > > Error but instead close directly because this aligns better with our
> > > > state machine.
> > > >
> > > > 2) I added a separate call back for the global thread as it does not
> > have
> > > > all the options as a streamThread does. i.e. replace. The default will
> > be
> > > > to close the client. that will also be the only option as that is the
> > > > current behavior for the global thread.
> > > >
> > > > you can find the diff here:
> > > >
> > >
> > https://cwiki.apache.org/confluence/pages/diffpagesbyversion.action?pageId=158876566&originalVersion=21&revisedVersion=23
> > > >
> > > > If you have any problems with these changes let me know and we can
> > > discuss
> > > > them further
> > > >
> > > > Thank you,
> > > > Walker
> > > >
> > > > On Wed, Sep 30, 2020 at 7:33 AM Walker Carlson <wc...@confluent.io>
> > > > wrote:
> > > >
> > > >>
> > > >> Bruno Cadonna <br...@confluent.io>
> > > >> 4:51 AM (2 hours ago)
> > > >> to dev
> > > >> Thank you all for voting!
> > > >>
> > > >> This KIP is accepted with +3 binding (Guozhang, Bill, Matthias) and +2
> > > >> non-binding (Bruno, Leah).
> > > >>
> > > >> Matthias, we will take care of  the global threads, and for the
> > > >> replacement that will depend on Kip-663.
> > > >>
> > > >> Best,
> > > >>
> > > >> On Wed, Sep 30, 2020 at 4:59 AM Bruno Cadonna <br...@confluent.io>
> > > wrote:
> > > >>
> > > >>> Thanks a lot Walker!
> > > >>>
> > > >>> +1 (non-binding)
> > > >>>
> > > >>> Best,
> > > >>> Bruno
> > > >>>
> > > >>> On 30.09.20 03:10, Matthias J. Sax wrote:
> > > >>>> Thanks Walker. The proposed API changes LGTM.
> > > >>>>
> > > >>>> +1 (binding)
> > > >>>>
> > > >>>> One minor nit: you should also mention the global-thread that also
> > > needs
> > > >>>> to be shutdown if requested by the user.
> > > >>>>
> > > >>>> Minor side question: should we actually terminate a thread and
> > create
> > > a
> > > >>>> new one, or instead revive the existing thread (reusing its existing
> > > >>> ID)?
> > > >>>>
> > > >>>>
> > > >>>> -Matthias
> > > >>>>
> > > >>>> On 9/29/20 2:39 PM, Bill Bejeck wrote:
> > > >>>>> Thanks for the KIP Walker.
> > > >>>>>
> > > >>>>> +1 (binding)
> > > >>>>>
> > > >>>>> -Bill
> > > >>>>>
> > > >>>>> On Tue, Sep 29, 2020 at 4:59 PM Guozhang Wang <wa...@gmail.com>
> > > >>> wrote:
> > > >>>>>
> > > >>>>>> +1 again on the KIP.
> > > >>>>>>
> > > >>>>>> On Tue, Sep 29, 2020 at 1:51 PM Leah Thomas <lthomas@confluent.io
> > >
> > > >>> wrote:
> > > >>>>>>
> > > >>>>>>> Hey Walker,
> > > >>>>>>>
> > > >>>>>>> Thanks for the KIP! I'm +1, non-binding.
> > > >>>>>>>
> > > >>>>>>> Cheers,
> > > >>>>>>> Leah
> > > >>>>>>>
> > > >>>>>>> On Tue, Sep 29, 2020 at 1:56 PM Walker Carlson <
> > > >>> wcarlson@confluent.io>
> > > >>>>>>> wrote:
> > > >>>>>>>
> > > >>>>>>>> Hello all,
> > > >>>>>>>>
> > > >>>>>>>> I made some changes to the KIP the descriptions are on the
> > > >>> discussion
> > > >>>>>>>> thread. If you have already voted I would ask you to confirm
> > your
> > > >>> vote.
> > > >>>>>>>>
> > > >>>>>>>> Otherwise please vote so we can get this feature in.
> > > >>>>>>>>
> > > >>>>>>>> Thanks,
> > > >>>>>>>> Walker
> > > >>>>>>>>
> > > >>>>>>>> On Thu, Sep 24, 2020 at 4:36 PM John Roesler <
> > vvcephei@apache.org
> > > >
> > > >>>>>>> wrote:
> > > >>>>>>>>
> > > >>>>>>>>> Thanks for the KIP, Walker!
> > > >>>>>>>>>
> > > >>>>>>>>> I’m +1 (binding)
> > > >>>>>>>>>
> > > >>>>>>>>> -John
> > > >>>>>>>>>
> > > >>>>>>>>> On Mon, Sep 21, 2020, at 17:04, Guozhang Wang wrote:
> > > >>>>>>>>>> Thanks for finalizing the KIP. +1 (binding)
> > > >>>>>>>>>>
> > > >>>>>>>>>>
> > > >>>>>>>>>> Guozhang
> > > >>>>>>>>>>
> > > >>>>>>>>>> On Mon, Sep 21, 2020 at 1:38 PM Walker Carlson <
> > > >>>>>>> wcarlson@confluent.io>
> > > >>>>>>>>>> wrote:
> > > >>>>>>>>>>
> > > >>>>>>>>>>> Hello all,
> > > >>>>>>>>>>>
> > > >>>>>>>>>>> I would like to start a thread to vote for KIP-671 to add a
> > > >>>>>> method
> > > >>>>>>> to
> > > >>>>>>>>> close
> > > >>>>>>>>>>> all clients in a kafka streams application.
> > > >>>>>>>>>>>
> > > >>>>>>>>>>> KIP:
> > > >>>>>>>>>>>
> > > >>>>>>>>>>>
> > > >>>>>>>>>
> > > >>>>>>>>
> > > >>>>>>>
> > > >>>>>>
> > > >>>
> > >
> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-671%3A+Shutdown+Streams+Application+when+appropriate+exception+is+thrown
> > > >>>>>>>>>>>
> > > >>>>>>>>>>> Discussion thread: *here
> > > >>>>>>>>>>> <
> > > >>>>>>>>>>>
> > > >>>>>>>>>
> > > >>>>>>>>
> > > >>>>>>>
> > > >>>>>>
> > > >>>
> > >
> > https://mail-archives.apache.org/mod_mbox/kafka-dev/202009.mbox/%3CCAC55fuh3HAGCxz-PzxTJraczy6T-os2oiCV328PBeuJQSVYASg%40mail.gmail.com%3E
> > > >>>>>>>>>>>> *
> > > >>>>>>>>>>>
> > > >>>>>>>>>>> Thanks,
> > > >>>>>>>>>>> -Walker
> > > >>>>>>>>>>>
> > > >>>>>>>>>>
> > > >>>>>>>>>>
> > > >>>>>>>>>> --
> > > >>>>>>>>>> -- Guozhang
> > > >>>>>>>>>>
> > > >>>>>>>>>
> > > >>>>>>>>
> > > >>>>>>>
> > > >>>>>>
> > > >>>>>>
> > > >>>>>> --
> > > >>>>>> -- Guozhang
> > > >>>>>>
> > > >>>>>
> > > >>>
> > > >>
> > > >
> > >
> >
>

Re: [VOTE] KIP-671: Add method to Shutdown entire Streams Application

Posted by Walker Carlson <wc...@confluent.io>.

Those are good points Sophie and Matthias. I sepificed the defaults in the
kip and standardized the names fo the handler to make them a bit more
readable.

Thanks for the suggestions,
Walker

On Tue, Oct 13, 2020 at 12:24 PM Sophie Blee-Goldman <so...@confluent.io>
wrote:

> Super nit: can we standardize the method & enum names?
>
> Right now we have these enums:
> StreamsUncaughtExceptionHandlerResponse
> StreamsUncaughtExceptionHandlerResponseGlobalThread
>
> and these callbacks:
> handleUncaughtException()
> handleExceptionInGlobalThread()
>
> The method names have different syntax, which is a bit clunky. I don't have
> any
> strong opinions on what grammar they should follow, just that it should be
> the
> same for each. I also think that we should specify "StreamThread" somewhere
> in the name of the StreadThread-specific callback, now that we have a
> second
> callback that specifies it's for the GlobalThread. Something like
> "*handleStreamThreadException()*" and "*handleGlobalThreadException*"
>
> The enums are ok, although I think we should include "StreamThread"
> somewhere
> like with the callbacks. And we can probably shorten them a bit. For
> example
> "*StreamThreadExceptionResponse*" and "*GlobalThreadExceptionResponse*"
>
>
>
> On Tue, Oct 13, 2020 at 11:48 AM Matthias J. Sax <mj...@apache.org> wrote:
>
> > Thanks Walker.
> >
> > Overall, LGTM. However, I am wondering if we should have default
> > implementations for both handler methods? Before the latest change,
> > there was only one method and having a default was not necessary.
> > However, forcing people to implement both methods might not be the best
> > user experience: for example, if there is no global thread, one should
> > not need to implement the global handler method (and the other way
> around).
> >
> > Thus, it might be good to add default for both methods. If we add
> > defaults, we should explain the default behavior to the KIP.
> >
> > -Matthias
> >
> > On 10/12/20 2:32 PM, Walker Carlson wrote:
> > > Hello all,
> > >
> > > I just wanted to let you know that I had to make 2 minor updates to the
> > KIP.
> > >
> > > 1) I changed the behavior of the shutdown client to not leave the
> client
> > in
> > > Error but instead close directly because this aligns better with our
> > > state machine.
> > >
> > > 2) I added a separate call back for the global thread as it does not
> have
> > > all the options as a streamThread does. i.e. replace. The default will
> be
> > > to close the client. that will also be the only option as that is the
> > > current behavior for the global thread.
> > >
> > > you can find the diff here:
> > >
> >
> https://cwiki.apache.org/confluence/pages/diffpagesbyversion.action?pageId=158876566&originalVersion=21&revisedVersion=23
> > >
> > > If you have any problems with these changes let me know and we can
> > discuss
> > > them further
> > >
> > > Thank you,
> > > Walker
> > >
> > > On Wed, Sep 30, 2020 at 7:33 AM Walker Carlson <wc...@confluent.io>
> > > wrote:
> > >
> > >>
> > >> Bruno Cadonna <br...@confluent.io>
> > >> 4:51 AM (2 hours ago)
> > >> to dev
> > >> Thank you all for voting!
> > >>
> > >> This KIP is accepted with +3 binding (Guozhang, Bill, Matthias) and +2
> > >> non-binding (Bruno, Leah).
> > >>
> > >> Matthias, we will take care of  the global threads, and for the
> > >> replacement that will depend on Kip-663.
> > >>
> > >> Best,
> > >>
> > >> On Wed, Sep 30, 2020 at 4:59 AM Bruno Cadonna <br...@confluent.io>
> > wrote:
> > >>
> > >>> Thanks a lot Walker!
> > >>>
> > >>> +1 (non-binding)
> > >>>
> > >>> Best,
> > >>> Bruno
> > >>>
> > >>> On 30.09.20 03:10, Matthias J. Sax wrote:
> > >>>> Thanks Walker. The proposed API changes LGTM.
> > >>>>
> > >>>> +1 (binding)
> > >>>>
> > >>>> One minor nit: you should also mention the global-thread that also
> > needs
> > >>>> to be shutdown if requested by the user.
> > >>>>
> > >>>> Minor side question: should we actually terminate a thread and
> create
> > a
> > >>>> new one, or instead revive the existing thread (reusing its existing
> > >>> ID)?
> > >>>>
> > >>>>
> > >>>> -Matthias
> > >>>>
> > >>>> On 9/29/20 2:39 PM, Bill Bejeck wrote:
> > >>>>> Thanks for the KIP Walker.
> > >>>>>
> > >>>>> +1 (binding)
> > >>>>>
> > >>>>> -Bill
> > >>>>>
> > >>>>> On Tue, Sep 29, 2020 at 4:59 PM Guozhang Wang <wa...@gmail.com>
> > >>> wrote:
> > >>>>>
> > >>>>>> +1 again on the KIP.
> > >>>>>>
> > >>>>>> On Tue, Sep 29, 2020 at 1:51 PM Leah Thomas <lthomas@confluent.io
> >
> > >>> wrote:
> > >>>>>>
> > >>>>>>> Hey Walker,
> > >>>>>>>
> > >>>>>>> Thanks for the KIP! I'm +1, non-binding.
> > >>>>>>>
> > >>>>>>> Cheers,
> > >>>>>>> Leah
> > >>>>>>>
> > >>>>>>> On Tue, Sep 29, 2020 at 1:56 PM Walker Carlson <
> > >>> wcarlson@confluent.io>
> > >>>>>>> wrote:
> > >>>>>>>
> > >>>>>>>> Hello all,
> > >>>>>>>>
> > >>>>>>>> I made some changes to the KIP the descriptions are on the
> > >>> discussion
> > >>>>>>>> thread. If you have already voted I would ask you to confirm
> your
> > >>> vote.
> > >>>>>>>>
> > >>>>>>>> Otherwise please vote so we can get this feature in.
> > >>>>>>>>
> > >>>>>>>> Thanks,
> > >>>>>>>> Walker
> > >>>>>>>>
> > >>>>>>>> On Thu, Sep 24, 2020 at 4:36 PM John Roesler <
> vvcephei@apache.org
> > >
> > >>>>>>> wrote:
> > >>>>>>>>
> > >>>>>>>>> Thanks for the KIP, Walker!
> > >>>>>>>>>
> > >>>>>>>>> I’m +1 (binding)
> > >>>>>>>>>
> > >>>>>>>>> -John
> > >>>>>>>>>
> > >>>>>>>>> On Mon, Sep 21, 2020, at 17:04, Guozhang Wang wrote:
> > >>>>>>>>>> Thanks for finalizing the KIP. +1 (binding)
> > >>>>>>>>>>
> > >>>>>>>>>>
> > >>>>>>>>>> Guozhang
> > >>>>>>>>>>
> > >>>>>>>>>> On Mon, Sep 21, 2020 at 1:38 PM Walker Carlson <
> > >>>>>>> wcarlson@confluent.io>
> > >>>>>>>>>> wrote:
> > >>>>>>>>>>
> > >>>>>>>>>>> Hello all,
> > >>>>>>>>>>>
> > >>>>>>>>>>> I would like to start a thread to vote for KIP-671 to add a
> > >>>>>> method
> > >>>>>>> to
> > >>>>>>>>> close
> > >>>>>>>>>>> all clients in a kafka streams application.
> > >>>>>>>>>>>
> > >>>>>>>>>>> KIP:
> > >>>>>>>>>>>
> > >>>>>>>>>>>
> > >>>>>>>>>
> > >>>>>>>>
> > >>>>>>>
> > >>>>>>
> > >>>
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-671%3A+Shutdown+Streams+Application+when+appropriate+exception+is+thrown
> > >>>>>>>>>>>
> > >>>>>>>>>>> Discussion thread: *here
> > >>>>>>>>>>> <
> > >>>>>>>>>>>
> > >>>>>>>>>
> > >>>>>>>>
> > >>>>>>>
> > >>>>>>
> > >>>
> >
> https://mail-archives.apache.org/mod_mbox/kafka-dev/202009.mbox/%3CCAC55fuh3HAGCxz-PzxTJraczy6T-os2oiCV328PBeuJQSVYASg%40mail.gmail.com%3E
> > >>>>>>>>>>>> *
> > >>>>>>>>>>>
> > >>>>>>>>>>> Thanks,
> > >>>>>>>>>>> -Walker
> > >>>>>>>>>>>
> > >>>>>>>>>>
> > >>>>>>>>>>
> > >>>>>>>>>> --
> > >>>>>>>>>> -- Guozhang
> > >>>>>>>>>>
> > >>>>>>>>>
> > >>>>>>>>
> > >>>>>>>
> > >>>>>>
> > >>>>>>
> > >>>>>> --
> > >>>>>> -- Guozhang
> > >>>>>>
> > >>>>>
> > >>>
> > >>
> > >
> >
>

Re: [VOTE] KIP-671: Add method to Shutdown entire Streams Application

Posted by Sophie Blee-Goldman <so...@confluent.io>.

Super nit: can we standardize the method & enum names?

Right now we have these enums:
StreamsUncaughtExceptionHandlerResponse
StreamsUncaughtExceptionHandlerResponseGlobalThread

and these callbacks:
handleUncaughtException()
handleExceptionInGlobalThread()

The method names have different syntax, which is a bit clunky. I don't have
any
strong opinions on what grammar they should follow, just that it should be
the
same for each. I also think that we should specify "StreamThread" somewhere
in the name of the StreadThread-specific callback, now that we have a second
callback that specifies it's for the GlobalThread. Something like
"*handleStreamThreadException()*" and "*handleGlobalThreadException*"

The enums are ok, although I think we should include "StreamThread"
somewhere
like with the callbacks. And we can probably shorten them a bit. For example
"*StreamThreadExceptionResponse*" and "*GlobalThreadExceptionResponse*"



On Tue, Oct 13, 2020 at 11:48 AM Matthias J. Sax <mj...@apache.org> wrote:

> Thanks Walker.
>
> Overall, LGTM. However, I am wondering if we should have default
> implementations for both handler methods? Before the latest change,
> there was only one method and having a default was not necessary.
> However, forcing people to implement both methods might not be the best
> user experience: for example, if there is no global thread, one should
> not need to implement the global handler method (and the other way around).
>
> Thus, it might be good to add default for both methods. If we add
> defaults, we should explain the default behavior to the KIP.
>
> -Matthias
>
> On 10/12/20 2:32 PM, Walker Carlson wrote:
> > Hello all,
> >
> > I just wanted to let you know that I had to make 2 minor updates to the
> KIP.
> >
> > 1) I changed the behavior of the shutdown client to not leave the client
> in
> > Error but instead close directly because this aligns better with our
> > state machine.
> >
> > 2) I added a separate call back for the global thread as it does not have
> > all the options as a streamThread does. i.e. replace. The default will be
> > to close the client. that will also be the only option as that is the
> > current behavior for the global thread.
> >
> > you can find the diff here:
> >
> https://cwiki.apache.org/confluence/pages/diffpagesbyversion.action?pageId=158876566&originalVersion=21&revisedVersion=23
> >
> > If you have any problems with these changes let me know and we can
> discuss
> > them further
> >
> > Thank you,
> > Walker
> >
> > On Wed, Sep 30, 2020 at 7:33 AM Walker Carlson <wc...@confluent.io>
> > wrote:
> >
> >>
> >> Bruno Cadonna <br...@confluent.io>
> >> 4:51 AM (2 hours ago)
> >> to dev
> >> Thank you all for voting!
> >>
> >> This KIP is accepted with +3 binding (Guozhang, Bill, Matthias) and +2
> >> non-binding (Bruno, Leah).
> >>
> >> Matthias, we will take care of  the global threads, and for the
> >> replacement that will depend on Kip-663.
> >>
> >> Best,
> >>
> >> On Wed, Sep 30, 2020 at 4:59 AM Bruno Cadonna <br...@confluent.io>
> wrote:
> >>
> >>> Thanks a lot Walker!
> >>>
> >>> +1 (non-binding)
> >>>
> >>> Best,
> >>> Bruno
> >>>
> >>> On 30.09.20 03:10, Matthias J. Sax wrote:
> >>>> Thanks Walker. The proposed API changes LGTM.
> >>>>
> >>>> +1 (binding)
> >>>>
> >>>> One minor nit: you should also mention the global-thread that also
> needs
> >>>> to be shutdown if requested by the user.
> >>>>
> >>>> Minor side question: should we actually terminate a thread and create
> a
> >>>> new one, or instead revive the existing thread (reusing its existing
> >>> ID)?
> >>>>
> >>>>
> >>>> -Matthias
> >>>>
> >>>> On 9/29/20 2:39 PM, Bill Bejeck wrote:
> >>>>> Thanks for the KIP Walker.
> >>>>>
> >>>>> +1 (binding)
> >>>>>
> >>>>> -Bill
> >>>>>
> >>>>> On Tue, Sep 29, 2020 at 4:59 PM Guozhang Wang <wa...@gmail.com>
> >>> wrote:
> >>>>>
> >>>>>> +1 again on the KIP.
> >>>>>>
> >>>>>> On Tue, Sep 29, 2020 at 1:51 PM Leah Thomas <lt...@confluent.io>
> >>> wrote:
> >>>>>>
> >>>>>>> Hey Walker,
> >>>>>>>
> >>>>>>> Thanks for the KIP! I'm +1, non-binding.
> >>>>>>>
> >>>>>>> Cheers,
> >>>>>>> Leah
> >>>>>>>
> >>>>>>> On Tue, Sep 29, 2020 at 1:56 PM Walker Carlson <
> >>> wcarlson@confluent.io>
> >>>>>>> wrote:
> >>>>>>>
> >>>>>>>> Hello all,
> >>>>>>>>
> >>>>>>>> I made some changes to the KIP the descriptions are on the
> >>> discussion
> >>>>>>>> thread. If you have already voted I would ask you to confirm your
> >>> vote.
> >>>>>>>>
> >>>>>>>> Otherwise please vote so we can get this feature in.
> >>>>>>>>
> >>>>>>>> Thanks,
> >>>>>>>> Walker
> >>>>>>>>
> >>>>>>>> On Thu, Sep 24, 2020 at 4:36 PM John Roesler <vvcephei@apache.org
> >
> >>>>>>> wrote:
> >>>>>>>>
> >>>>>>>>> Thanks for the KIP, Walker!
> >>>>>>>>>
> >>>>>>>>> I’m +1 (binding)
> >>>>>>>>>
> >>>>>>>>> -John
> >>>>>>>>>
> >>>>>>>>> On Mon, Sep 21, 2020, at 17:04, Guozhang Wang wrote:
> >>>>>>>>>> Thanks for finalizing the KIP. +1 (binding)
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> Guozhang
> >>>>>>>>>>
> >>>>>>>>>> On Mon, Sep 21, 2020 at 1:38 PM Walker Carlson <
> >>>>>>> wcarlson@confluent.io>
> >>>>>>>>>> wrote:
> >>>>>>>>>>
> >>>>>>>>>>> Hello all,
> >>>>>>>>>>>
> >>>>>>>>>>> I would like to start a thread to vote for KIP-671 to add a
> >>>>>> method
> >>>>>>> to
> >>>>>>>>> close
> >>>>>>>>>>> all clients in a kafka streams application.
> >>>>>>>>>>>
> >>>>>>>>>>> KIP:
> >>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>
> >>>>>>
> >>>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-671%3A+Shutdown+Streams+Application+when+appropriate+exception+is+thrown
> >>>>>>>>>>>
> >>>>>>>>>>> Discussion thread: *here
> >>>>>>>>>>> <
> >>>>>>>>>>>
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>
> >>>>>>
> >>>
> https://mail-archives.apache.org/mod_mbox/kafka-dev/202009.mbox/%3CCAC55fuh3HAGCxz-PzxTJraczy6T-os2oiCV328PBeuJQSVYASg%40mail.gmail.com%3E
> >>>>>>>>>>>> *
> >>>>>>>>>>>
> >>>>>>>>>>> Thanks,
> >>>>>>>>>>> -Walker
> >>>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> --
> >>>>>>>>>> -- Guozhang
> >>>>>>>>>>
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>
> >>>>>>
> >>>>>>
> >>>>>> --
> >>>>>> -- Guozhang
> >>>>>>
> >>>>>
> >>>
> >>
> >
>

Re: [VOTE] KIP-671: Add method to Shutdown entire Streams Application

Posted by "Matthias J. Sax" <mj...@apache.org>.

Thanks Walker.

Overall, LGTM. However, I am wondering if we should have default
implementations for both handler methods? Before the latest change,
there was only one method and having a default was not necessary.
However, forcing people to implement both methods might not be the best
user experience: for example, if there is no global thread, one should
not need to implement the global handler method (and the other way around).

Thus, it might be good to add default for both methods. If we add
defaults, we should explain the default behavior to the KIP.

-Matthias

On 10/12/20 2:32 PM, Walker Carlson wrote:
> Hello all,
> 
> I just wanted to let you know that I had to make 2 minor updates to the KIP.
> 
> 1) I changed the behavior of the shutdown client to not leave the client in
> Error but instead close directly because this aligns better with our
> state machine.
> 
> 2) I added a separate call back for the global thread as it does not have
> all the options as a streamThread does. i.e. replace. The default will be
> to close the client. that will also be the only option as that is the
> current behavior for the global thread.
> 
> you can find the diff here:
> https://cwiki.apache.org/confluence/pages/diffpagesbyversion.action?pageId=158876566&originalVersion=21&revisedVersion=23
> 
> If you have any problems with these changes let me know and we can discuss
> them further
> 
> Thank you,
> Walker
> 
> On Wed, Sep 30, 2020 at 7:33 AM Walker Carlson <wc...@confluent.io>
> wrote:
> 
>>
>> Bruno Cadonna <br...@confluent.io>
>> 4:51 AM (2 hours ago)
>> to dev
>> Thank you all for voting!
>>
>> This KIP is accepted with +3 binding (Guozhang, Bill, Matthias) and +2
>> non-binding (Bruno, Leah).
>>
>> Matthias, we will take care of  the global threads, and for the
>> replacement that will depend on Kip-663.
>>
>> Best,
>>
>> On Wed, Sep 30, 2020 at 4:59 AM Bruno Cadonna <br...@confluent.io> wrote:
>>
>>> Thanks a lot Walker!
>>>
>>> +1 (non-binding)
>>>
>>> Best,
>>> Bruno
>>>
>>> On 30.09.20 03:10, Matthias J. Sax wrote:
>>>> Thanks Walker. The proposed API changes LGTM.
>>>>
>>>> +1 (binding)
>>>>
>>>> One minor nit: you should also mention the global-thread that also needs
>>>> to be shutdown if requested by the user.
>>>>
>>>> Minor side question: should we actually terminate a thread and create a
>>>> new one, or instead revive the existing thread (reusing its existing
>>> ID)?
>>>>
>>>>
>>>> -Matthias
>>>>
>>>> On 9/29/20 2:39 PM, Bill Bejeck wrote:
>>>>> Thanks for the KIP Walker.
>>>>>
>>>>> +1 (binding)
>>>>>
>>>>> -Bill
>>>>>
>>>>> On Tue, Sep 29, 2020 at 4:59 PM Guozhang Wang <wa...@gmail.com>
>>> wrote:
>>>>>
>>>>>> +1 again on the KIP.
>>>>>>
>>>>>> On Tue, Sep 29, 2020 at 1:51 PM Leah Thomas <lt...@confluent.io>
>>> wrote:
>>>>>>
>>>>>>> Hey Walker,
>>>>>>>
>>>>>>> Thanks for the KIP! I'm +1, non-binding.
>>>>>>>
>>>>>>> Cheers,
>>>>>>> Leah
>>>>>>>
>>>>>>> On Tue, Sep 29, 2020 at 1:56 PM Walker Carlson <
>>> wcarlson@confluent.io>
>>>>>>> wrote:
>>>>>>>
>>>>>>>> Hello all,
>>>>>>>>
>>>>>>>> I made some changes to the KIP the descriptions are on the
>>> discussion
>>>>>>>> thread. If you have already voted I would ask you to confirm your
>>> vote.
>>>>>>>>
>>>>>>>> Otherwise please vote so we can get this feature in.
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Walker
>>>>>>>>
>>>>>>>> On Thu, Sep 24, 2020 at 4:36 PM John Roesler <vv...@apache.org>
>>>>>>> wrote:
>>>>>>>>
>>>>>>>>> Thanks for the KIP, Walker!
>>>>>>>>>
>>>>>>>>> I’m +1 (binding)
>>>>>>>>>
>>>>>>>>> -John
>>>>>>>>>
>>>>>>>>> On Mon, Sep 21, 2020, at 17:04, Guozhang Wang wrote:
>>>>>>>>>> Thanks for finalizing the KIP. +1 (binding)
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Guozhang
>>>>>>>>>>
>>>>>>>>>> On Mon, Sep 21, 2020 at 1:38 PM Walker Carlson <
>>>>>>> wcarlson@confluent.io>
>>>>>>>>>> wrote:
>>>>>>>>>>
>>>>>>>>>>> Hello all,
>>>>>>>>>>>
>>>>>>>>>>> I would like to start a thread to vote for KIP-671 to add a
>>>>>> method
>>>>>>> to
>>>>>>>>> close
>>>>>>>>>>> all clients in a kafka streams application.
>>>>>>>>>>>
>>>>>>>>>>> KIP:
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-671%3A+Shutdown+Streams+Application+when+appropriate+exception+is+thrown
>>>>>>>>>>>
>>>>>>>>>>> Discussion thread: *here
>>>>>>>>>>> <
>>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>> https://mail-archives.apache.org/mod_mbox/kafka-dev/202009.mbox/%3CCAC55fuh3HAGCxz-PzxTJraczy6T-os2oiCV328PBeuJQSVYASg%40mail.gmail.com%3E
>>>>>>>>>>>> *
>>>>>>>>>>>
>>>>>>>>>>> Thanks,
>>>>>>>>>>> -Walker
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> --
>>>>>>>>>> -- Guozhang
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> -- Guozhang
>>>>>>
>>>>>
>>>
>>
>