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 >
