Matthias, I'm guessing you meant to send this to the KIP-633 list? This is KIP-663
On Wed, Jun 16, 2021 at 12:37 PM Matthias J. Sax <mj...@apache.org> wrote: > Quick follow up. I did a small update to the KIP with regard to > https://issues.apache.org/jira/browse/KAFKA-12909 > > Israel, Sophie, and Guozhang did agree to this change. I don't think we > need to re-vote. > > Please let us know if there are any concerns. > > > -Matthias > > On 1/27/21 12:48 PM, Sophie Blee-Goldman wrote: > > Thanks Bruno, that sounds like a good addition. +1 > > > > On Wed, Jan 27, 2021 at 12:34 PM Bruno Cadonna <br...@confluent.io> > wrote: > > > >> Hi all, > >> > >> During the implementation, we notices that method removeStreamThread() > >> may block indefinitely when the stream thread chosen for removal is > >> blocked and cannot be shut down. Thus, we will add an overload that > >> takes a timeout. The newly added method will throw a TimeoutException, > >> when the timeout is exceeded. > >> > >> We updated the KIP accordingly. > >> > >> KIP: https://cwiki.apache.org/confluence/x/FDd4CQ > >> > >> Best, > >> Bruno > >> > >> On 30.09.20 13:51, Bruno Cadonna wrote: > >>> Thank you all for voting! > >>> > >>> This KIP is accepted with 3 binding +1 (Guozhang, John, Matthias). > >>> > >>> Best, > >>> Bruno > >>> > >>> On 29.09.20 22:24, Matthias J. Sax wrote: > >>>> +1 (binding) > >>>> > >>>> I am not super happy with the impact on the client state. For > example, I > >>>> don't understand why it's ok to scale out if we lose one thread out of > >>>> four, but why it's not ok to scale out if we lose one thread out of > one > >>>> (for this case, we would enter ERROR state and cannot add new threads > >>>> afterwards). > >>>> > >>>> However, this might be an issue for a follow up KIP. > >>>> > >>>> > >>>> -Matthias > >>>> > >>>> On 9/29/20 7:20 AM, John Roesler wrote: > >>>>> Thanks, Bruno, this sounds good to me. > >>>>> -John > >>>>> > >>>>> On Tue, Sep 29, 2020, at 03:13, Bruno Cadonna wrote: > >>>>>> Hi all, > >>>>>> > >>>>>> I did two minor modifications to the KIP. > >>>>>> > >>>>>> - I removed the rather strict guarantee "Dead stream threads are > >>>>>> removed > >>>>>> from a Kafka Streams client at latest after the next call to > >>>>>> KafkaStreams#addStreamThread() or KafkaStreams#removeStreamThread() > >>>>>> following the transition to state DEAD." > >>>>>> Dead stream threads will be still removed, but the behavior will be > >>>>>> less > >>>>>> strict. > >>>>>> > >>>>>> - Added a sentence that states that the Kafka Streams client will > >>>>>> transit to ERROR if the last alive stream thread dies exceptionally. > >>>>>> This corresponds to the current behavior. > >>>>>> > >>>>>> I will not restart voting and keep the votes so far. > >>>>>> > >>>>>> Best, > >>>>>> Bruno > >>>>>> > >>>>>> On 22.09.20 01:19, John Roesler wrote: > >>>>>>> I’m +1 also. Thanks, Bruno! > >>>>>>> -John > >>>>>>> > >>>>>>> On Mon, Sep 21, 2020, at 17:08, Guozhang Wang wrote: > >>>>>>>> Thanks Bruno. I'm +1 on the KIP. > >>>>>>>> > >>>>>>>> On Mon, Sep 21, 2020 at 2:49 AM Bruno Cadonna <br...@confluent.io > > > >>>>>>>> wrote: > >>>>>>>> > >>>>>>>>> Hi, > >>>>>>>>> > >>>>>>>>> I would like to restart from zero the voting on KIP-663 that > >>>>>>>>> proposes to > >>>>>>>>> add methods to the Kafka Streams client to add and remove stream > >>>>>>>>> threads > >>>>>>>>> during execution. > >>>>>>>>> > >>>>>>>>> > >>>>>>>>> > >> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-663%3A+API+to+Start+and+Shut+Down+Stream+Threads > >>>>>>>>> > >>>>>>>>> > >>>>>>>>> Matthias, if you are still +1, please vote again. > >>>>>>>>> > >>>>>>>>> Best, > >>>>>>>>> Bruno > >>>>>>>>> > >>>>>>>>> On 04.09.20 23:12, John Roesler wrote: > >>>>>>>>>> Hi Sophie, > >>>>>>>>>> > >>>>>>>>>> Uh, oh, it's never a good sign when the discussion moves > >>>>>>>>>> into the vote thread :) > >>>>>>>>>> > >>>>>>>>>> I agree with you, it seems like a good touch for > >>>>>>>>>> removeStreamThread() to return the name of the thread that > >>>>>>>>>> got removed, rather than a boolean flag. Maybe the return > >>>>>>>>>> value would be `null` if there is no thread to remove. > >>>>>>>>>> > >>>>>>>>>> If we go that way, I'd suggest that addStreamThread() also > >>>>>>>>>> return the name of the newly created thread, or null if no > >>>>>>>>>> thread can be created right now. > >>>>>>>>>> > >>>>>>>>>> I'm not completely sure if I think that callers of this > >>>>>>>>>> method would know exactly how many threads there are. Sure, > >>>>>>>>>> if a human being is sitting there looking at the metrics or > >>>>>>>>>> logs and decides to call the method, it would work out, but > >>>>>>>>>> I'd expect this kind of method to find its way into > >>>>>>>>>> automated tooling that reacts to things like current system > >>>>>>>>>> load or resource saturation. Those kinds of toolchains often > >>>>>>>>>> are part of a distributed system, and it's probably not that > >>>>>>>>>> easy to guarantee that the thread count they observe is > >>>>>>>>>> fully consistent with the number of threads that are > >>>>>>>>>> actually running. Therefore, an in-situ `int > >>>>>>>>>> numStreamThreads()` method might not be a bad idea. Then > >>>>>>>>>> again, it seems sort of optional. A caller can catch an > >>>>>>>>>> exception or react to a `null` return value just the same > >>>>>>>>>> either way. Having both add/remove methods behave similarly > >>>>>>>>>> is probably more valuable. > >>>>>>>>>> > >>>>>>>>>> Thanks, > >>>>>>>>>> -John > >>>>>>>>>> > >>>>>>>>>> > >>>>>>>>>> On Thu, 2020-09-03 at 12:15 -0700, Sophie Blee-Goldman > >>>>>>>>>> wrote: > >>>>>>>>>>> Hey, sorry for the late reply, I just have one minor > >>>>>>>>>>> suggestion. Since > >>>>>>>>> we > >>>>>>>>>>> don't > >>>>>>>>>>> make any guarantees about which thread gets removed or allow > >>>>>>>>>>> the user to > >>>>>>>>>>> specify, I think we should return either the index or full name > >>>>>>>>>>> of the > >>>>>>>>>>> thread > >>>>>>>>>>> that does get removed by removeThread(). > >>>>>>>>>>> > >>>>>>>>>>> I know you just updated the KIP to return true/false if there > >>>>>>>>> are/aren't any > >>>>>>>>>>> threads to be removed, but I think this would be more > >>>>>>>>>>> appropriate as an > >>>>>>>>>>> exception than as a return type. I think it's reasonable to > >> expect > >>>>>>>>> users to > >>>>>>>>>>> have some sense to how many threads are remaining, and not try > >>>>>>>>>>> to remove > >>>>>>>>>>> a thread when there is none left. To me, that indicates > >>>>>>>>>>> something wrong > >>>>>>>>>>> with the user application code and should be treated as an > >>>>>>>>>>> exceptional > >>>>>>>>> case. > >>>>>>>>>>> I don't think the same code clarify argument applies here as to > >>>>>>>>>>> the > >>>>>>>>>>> addStreamThread() case, as there's no reason for an application > >>>>>>>>>>> to be > >>>>>>>>>>> looping and retrying removeStreamThread() since if that fails, > >>>>>>>>>>> it's > >>>>>>>>> because > >>>>>>>>>>> there are no threads left and thus it will continue to always > >>>>>>>>>>> fail. And > >>>>>>>>> if > >>>>>>>>>>> the > >>>>>>>>>>> user actually wants to shut down all threads, they should just > >>>>>>>>>>> close the > >>>>>>>>>>> whole application rather than call removeStreamThread() in a > >> loop. > >>>>>>>>>>> > >>>>>>>>>>> While I generally think it should be straightforward for users > >>>>>>>>>>> to track > >>>>>>>>> how > >>>>>>>>>>> many stream threads they have running, maybe it would be nice > >>>>>>>>>>> to add > >>>>>>>>>>> a small utility method that does this for them. Something like > >>>>>>>>>>> > >>>>>>>>>>> // Returns the number of currently alive threads > >>>>>>>>>>> boolean runningStreamThreads(); > >>>>>>>>>>> > >>>>>>>>>>> On Thu, Sep 3, 2020 at 7:41 AM Matthias J. Sax < > mj...@apache.org > >>> > >>>>>>>>> wrote: > >>>>>>>>>>> > >>>>>>>>>>>> +1 (binding) > >>>>>>>>>>>> > >>>>>>>>>>>> On 9/3/20 6:16 AM, Bruno Cadonna wrote: > >>>>>>>>>>>>> Hi, > >>>>>>>>>>>>> > >>>>>>>>>>>>> I would like to start the voting on KIP-663 that proposes to > >> add > >>>>>>>>> methods > >>>>>>>>>>>>> to the Kafka Streams client to add and remove stream threads > >>>>>>>>>>>>> during > >>>>>>>>>>>>> execution. > >>>>>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>> > >> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-663%3A+API+to+Start+and+Shut+Down+Stream+Threads > >>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>>>> Best, > >>>>>>>>>>>>> Bruno > >>>>>>>>>> > >>>>>>>>> > >>>>>>>> > >>>>>>>> > >>>>>>>> -- > >>>>>>>> -- Guozhang > >>>>>>>> > >>>>>> > >> > > >
