Hi Ahmed, Are you still interested in this KIP? Should we take it over if you're busy on other things?
Thanks. Luke On Wed, Mar 27, 2024 at 12:26 PM Kamal Chandraprakash < [email protected]> wrote: > Hi Ahmed, > > Thanks for the KIP! It will be a useful addition to know the > last-stable-offset (LSO) > for a partition. Motivation section is not clear to me. Can you document > the > scenarios on how the exposed LSO can be used for txn support/debug? > > -- > Kamal > > On Thu, Mar 21, 2024 at 11:30 PM Justine Olshan > <[email protected]> wrote: > > > Hey Ahmed, > > > > Echoing Andrew's comments to clean up the public interfaces, it was > unclear > > to me if this new flag applies to all the options or just the "latest" > > option. > > Clarifying that and showing a few examples could help. > > > > Thanks, > > Justine > > > > On Thu, Mar 21, 2024 at 9:43 AM Andrew Schofield < > > [email protected]> wrote: > > > > > Hi, > > > Glad to hear you’re better. > > > > > > Re-reading the KIP, I think the changes to the public interfaces are > > > probably only the addition > > > of the new `--isolation` flag on the `kafka-get-offsets.sh` tool. The > > > `KafkaAdminClient.listOffsets` > > > parameters already have what you need I think (OffsetSpec.LATEST and > > > ListOffsetOptions.isolationLevel). > > > > > > If you tidy up the `Public Interfaces` section with the revised syntax > > for > > > the tool, I think the > > > committers will be able to see more easily what the changes are and it > > > would then be ready to vote > > > in my opinion. > > > > > > Thanks, > > > Andrew > > > > > > > On 20 Mar 2024, at 12:56, Ahmed Sobeh <[email protected]> > > > wrote: > > > > > > > > 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 < > > > > [email protected]> 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 <[email protected] > > > > > >> 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 <[email protected]> > 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 < > > > >>>> [email protected]> 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 > > <[email protected] > > > > > > > >>>>> 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 < > > > >>>>>> [email protected]> 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 > > > <[email protected] > > > >>> > > > >>>>>>> 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* > > > >>>>>> [email protected] <[email protected]> > > > >>>>>> 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* > > > >>> [email protected] <[email protected]> > > > >>> 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* > > > > [email protected] <[email protected]> > > > > 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 > > > > > > > > > > > >
