I did miss updating the KIP on Jose's comment. Have done that now, thanks for the reminder.
For the `kafka-metadata-quorum.sh` tool, it seems that the tool's dependence on the DescribeQuorum API is implicit given the original KIP [1]. I will add a section in this KIP demonstrating that the tool's output should contain the newly added fields as well. The tool itself is tracked under this JIRA [2] Thanks Niket [1] https://cwiki.apache.org/confluence/display/KAFKA/KIP-595%3A+A+Raft+Protocol+for+the+Metadata+Quorum#KIP595%3AARaftProtocolfortheMetadataQuorum-ToolingSupport= [2] https://issues.apache.org/jira/browse/KAFKA-13914 On Tue, May 17, 2022 at 7:31 PM deng ziming <dengziming1...@gmail.com> wrote: > Hello Niket, > > > 1. I find the DescribeQuorumResult still contains an > DescribeQuorumResponseData, which is not allowed as Jose commented, have > you forgot to change it? > > 2. You only add an Handle in AdminClient, can you also add an > `kafka-metadata-quorum.sh` tool to help this? > > > > On May 17, 2022, at 9:50 AM, Niket Goel <ng...@confluent.io.INVALID> > wrote: > > > > Thanks for the call out David. We will populate these fields for the > > Observers as well. I will clarify this in the KIP. > > > > On Mon, May 16, 2022 at 1:50 PM David Arthur <davidart...@apache.org> > wrote: > > > >> Niket, thanks for the KIP! > >> > >> Sorry for the late feedback on this, but I just had a quick question. > The > >> KIP indicates the two new fields will be set for voters only, however > this > >> ReplicaState struct is also used by the Observers in > >> DescribeQuorumResponse. Will we simply fill in -1 for these values, or > do > >> we intend to report the last fetch and caught-up time of the observers > as > >> well? > >> > >> Thanks! > >> David > >> > >> > >> On Mon, May 16, 2022 at 1:46 PM Niket Goel <ng...@confluent.io.invalid> > >> wrote: > >> > >>> Hi all, > >>> > >>> Thank you for the feedback on this. I have started a voting thread for > >>> this KIP here: > >>> https://lists.apache.org/thread/bkb7gsbxpljh5qh014ztffq7bldjrb2x > >>> > >>> Thanks > >>> Niket Goel > >>> > >>> > >>> From: Niket Goel <ng...@confluent.io> > >>> Date: Thursday, May 12, 2022 at 5:25 PM > >>> To: dev@kafka.apache.org <dev@kafka.apache.org> > >>> Subject: Re: [DISCUSS] KIP-836: Addition of Information in > >>> DescribeQuorumResponse about Voter Lag > >>> Appreciate the careful review Jose.! > >>> > >>> Ack on 1 and 2. Will fix. > >>> > >>> For number 3 (and I am using [1] as a reference for this discussion), I > >>> think the correct language to use would be: > >>> > >>> "Whenever a new fetch request > >>> comes in the replica's last caught up time is updated to the time of > >>> this fetch request if it requests an offset greater than or equal to > the > >>> leader's > >>> current end offset" > >>> Does that sound right now? > >>> > >>> Although I think I will go ahead and rewrite the explanation in a way > >> that > >>> is more understandable. Thanks for pointing this out. > >>> > >>> Thanks > >>> > >>> [1] > >>> > >> > https://github.com/apache/kafka/blob/fa59be4e770627cd34cef85986b58ad7f606928d/core/src/main/scala/kafka/cluster/Replica.scala#L97 > >>> > >>> > >>> > >>> On Thu, May 12, 2022 at 3:20 PM José Armando García Sancio > >>> <jsan...@confluent.io.invalid> wrote: > >>> Thanks for the Kafka improvement Niket. > >>> > >>> 1. For the fields `LastFetchTime` and `LastCaughtUpTime`, Kafka tends > >>> to use the suffix "Timestamp" when the value is an absolute wall clock > >>> value. > >>> > >>> 2. The method `result()` for the type `DescribeQuorumResult` returns > >>> the type `DescribeQuorumResponseData`. The types generated from the > >>> RPC JSON schema are internal to Kafka and not exposed to clients. For > >>> the admin client we should use a different type that is explicitly > >>> public. See `org.apache.kafka.client.admin.DescribeTopicsResult` for > >>> an example. > >>> > >>> 3. The proposed section has his sentence "Whenever a new fetch request > >>> comes in the replica's last caught up time is updated to the time of > >>> the fetch request if it requests an offset greater than the leader's > >>> current end offset." Did you mean "previous fetch time" instead of > >>> "last caught up time"? What do you mean by "requests an offset greater > >>> than the leader's current end offset.?" Excluding diverging logs the > >>> follower fetch offset should never be greater than the leader LEO. > >>> > >>> Thanks, > >>> -José > >>> > >>> > >>> -- > >>> - Niket > >>> > >> > > > > > > -- > > - Niket > > -- - Niket
