Hey José, thanks for the KIP! If I understand correctly, the problem at hand is that the follower may need a new HWM, but if there is no new data waiting it can take up to MaxWaitMs. Having the follower report its HWM in the request seems like a good approach.
DA1. It might be more clear if we call the field something like "LastFetchedHighWaterMark" (similar to "LastFetchedEpoch"). "HighWaterMark" is already very prevalent in ReplicaManager, so it might be nice to have a different field name :) DA2. Why use a default of max int64 instead of -1? Will these two values have any practical difference? It seems like both values will have the effect of bypassing the new logic. DA3. Do we always want to return immediately if the leader sees the follower lagging behind the HWM? Would there be any benefit to allow the leader to wait a short time for data to accumulate? Something like an order of magnitude less time than MaxWaitMs. DA4. The motivation section seems focused on KRaft (500ms MaxWaitMs). If I understand correctly, this enhancement will apply to all FETCH, not just in KRaft. Thanks! David A On Fri, Apr 25, 2025 at 1:12 PM Jun Rao <j...@confluent.io.invalid> wrote: > Hi, Jose, > > Thanks for the reply. Sounds good to me. > > Jun > > On Fri, Apr 25, 2025 at 9:55 AM José Armando García Sancio > <jsan...@confluent.io.invalid> wrote: > > > Hi Jun, > > > > > On Thu, Apr 24, 2025 at 1:34 PM Jun Rao <j...@confluent.io.invalid> > > wrote: > > > > Also, in FetchResponse, we use -1 as the default for various offsets. > > > > Should we follow that convention for the new field? > > > > In KRaft, a value of -1 for the HighWatermark field in the FETCH response > > specifically indicates an unknown high-watermark. It appears that the > > replica manager uses the log start offset to indicate an unknown HWM. > > > > This is why I was motivated to use -1 as the unknown value for the new > > field. > > > > Thanks, > > -- > > -José > > > -- -David
