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
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 guess that could be good idea to allow users to log which stream
thread was removed and why. Did you have another use case in mind?
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.
I do not agree that removing a stream thread from a client that does not
have any stream threads is exceptional -- at least not for the Kafka
Streams client. It may be for the caller, but then the caller should
throw an exception.
It is not true that removeStreamThread() will always fail once it starts
to fail, because a stream thread could be added between two calls to
removeStreamThread().
I can imagine that users might also want to keep around a running Kafka
Streams client without running stream threads to be able to start new
stream threads faster, especially when a global table is involved which
would still be updated also without running stream threads.
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();
I am not completely against this. I would just not call it
runningStreamThreads() because that could be misunderstood as returning
handlers to stream threads in state running.
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
