Thanks for updating the KIP. I also had a quick look into your PR. I actually think that the original idea to add a new state DISCONNECTED would provide a better user experience.
Your current proposal does not add a new state, even if it mentions this in the beginning. Compare: https://github.com/apache/kafka/blob/trunk/streams/src/main/java/org/apache/kafka/streams/processor/internals/StreamThread.java#L74-L153 -Matthias On 4/23/19 7:56 PM, Richard Yu wrote: > Oh, so if possible. I thought it would be good if we could finish this KIP > up. > Matthias, or Michael, if you have any further comments, please let me know. > :) > > Otherwise, I might restart the voting thread in a few days. > > Cheers, > Richard > > On Wed, Apr 17, 2019 at 2:30 PM Richard Yu <yohan.richard...@gmail.com> > wrote: > >> Alright, so I made a few changes to the KIP. >> I realized that there might be an easier way to give the user information >> on the connection state of Kafka Streams. >> In implementation, if one wishes to have DISCONNECTED as a state, then one >> would have to factor in proper state transitions. >> The other approach that is now outlined in the KIP. Instead, we could just >> add a method which I think achieves the same effect. >> If any of you thinks there is wrong with this approach, please let me >> know. :) >> >> Cheers, >> Richard >> >> On Wed, Apr 17, 2019 at 11:49 AM Richard Yu <yohan.richard...@gmail.com> >> wrote: >> >>> I just realized something. >>> >>> Hi Matthias, might need your input here. >>> I realized that when implementing this change, as noted in the JIRA, we >>> would need to "check the behaviour of the consumer" since its consumer's >>> connection with broker that we are dealing with. >>> >>> So doesn't that mean we would also be dealing with consumer API changes >>> as well? >>> I don't think consumer has any methods which would give us the state of a >>> connection either. >>> >>> - Richard >>> >>> On Wed, Apr 17, 2019 at 8:43 AM Richard Yu <yohan.richard...@gmail.com> >>> wrote: >>> >>>> Hi Micheal, >>>> >>>> Yeah, those are some points I should've clarified. >>>> No problem. Have got it done. >>>> >>>> >>>> >>>> On Wed, Apr 17, 2019 at 6:42 AM Michael Noll <mich...@confluent.io> >>>> wrote: >>>> >>>>> Richard, >>>>> >>>>> thanks for looking into this! >>>>> >>>>> However, I have some concerns. The KIP you created ( >>>>> >>>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-457%3A+Add+DISCONNECTED+status+to+Kafka+Streams >>>>> ) >>>>> doesn't yet address open questions such as the ones mentioned by >>>>> Matthias: >>>>> >>>>> 1) What is the difference between DEAD and the proposed DISCONNECTED? >>>>> This >>>>> should be defined in the KIP. >>>>> >>>>> 2) Difference between your KIP and the JIRA ( >>>>> https://issues.apache.org/jira/browse/KAFKA-6520): In the JIRA ticket, >>>>> the >>>>> DISCONNECTED state was proposed for the scenario that the KStreams >>>>> application is healthy but the Kafka broker is down. This is different >>>>> to >>>>> what you wrote in the KIP: "When something happens in Kafka Streams, >>>>> such >>>>> as an unexpected crash or error, KafkaStreams#state() will return >>>>> State.DISCONNECTED.", which seems to mean that DISCONNECTED should be >>>>> the >>>>> state when the KStreams app is down. >>>>> >>>>> I wouldn't expect a KIP vote to pass if these basic questions aren't >>>>> properly sorted out in the KIP. >>>>> >>>>> Best, >>>>> Michael >>>>> >>>>> >>>>> >>>>> On Wed, Apr 17, 2019 at 3:35 AM Richard Yu <yohan.richard...@gmail.com> >>>>> wrote: >>>>> >>>>>> Hi all, >>>>>> >>>>>> Considering that this is a simple KIP, I would probably start the >>>>> voting >>>>>> tomorrow. >>>>>> I think it would be good if we could get this in fast. >>>>>> >>>>>> On Tue, Apr 16, 2019 at 3:31 PM Richard Yu < >>>>> yohan.richard...@gmail.com> >>>>>> wrote: >>>>>> >>>>>>> Oh, I probably misunderstood the difference between DISCONNECTED and >>>>>> DEAD. >>>>>>> I will update the KIP accordingly. >>>>>>> Thanks for pointing that out! >>>>>>> >>>>>>> >>>>>>> On Tue, Apr 16, 2019 at 3:13 PM Matthias J. Sax < >>>>> matth...@confluent.io> >>>>>>> wrote: >>>>>>> >>>>>>>> Thanks for the initiative. >>>>>>>> >>>>>>>> In the motivation you mention that you want to use DISCONNECT to >>>>>>>> indicate that the application was killed. >>>>>>>> >>>>>>>> What is the difference to existing state DEAD? >>>>>>>> >>>>>>>> Also, the backing JIRA seems to have a different motivation to add >>>>> a >>>>>>>> DISCONNECT state. There, the Kafka Streams application itself is >>>>>>>> healthy, but it cannot connect to the brokers. It seems reasonable >>>>> to >>>>>>>> add a DISCONNECT for this case though. >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> -Matthias >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> On 4/16/19 9:30 AM, Richard Yu wrote: >>>>>>>>> Hi all, >>>>>>>>> >>>>>>>>> I like to propose a small KIP on adding a new state to >>>>>>>> KafkaStreams#state(). >>>>>>>>> It is very simple, so this should pass relatively quickly! >>>>>>>>> Here is the discussion link: >>>>>>>>> >>>>>>>> >>>>>> >>>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-457%3A+Add+DISCONNECTED+status+to+Kafka+Streams >>>>>>>>> >>>>>>>>> Cheers, >>>>>>>>> Richard >>>>>>>>> >>>>>>>> >>>>>>>> >>>>>> >>>>> >>>> >
signature.asc
Description: OpenPGP digital signature
