Thanks for the KIP, Walker!
The KIP looks good to me. I have just a minor comment about the KIP
document.
You talk about SHUTDOWN_CLIENT in the KIP, but never explain that it is
a possible action that can be taken in the Streams uncaught exception
handler. Could you please clarify that?
Best,
Bruno
On 09.12.20 19:04, Walker Carlson wrote:
Thanks for the comments. If there are no further concerns I would like to
call for a vote on KIP-696 to clarify and clean up the Streams State
Machine.
walker
On Wed, Dec 9, 2020 at 8:50 AM John Roesler <vvcep...@apache.org> wrote:
Thanks, Walker!
Your proposal looks good to me.
-John
On Tue, 2020-12-08 at 18:29 -0800, Walker Carlson wrote:
Thanks for the feedback Guozhang!
I clarified some of the points in the Proposed Changes section so
hopefully
it will be more clear what is going on now. I also agree with your
suggestion about the possible call to close() on ERROR so I added this
line.
"Close() called on ERROR will be idempotent and not throw an exception,
but
we will log a warning."
I have linked those tickets and I will leave a comment trying to explain
how these changes will affect their issue.
walker
On Tue, Dec 8, 2020 at 4:57 PM Guozhang Wang <wangg...@gmail.com> wrote:
Hello Walker,
Thanks for the KIP! Overall it looks reasonable to me. Just a few minor
comments for the wiki page itself:
1) Could you clarify the conditions when RUNNING / REBALANCING ->
PENDING_ERROR will happen; and when PENDING_ERROR -> ERROR will happen.
E.g. when I read "Streams will only reach ERROR state in the event of
an
exceptional failure in which the `StreamsUncaughtExceptionHandler`
chose to
either shutdown the application or the client." I thought the first
transition would happen before the handler, and the second transition
would
happen immediately after the handler returns "shutdown client" or
"shutdown
application", until I read the last statement regarding
"SHUTDOWN_CLIENT".
2) A compatibility issue: today it is possible that users would call
Streams APIs like shutdown in the global state transition listener. And
it's common to try shutting down the application automatically when
transiting to ERROR (assuming it was not a terminating state). I think
we
could consider making this call a no-op and log a warning.
3) Could you link the following JIRAs in the "JIRA" field?
https://issues.apache.org/jira/browse/KAFKA-10555
https://issues.apache.org/jira/browse/KAFKA-9638
https://issues.apache.org/jira/browse/KAFKA-6520
And maybe we can also left a comment on those tickets explaining what
would
happen to tackle the issues after this KIP.
Guozhang
On Tue, Dec 8, 2020 at 12:16 PM Walker Carlson <wcarl...@confluent.io>
wrote:
Hello all,
I'd like to propose KIP-696 to clarify the meaning of ERROR state in
the
KafkaStreams Client State Machine. This will update the States to be
consistent with changes in KIP-671 and KIP-663.
Here are the details: https://cwiki.apache.org/confluence/x/lCvZCQ
Thanks,
Walker
--
-- Guozhang