Hi David and Jason, Thank you for looking over the KIP. I've added extra text regarding motivation and included the bump in API version required for the earlier versions issue.
Regarding returning the timestamp type, I was trying to keep the KIP small and consistent with the existing behaviour (right now we return timestamps for OffsetSpec.TimestampSpec and don't provide information on the timestamp type). Also, I think the timestamp type is likely to be something we can expect to be fairly constant (maybe i'm wrong on this) and so fetching it on every call to listOffsets may be too frequent. Thanks Tom On Tue, Apr 27, 2021 at 11:24 PM Jason Gustafson <ja...@confluent.io.invalid> wrote: > Hi Tom, > > I had the same question as David. It sounds like this will require a bump > to the `ListOffsets` API? Otherwise, looking at the code, the sentinel > would be interpreted as a timestamp query and it looks like it would end up > returning the smallest timestamp ;). > > A second thought I had is whether it would be useful to include the > timestamp type in the response. Otherwise it can be difficult to interpret > the timestamp value. > > Best, > Jason > > On Tue, Apr 27, 2021 at 5:18 AM David Jacot <dja...@confluent.io.invalid> > wrote: > > > Hey Thomas, > > > > Thanks for the KIP. I have few comments/questions about it: > > > > 1. It would be great if we could better explain why we need this in > > the motivation. At the moment, it only explains what will be added. > > > > 2. You plan to add the `MAX_TIMESTAMP` constant to tell the > > broker to return the maximum timestamp. How do you plan to > > handle the case where a new admin client would send this to > > an old broker which does not support it? > > > > Best, > > David > > > > On Tue, Apr 20, 2021 at 6:27 PM Thomas Scott <t...@confluent.io.invalid> > > wrote: > > > > > Hey all, > > > > > > Just a quick prod for reviews on this. I'm looking to open a vote on > > > Thursday if there are no objections. > > > > > > Thanks > > > > > > Tom > > > > > > > > > > > > > > Hey all, > > > > > > > > I'd like to open up the discussion on a KIP-734. This adds a new > > > > OffsetSpec to AdminClient.listOffsets so that we can easily determine > > the > > > > offset and timestamp of the message with the largest timestamp on a > > > > partition. > > > > > > > > Please give it a look over and let me know what you think. > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-734%3A+Improve+AdminClient.listOffsets+to+return+timestamp+and+offset+for+the+record+with+the+largest+timestamp > > > > > > > > Thanks > > > > > > > > Tom > > > > > > > > > >
