I'm fine with always removing threads at DEAD state from
localThreadsMetadata().
On the other hand, if we want to give users more info for debugging, we can
consider eating the complexity ourselves by guaranteeing that, within the
user registered uncaught exception handler, the
Makes sense to me :)
On Thu, Sep 17, 2020 at 9:34 AM Bruno Cadonna wrote:
> Hi Sophie,
>
> Thank you for the feedback! I replied inline.
>
> Best,
> Bruno
>
> On 16.09.20 19:19, Sophie Blee-Goldman wrote:
> >>
> >> We guarantee that the metadata of the dead stream threads will be
> >> returned
Hi Sophie,
Thank you for the feedback! I replied inline.
Best,
Bruno
On 16.09.20 19:19, Sophie Blee-Goldman wrote:
We guarantee that the metadata of the dead stream threads will be
returned by KafkaStreams#localThreadsMetadata() at least until the next
call to KafkaStreams#addStreamThread()
>
> We guarantee that the metadata of the dead stream threads will be
> returned by KafkaStreams#localThreadsMetadata() at least until the next
> call to KafkaStreams#addStreamThread() or
> KafkaStreams#removeStreamThread() after the stream thread transited to
> DEAD
This seems kind of
Hi again,
I just realized that if we filter out DEAD stream threads in
localThreadsMetadata(), users cannot log the metadata of dying stream
threads in the uncaught exception handler.
I realized this thanks to the example Guozhang requested in the KIP.
Thank you for that, Guozhang!
Hence,
Hi Guozhang,
Good point! I would propose to filter out DEAD stream threads in
localThreadsMetadata() to get consistent results that do not depend on
timing. I will update the KIP accordingly.
Best,
Bruno
On 16.09.20 06:02, Guozhang Wang wrote:
Thanks Bruno, your replies make sense to me.
Thanks Bruno, your replies make sense to me. As for
localThreadsMetadata() itself,
I'd like to clarify if it would return any still-bookkept threads, or would
it specifically filter out those DEAD threads even if they are not yet
removed.
Otherwise, the KIP LGTM.
Guozhang
On Tue, Sep 15, 2020
Hi Guozhang,
Thank you for your feedback. I replied inline.
Best,
Bruno
On 09.09.20 23:43, Guozhang Wang wrote:
Hello Bruno,
Finally got some time to review your KIP and the discussion thread now.. a
few comments below:
1) I'm with Matthias about the newly added numberOfAliveStreamThreads
Hello Bruno,
Finally got some time to review your KIP and the discussion thread now.. a
few comments below:
1) I'm with Matthias about the newly added numberOfAliveStreamThreads v.s.
existing localThreadsMetadata: to me it seems we can always achieve the
first based on the second. It seems not
I would prefer to not add a new method. It seems unnecessary.
`localThreadMetadata` does return all threads in all states(*) and thus
provides full insight.
(*) A thread in state DEAD could be returned as long as it's not removed
yet.
I don't see any advantage to pre-filter threads and to
I agree that the current behavior of localThreadsMetadata() does not seem
to match, but it seems like we will be forced to change it to only return
currently-alive threads. For one thing, we plan to recycle old thread names.
It would be pretty confusing for a user to get two (or more)
Hi Matthias and Sophie,
I agree that localThreadsMetadata() can be used here. However,
localThreadsMetadata() returns all stream threads irrespectively of
their states. Alive stream threads are specified as being in one of the
following states: RUNNING, STARTING, PARTITIONS_REVOKED, and
Currently we, don't cleanup dead threads, but the KIP proposes to change
this:
> Stream threads that are in state DEAD will be removed from the stream threads
> of a Kafka Streams client.
-Matthias
On 9/8/20 2:37 PM, Sophie Blee-Goldman wrote:
> Ah, I forgot about localThreadsMetadata(). In
Ah, I forgot about localThreadsMetadata(). In that. case I agree, there's
no reason
to introduce a new method when we can get both the names and number of all
running threads from this.
I assume that we would update localThreadsMetadata to only return currently
alive threads as part of this KIP
I am not sure if we need a new method? There is already
`localThreadsMetadata()`. What do we gain by adding a new one?
Returning the thread's name (as `Optional`) for both add() and
remove() is fine with me.
-Matthias
On 9/8/20 12:58 PM, Sophie Blee-Goldman wrote:
> Sorry Bruno, I think I
Sorry Bruno, I think I missed the end of your message with the
numberOfAliveStreamThreads()
proposal. I agree, that would be better than the alternatives I listed.
That said:
> They rather suggest that the method returns a list of handles to the
stream threads.
I hadn't thought of that
Thank you again for the feedback Sophie!
As I tried to point out in my previous e-mail, removing a stream thread
from a Kafka Streams client that does not have alive stream threads is
nothing exceptional for the client per se. However, it can become
exceptional within the context of the user.
> it's never a good sign when the discussion moves into the vote thread
Hah, sorry, the gmail consolidation of [VOTE] and [DISCUSS] threads strikes
again.
Thanks for redirecting me Bruno
I suppose it's unfair to expect the callers to keep perfect track of the
current
number of stream threads,
Hi Sophie,
Thanks for your feedback!
I replied inline.
I changed the subject from [VOTE] to [DISCUSS] so that we can follow up
in the discussion thread.
Best,
Bruno
On 03.09.20 21:15, Sophie Blee-Goldman wrote:
Hey, sorry for the late reply, I just have one minor suggestion. Since we
Hi John,
I agree with you except for checking null. I would rather prefer to use
Optional as the return type to both methods.
I changed the subject from [VOTE] to [DISCUSS] so that we can follow up
in the discussion thread.
Best,
Bruno
On 04.09.20 23:12, John Roesler wrote:
Hi Sophie,
Thanks! SGTM.
-Matthias
On 9/3/20 3:17 AM, Bruno Cadonna wrote:
> Hi Matthias,
>
> I replied inline.
>
> Best,
> Bruno
>
> On 02.09.20 22:06, Matthias J. Sax wrote:
>> Thanks for updating the KIP.
>>
>> Why do you propose to return `boolean` from addStreamThread() if the
>> thread could not
Hi Matthias,
I replied inline.
Best,
Bruno
On 02.09.20 22:06, Matthias J. Sax wrote:
Thanks for updating the KIP.
Why do you propose to return `boolean` from addStreamThread() if the
thread could not be started? As an alternative, we could also throw an
exception if the client is not in
Thanks for updating the KIP.
Why do you propose to return `boolean` from addStreamThread() if the
thread could not be started? As an alternative, we could also throw an
exception if the client is not in state RUNNING? -- I guess both are
valid options: just want to see what the pros/cons of each
Hi Bruno,
I read through your updated KIP and it looks good to me. I agree with
adding the metric to keep track of crashed streams in replace of a list of
dead streams.
best,
Wlaker :)
On Tue, Sep 1, 2020 at 1:05 PM Bruno Cadonna wrote:
> Hi John,
>
> your proposal makes sense! I will update
Hi John,
your proposal makes sense! I will update the KIP.
Best,
Bruno
On 01.09.20 17:31, John Roesler wrote:
Hello Bruno,
Thanks for the update! The KIP looks good to me; I only have
a grammatical complaint about the proposed metric name.
"Died" is a verb, the past tense of "to die", but
Hello Bruno,
Thanks for the update! The KIP looks good to me; I only have
a grammatical complaint about the proposed metric name.
"Died" is a verb, the past tense of "to die", but in the
expression,"x stream threads", x should be an adjective. To
be fair, "died" is also the past participle of
Hi,
I updated the KIP with the feedback so far. I removed the API to close
the Kafka Streams client asynchronously, since it should be possible to
avoid the deadlock with the existing method and without a KIP.
Please have a look at the updated KIP and let me know what you think.
Thank you Matthias for the feedback.
Please find my answers inline.
Best,
Bruno
On 26.08.20 19:54, Matthias J. Sax wrote:
Thanks for the KIP Bruno.
While reading it, I had the same questions as raised by John and Walker
(so I won't repeat them).
In addition, I think that adding/removing
Hi Wlaker (pun intended),
Thank you for your feedback! Please find my answers inline.
Best,
Bruno
On 26.08.20 18:46, Walker Carlson wrote:
Hello Burno,
Thanks for the KIP!
Not to pile on, but I also had a couple additional questions. I am not
super familiar with the StreamThread internals
Hi John,
Thank you for your feedback!
Please find my answers inline.
Best,
Bruno
On 26.08.20 17:49, John Roesler wrote:
Hi Bruno,
Thanks for the well motivated and throrough KIP!
It's a good point that the record cache should be re-
distributed over the threads.
Reading your KIP leads me
Thanks for the KIP Bruno.
While reading it, I had the same questions as raised by John and Walker
(so I won't repeat them).
In addition, I think that adding/removing threads should only be allowed
if the client state is RUNNING (but not in any other state, maybe except
ERROR). Furthermore, it
Hello Burno,
Thanks for the KIP!
Not to pile on, but I also had a couple additional questions. I am not
super familiar with the StreamThread internals so please forgive any
misconceptions if these are not relevant questions.
1. In requestClose if a thread does not close properly and deadlocks
Hi Bruno,
Thanks for the well motivated and throrough KIP!
It's a good point that the record cache should be re-
distributed over the threads.
Reading your KIP leads me to a few questions:
1. Start vs. Add
Maybe this is paranoid, but I'm mildly concerned that users
who don't read the docs too
Hi,
I would like to propose the following KIP to start and shut down stream
threads during execution as well as to shut down asynchronously a Kafka
Streams client from an uncaught exception handler.
34 matches
Mail list logo
