Re: [DISCUSS] KIP-671: Shutdown Streams Application when appropriate exception is thrown

2020-09-29 Thread Guozhang Wang
Thanks for the updates Walker. They all lgtm. On Tue, Sep 29, 2020 at 8:33 AM Walker Carlson wrote: > Thank you for the feedback Guozhang and Bruno. See the responses below. > > I have updated the kip accordingly > > Thanks, > Walker > > On Tue, Sep 29, 2020 at 1:59 AM Bruno Cadonna wrote: > >

Re: [DISCUSS] KIP-671: Shutdown Streams Application when appropriate exception is thrown

2020-09-29 Thread Walker Carlson
Thank you for the feedback Guozhang and Bruno. See the responses below. I have updated the kip accordingly Thanks, Walker On Tue, Sep 29, 2020 at 1:59 AM Bruno Cadonna wrote: > Hi Walker, > > Thanks for updating the KIP! > > 1. I would add response REPLACE_STREAM_THREAD to the > StreamsUncaugh

Re: [DISCUSS] KIP-671: Shutdown Streams Application when appropriate exception is thrown

2020-09-29 Thread Bruno Cadonna
Hi Walker, Thanks for updating the KIP! 1. I would add response REPLACE_STREAM_THREAD to the StreamsUncaughtExceptionHandlerResponse enum to start a new stream thread that replaces the failed one. I suspect you did not add it because it depends on KIP-663. A dependency to another unfinished K

Re: [DISCUSS] KIP-671: Shutdown Streams Application when appropriate exception is thrown

2020-09-29 Thread Bruno Cadonna
Hi John, I totally agree with you and Walker. I also think that we should leave this as a problem for the future and that we should document this limitation. Best, Bruno On 24.09.20 16:51, John Roesler wrote: Hello all, Thanks for bringing this up, Bruno. It’s a really good point that a d

Re: [DISCUSS] KIP-671: Shutdown Streams Application when appropriate exception is thrown

2020-09-28 Thread Guozhang Wang
Hello Walker, Thanks for the updated KIP proposal. A few more comments below: 1. "The RocksDB metrics recording thread is not shutdown." Why it should not be shut down in either client or application shutdown cases? 2. Should we deprecate the existing overloaded function with the java UncaughtEx

Re: [DISCUSS] KIP-671: Shutdown Streams Application when appropriate exception is thrown

2020-09-28 Thread Walker Carlson
I think that Guozhang and Matthias make good points. https://cwiki.apache.org/confluence/display/KAFKA/KIP-671%3A+Introduce+Kafka+Streams+Specific+Uncaught+Exception+Handler I have updated the kip to include a StreamsUncaughtExceptionHandler On Sun, Sep 27, 2020 at 7:28 PM Guozhang Wang wrote

Re: [DISCUSS] KIP-671: Shutdown Streams Application when appropriate exception is thrown

2020-09-27 Thread Guozhang Wang
I think single-threaded clients may be common in practice, and what Matthias raised is a valid concern. We had a related discussion in KIP-663, that maybe we can tweak the `UncaughtExceptionExceptionHandler` a bit such that instead of just registered users' function into the individual threads, we

Re: [DISCUSS] KIP-671: Shutdown Streams Application when appropriate exception is thrown

2020-09-25 Thread Matthias J. Sax
I am wondering about the usage pattern of this new method. As already discussed, the method only works if there is at least one running thread... Do we have any sense how many apps actually run multi-threaded vs single-threaded? It seems that the feature might be quite limited without having a han

Re: [DISCUSS] KIP-671: Shutdown Streams Application when appropriate exception is thrown

2020-09-24 Thread John Roesler
Hello all, Thanks for bringing this up, Bruno. It’s a really good point that a disconnected node would miss the signal and then resurrect a single-node “zombie cluster” when it reconnects. Offhand, I can’t think of a simple and reliable way to distinguish this case from one in which an operato

Re: [DISCUSS] KIP-671: Shutdown Streams Application when appropriate exception is thrown

2020-09-23 Thread Walker Carlson
Bruno, I think that we can't guarantee that the message will get propagated perfectly in every case of, say network partitioning, though it will work for many cases. So I would say it's best effort and I will mention it in the kip. As for when to use it I think we can discuss if this will be suff

Re: [DISCUSS] KIP-671: Shutdown Streams Application when appropriate exception is thrown

2020-09-22 Thread Bruno Cadonna
Walker, I am sorry, but I still have a comment on the KIP although you have already started voting. What happens when a consumer of the group skips the rebalancing that propagates the shutdown request? Do you give a guarantee that all Kafka Streams clients are shutdown or is it best effort?

Re: [DISCUSS] KIP-671: Shutdown Streams Application when appropriate exception is thrown

2020-09-21 Thread Walker Carlson
The error code right now is the assignor error, 2 is coded for shutdown but it could be expanded to encode the causes or for other errors that need to be communicated. For example we can add error code 3 to close the thread but leave the client in an error state if we choose to do so in the future.

Re: [DISCUSS] KIP-671: Shutdown Streams Application when appropriate exception is thrown

2020-09-21 Thread Boyang Chen
Thanks for the KIP Walker. In the KIP we mentioned "In order to communicate the shutdown request from one client to the others we propose to update the SubcriptionInfoData to include a short field which will encode an error code.", is there a dedicated error code that we should define here, or it

Re: [DISCUSS] KIP-671: Shutdown Streams Application when appropriate exception is thrown

2020-09-21 Thread Walker Carlson
I am changing the name to "Add method to Shutdown entire Streams Application" since we are no longer using an Exception, it seems more appropriate. Also it looks like the discussion is pretty much finished so I will be calling it to a vote. thanks, Walker On Sat, Sep 19, 2020 at 7:49 PM Guozhang

Re: [DISCUSS] KIP-671: Shutdown Streams Application when appropriate exception is thrown

2020-09-19 Thread Guozhang Wang
Sounds good to me. I also feel that this call should be non-blocking but I guess I was confused from the discussion thread that the API is designed in a blocking fashion which contradicts with my perspective and hence I asked for clarification :) Guozhang On Wed, Sep 16, 2020 at 12:32 PM Walker

Re: [DISCUSS] KIP-671: Shutdown Streams Application when appropriate exception is thrown

2020-09-16 Thread Walker Carlson
Hello Guozhang, As for the logging I plan on having three logs. First, the client log that it is requesting an application shutdown, second, the leader log processId of the invoker, third, then the StreamRebalanceListener it logs that it is closing because of an `stream.appShutdown`. Hopefully thi

Re: [DISCUSS] KIP-671: Shutdown Streams Application when appropriate exception is thrown

2020-09-16 Thread Guozhang Wang
Hello Walker, Thanks for the updated KIP. Previously I'm also a bit hesitant on the newly added public exception to communicate user-requested whole app shutdown, but the reason I did not bring this up is that I feel there's still a need from operational aspects that we can differentiate the scena

Re: [DISCUSS] KIP-671: Shutdown Streams Application when appropriate exception is thrown

2020-09-16 Thread Walker Carlson
Hello all again, I have updated the kip to no longer use an exception and instead add a method to the KafkaStreams class, this seems to satisfy everyone's concerns about how and when the functionality will be invoked. There is still a question over the name. We must decide between "shutdownApplic

Re: [DISCUSS] KIP-671: Shutdown Streams Application when appropriate exception is thrown

2020-09-16 Thread Walker Carlson
Hello Guozhang and Bruno, Thanks for the feedback. I will respond in two parts but I would like to clarify that I am not tied down to any of these names, but since we are still deciding if we want to have an exception or not I would rather not get tripped up on choosing a name just yet. Guozhang

Re: [DISCUSS] KIP-671: Shutdown Streams Application when appropriate exception is thrown

2020-09-16 Thread Bruno Cadonna
Hi Walker, Thank you for the KIP! I like the motivation of the KIP and the method to request a shutdown of all Kafka Streams clients of Kafka Streams application. I think we really need such functionality to react on errors. However, I am not convinced that throwing an exception to shutdown a

Re: [DISCUSS] KIP-671: Shutdown Streams Application when appropriate exception is thrown

2020-09-15 Thread Guozhang Wang
Hello Walker, Thanks for proposing the KIP! I have a couple more comments: 1. ShutdownRequestedException: my understanding is that this exception is only used if the application-shutdown was initiated by by the user triggered "shutdownApplication()", otherwise e.g. if it is due to source topic no

Re: [DISCUSS] KIP-671: Shutdown Streams Application when appropriate exception is thrown

2020-09-14 Thread Walker Carlson
Hello Matthias and Sophie, You both make good points. I will respond to the separately below. Matthias: That is a fair point. KIP-662 , which is accepted, will make it so

Re: [DISCUSS] KIP-671: Shutdown Streams Application when appropriate exception is thrown

2020-09-11 Thread Sophie Blee-Goldman
Hey Walker, The proposal makes sense to me, but while reading up on those old tickets I started wondering if we should give users two options: one that would shut down the entire application, as described in the current KIP, and another that would only shut down an individual instance. I think th

Re: [DISCUSS] KIP-671: Shutdown Streams Application when appropriate exception is thrown

2020-09-11 Thread Matthias J. Sax
Thanks for the KIP. It seem that the new exception would need to be thrown by user code? However, in the motivation you mention the scenario of a missing source topic that a user cannot detect, but KafkaStreams runtime would be responsible to handle. How do both things go together? -Matthias O

[DISCUSS] KIP-671: Shutdown Streams Application when appropriate exception is thrown

2020-09-11 Thread Walker Carlson
Hello all, I have created KIP-671 to give the option to shutdown a streams application in response to an error. https://cwiki.apache.org/confluence/display/KAFKA/KIP-671%3A+Shutdown+Streams+Application+when+appropriate+exception+is+thrown This is because of the Jira ticket