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