Hi Matthias, Sure, I could do the DISCONNECTED state.
On Sat, Apr 27, 2019 at 3:16 PM Matthias J. Sax <matth...@confluent.io> wrote: > 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 > >>>>>>>>> > >>>>>>>> > >>>>>>>> > >>>>>> > >>>>> > >>>> > > > >
