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

2020-10-20 Thread Matthias J. Sax
+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

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

2020-10-19 Thread John Roesler
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 ma

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

2020-10-19 Thread Walker Carlson
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/

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

2020-10-14 Thread John Roesler
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 t

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

2020-10-14 Thread Sophie Blee-Goldman
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, w

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

2020-10-14 Thread John Roesler
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 uncau

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

2020-10-13 Thread Walker Carlson
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 wrote: > Super nit: can we standardize the method &

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

2020-10-13 Thread Sophie Blee-Goldman
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,

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

2020-10-13 Thread Matthias J. Sax
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 experie

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

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

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

2020-09-30 Thread Walker Carlson
Bruno Cadonna 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,

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

2020-09-30 Thread Bruno Cadonna
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: shoul

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

2020-09-29 Thread Matthias J. Sax
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 (reu

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

2020-09-29 Thread Bill Bejeck
Thanks for the KIP Walker. +1 (binding) -Bill On Tue, Sep 29, 2020 at 4:59 PM Guozhang Wang wrote: > +1 again on the KIP. > > On Tue, Sep 29, 2020 at 1:51 PM Leah Thomas wrote: > > > Hey Walker, > > > > Thanks for the KIP! I'm +1, non-binding. > > > > Cheers, > > Leah > > > > On Tue, Sep 29,

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

2020-09-29 Thread Guozhang Wang
+1 again on the KIP. On Tue, Sep 29, 2020 at 1:51 PM Leah Thomas 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 > wrote: > > > Hello all, > > > > I made some changes to the KIP the descriptions are on the d

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

2020-09-29 Thread Leah Thomas
Hey Walker, Thanks for the KIP! I'm +1, non-binding. Cheers, Leah On Tue, Sep 29, 2020 at 1:56 PM Walker Carlson 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

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

2020-09-29 Thread Walker Carlson
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 wrote: > Thanks for the KIP

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

2020-09-24 Thread John Roesler
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 > wrote: > > > Hello all, > > > > I would like to start a thread to vote fo

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

2020-09-21 Thread Guozhang Wang
Thanks for finalizing the KIP. +1 (binding) Guozhang On Mon, Sep 21, 2020 at 1:38 PM Walker Carlson 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

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

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