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 <vvcep...@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 <
> sop...@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 <
> wcarl...@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 <
> wangg...@gmail.com>
> > > > >>> wrote:
> > > > >>>>>
> > > > >>>>>> +1 again on the KIP.
> > > > >>>>>>
> > > > >>>>>> On Tue, Sep 29, 2020 at 1:51 PM Leah Thomas <
> ltho...@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 <
> > > > >>> wcarl...@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 <
> > > vvcep...@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 <
> > > > >>>>>>> wcarl...@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
> > > > >>>>>>
> > > > >>>>>
> > > > >>>
> > > > >>
> > > > >
> > > >
> > >
> >
>

Reply via email to