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 >>>> > >> > >>>> > >> >>>> > >> >>>> > >>>> >>>
