Yes. Sorry about this mistake.
On 6/16/21 2:29 PM, Israel Ekpo wrote: > Thanks for clarifying that @Sophie it is in regards to KIP-633 > > On Wed, Jun 16, 2021 at 4:00 PM Sophie Blee-Goldman > <sop...@confluent.io.invalid> wrote: > >> 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 >>>>>>>>>>> >>>>>>>>> >>>>> >>>> >>> >> >
