Hi Andrew, Apologies for disappearing, I had to undergo knee surgery, all good now!
I adjusted the KIP as you suggested, do you think it's ready for the voting stage? On Wed, Mar 6, 2024 at 4:02 PM Andrew Schofield < andrew_schofield_j...@outlook.com> wrote: > Hi Ahmed, > Looks good to me with one remaining comment. > > You’ve used: > > bin/kafka-get-offsets.sh … --time -1 --isolation -committed > bin/kafka-get-offsets.sh … --time latest --isolation -committed > bin/kafka-get-offsets.sh … --time -1 --isolation -uncommitted > bin/kafka-get-offsets.sh … --time latest --isolation -uncommitted > > I find the flags starting with a single - to be a bit unusual and doesn’t > match the --time options such as “latest”. I suggest: > > bin/kafka-get-offsets.sh … --time -1 --isolation committed > bin/kafka-get-offsets.sh … --time latest --isolation committed > bin/kafka-get-offsets.sh … --time -1 --isolation uncommitted > bin/kafka-get-offsets.sh … --time latest --isolation uncommitted > > Or even: > > bin/kafka-get-offsets.sh … --time -1 --isolation read-committed > bin/kafka-get-offsets.sh … --time latest --isolation read-committed > bin/kafka-get-offsets.sh … --time -1 --isolation read-uncommitted > bin/kafka-get-offsets.sh … --time latest --isolation read-uncommitted > > Apart from that nit, looks good to me. > > Thanks, > Andrew > > > > On 5 Mar 2024, at 16:35, Ahmed Sobeh <ahmed.so...@aiven.io.INVALID> > wrote: > > > > I adjusted the KIP according to what we agreed on, let me know if you > have > > any comments! > > > > Best, > > Ahmed > > > > On Thu, Feb 29, 2024 at 1:44 AM Luke Chen <show...@gmail.com> wrote: > > > >> Hi Ahmed, > >> > >> Thanks for the KIP! > >> > >> Comments: > >> 1. If we all agree with the suggestion from Andrew, you could update the > >> KIP. > >> > >> Otherwise, LGTM! > >> > >> > >> Thanks. > >> Luke > >> > >> On Thu, Feb 29, 2024 at 1:32 AM Andrew Schofield < > >> andrew_schofield_j...@outlook.com> wrote: > >> > >>> Hi Ahmed, > >>> Could do. Personally, I find the existing “--time -1” totally horrid > >>> anyway, which was why > >>> I suggested an alternative. I think your suggestion of a flag for > >>> isolation level is much > >>> better than -6. > >>> > >>> Maybe I should put in a KIP which adds: > >>> --latest (as a synonym for --time -1) > >>> --earliest (as a synonym for --time -2) > >>> --max-timestamp (as a synonym for --time -3) > >>> > >>> That’s really what I would prefer. If the user has a timestamp, use > >>> `--time`. If they want a > >>> specific special offset, use a separate flag. > >>> > >>> Thanks, > >>> Andrew > >>> > >>>> On 28 Feb 2024, at 09:22, Ahmed Sobeh <ahmed.so...@aiven.io.INVALID> > >>> wrote: > >>>> > >>>> Hi Andrew, > >>>> > >>>> Thanks for the hint! That sounds reasonable, do you think adding a > >>>> conditional argument, something like "--time -1 --isolation > -committed" > >>> and > >>>> "--time -1 --isolation -uncommitted" would make sense to keep the > >>>> consistency of getting the offset by time? or do you think having a > >>> special > >>>> argument for this case is better? > >>>> > >>>> On Tue, Feb 27, 2024 at 2:19 PM Andrew Schofield < > >>>> andrew_schofield_j...@outlook.com> wrote: > >>>> > >>>>> Hi Ahmed, > >>>>> Thanks for the KIP. It looks like a useful addition. > >>>>> > >>>>> The use of negative timestamps, and in particular letting the user > use > >>>>> `--time -1` or the equivalent `--time latest` > >>>>> is a bit peculiar in this tool already. The negative timestamps come > >>> from > >>>>> org.apache.kafka.common.requests.ListOffsetsRequest, > >>>>> but you’re not actually adding another value to that. As a result, I > >>>>> really wouldn’t recommend using -6 for the new > >>>>> flag. LSO is really a variant of -1 with read_committed isolation > >> level. > >>>>> > >>>>> I think that perhaps it would be better to add `--last-stable` as an > >>>>> alternative to `--time`. Then you’ll get the LSO with > >>>>> cleaner syntax. > >>>>> > >>>>> Thanks, > >>>>> Andrew > >>>>> > >>>>> > >>>>>> On 27 Feb 2024, at 10:12, Ahmed Sobeh <ahmed.so...@aiven.io.INVALID > > > >>>>> wrote: > >>>>>> > >>>>>> Hi all, > >>>>>> I would like to start a discussion on KIP-1021, which would enable > >>>>> getting > >>>>>> LSO in kafka-get-offsets.sh: > >>>>>> > >>>>> > >>> > >> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-1021%3A+Allow+to+get+last+stable+offset+%28LSO%29+in+kafka-get-offsets.sh > >>>>>> > >>>>>> Best, > >>>>>> Ahmed > >>>>> > >>>>> > >>>> > >>>> -- > >>>> [image: Aiven] <https://www.aiven.io/> > >>>> *Ahmed Sobeh* > >>>> Engineering Manager OSPO, *Aiven* > >>>> ahmed.so...@aiven.io <i...@aiven.io> > >>>> aiven.io <https://www.aiven.io/> | < > >>> https://www.facebook.com/aivencloud> > >>>> <https://www.linkedin.com/company/aiven/> < > >>> https://twitter.com/aiven_io> > >>>> *Aiven Deutschland GmbH* > >>>> Immanuelkirchstraße 26, 10405 Berlin > >>>> Geschäftsführer: Oskari Saarenmaa & Hannu Valtonen > >>>> Amtsgericht Charlottenburg, HRB 209739 B > >>> > >>> > >>> > >> > > > > > > -- > > [image: Aiven] <https://www.aiven.io/> > > *Ahmed Sobeh* > > Engineering Manager OSPO, *Aiven* > > ahmed.so...@aiven.io <i...@aiven.io> > > aiven.io <https://www.aiven.io/> | < > https://www.facebook.com/aivencloud> > > <https://www.linkedin.com/company/aiven/> < > https://twitter.com/aiven_io> > > *Aiven Deutschland GmbH* > > Immanuelkirchstraße 26, 10405 Berlin > > Geschäftsführer: Oskari Saarenmaa & Hannu Valtonen > > Amtsgericht Charlottenburg, HRB 209739 B > > > -- [image: Aiven] <https://www.aiven.io> *Ahmed Sobeh* Engineering Manager OSPO, *Aiven* ahmed.so...@aiven.io <i...@aiven.io> aiven.io <https://www.aiven.io> | <https://www.facebook.com/aivencloud> <https://www.linkedin.com/company/aiven/> <https://twitter.com/aiven_io> *Aiven Deutschland GmbH* Immanuelkirchstraße 26, 10405 Berlin Geschäftsführer: Oskari Saarenmaa & Hannu Valtonen Amtsgericht Charlottenburg, HRB 209739 B
