Hi Christo, Thanks for the update. For "-4 or earliest-local" but tiered storage disabled, I agree it should work as requesting for as "-2 or earliest". For "-5 or latest-tiered" but tiered storage disabled, returning the earliest timestamp doesn't make sense to me. I'm thinking if we can return nothing, like what we did for this "[Note: No offset is returned, if the timestamp greater than recently committed record timestamp is given.]"
WDYT? Thanks. Luke On Mon, Jan 15, 2024 at 6:46 PM Christo Lolov <christolo...@gmail.com> wrote: > Heya Luke, > > Thank for the question! I have expanded in the KIP - in my opinion if -5 > (latest-tiered) is requested when tiered storage is disabled Kafka should > return -2. My reasoning is that if there is no remote storage then we > should be returning an offset which is within the bounds of the log. Let me > know if you disagree! > > Best, > Christo > > On Fri, 12 Jan 2024 at 03:43, Luke Chen <show...@gmail.com> wrote: > > > Hi Christo, > > > > Thanks for the KIP! > > One question: > > > > What will the offset return if tiered storage is disabled? > > For "-4 or earliest-local", it should be the same as "-2 or earliest", > > right? > > For "-5 or latest-tiered", it will be...0? > > > > I think the result should be written in the KIP (or script help text) > > explicitly. > > > > Thanks. > > Luke > > > > On Thu, Jan 11, 2024 at 6:54 PM Divij Vaidya <divijvaidy...@gmail.com> > > wrote: > > > > > Thank you for making the change Christo. It looks good to me. > > > > > > -- > > > Divij Vaidya > > > > > > > > > > > > On Thu, Jan 11, 2024 at 11:19 AM Christo Lolov <christolo...@gmail.com > > > > > wrote: > > > > > > > Thank you Divij! > > > > > > > > I have updated the KIP to explicitly state that the broker will have > a > > > > different behaviour when a timestamp of -5 is requested as part of > > > > ListOffsets. > > > > > > > > Best, > > > > Christo > > > > > > > > On Tue, 2 Jan 2024 at 11:10, Divij Vaidya <divijvaidy...@gmail.com> > > > wrote: > > > > > > > > > Thanks for the KIP Christo. > > > > > > > > > > The shell command that you mentioned calls ListOffsets API > > internally. > > > > > Hence, I believe that we would be making a public interface change > > > (and a > > > > > version bump) to ListOffsetsAPI as well to include -5? If yes, can > > you > > > > > please add that information to the change in public interfaces in > the > > > > KIP. > > > > > > > > > > -- > > > > > Divij Vaidya > > > > > > > > > > > > > > > > > > > > On Tue, Nov 21, 2023 at 2:19 PM Christo Lolov < > > christolo...@gmail.com> > > > > > wrote: > > > > > > > > > > > Heya! > > > > > > > > > > > > Thanks a lot for this. I have updated the KIP to include exposing > > the > > > > > > tiered-offset as well. Let me know whether the Public Interfaces > > > > section > > > > > > needs more explanations regarding the changes needed to the > > > OffsetSpec > > > > or > > > > > > others. > > > > > > > > > > > > Best, > > > > > > Christo > > > > > > > > > > > > On Tue, 21 Nov 2023 at 04:20, Satish Duggana < > > > satish.dugg...@gmail.com > > > > > > > > > > > wrote: > > > > > > > > > > > > > Thanks Christo for starting the discussion on the KIP. > > > > > > > > > > > > > > As mentioned in KAFKA-15857[1], the goal is to add new entries > > for > > > > > > > local-log-start-offset and tierd-offset in OffsetSpec. This > will > > be > > > > > > > used in AdminClient APIs and also to be added as part of > > > > > > > GetOffsetShell. This was also raised by Kamal in the earlier > > email. > > > > > > > > > > > > > > OffsetSpec related changes for these entries also need to be > > > > mentioned > > > > > > > as part of the PublicInterfaces section because these are > exposed > > > to > > > > > > > users as public APIs through Admin#listOffsets() APIs[2, 3]. > > > > > > > > > > > > > > Please update the KIP with the above details. > > > > > > > > > > > > > > 1. https://issues.apache.org/jira/browse/KAFKA-15857 > > > > > > > 2. > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://github.com/apache/kafka/blob/trunk/clients/src/main/java/org/apache/kafka/clients/admin/Admin.java#L1238 > > > > > > > 3. > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://github.com/apache/kafka/blob/trunk/clients/src/main/java/org/apache/kafka/clients/admin/Admin.java#L1226 > > > > > > > > > > > > > > ~Satish. > > > > > > > > > > > > > > On Mon, 20 Nov 2023 at 18:35, Kamal Chandraprakash > > > > > > > <kamal.chandraprak...@gmail.com> wrote: > > > > > > > > > > > > > > > > Hi Christo, > > > > > > > > > > > > > > > > Thanks for the KIP! > > > > > > > > > > > > > > > > Similar to the earliest-local-log offset, can we also expose > > the > > > > > > > > highest-copied-remote-offset via > > > > > > > > GetOffsetShell tool? This will be useful during the debugging > > > > > session. > > > > > > > > > > > > > > > > > > > > > > > > On Mon, Nov 20, 2023 at 5:38 PM Christo Lolov < > > > > > christolo...@gmail.com> > > > > > > > > wrote: > > > > > > > > > > > > > > > > > Hello all! > > > > > > > > > > > > > > > > > > I would like to start a discussion for > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-1005%3A+Add+EarliestLocalOffset+to+GetOffsetShell > > > > > > > > > . > > > > > > > > > > > > > > > > > > A new offset called local log start offset was introduced > as > > > part > > > > > of > > > > > > > > > KIP-405: Kafka Tiered Storage. KIP-1005 aims to expose this > > > > offset > > > > > by > > > > > > > > > changing the AdminClient and in particular the > GetOffsetShell > > > > tool. > > > > > > > > > > > > > > > > > > I am looking forward to your suggestions for improvement! > > > > > > > > > > > > > > > > > > Best, > > > > > > > > > Christo > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
