Hi Mickael, Thanks for the KIP!
Just some minor comments. 1. Java class names are stale, e.g. "CommitOffsetsOptions.java" should be "AlterOffsetsOptions". 2. I'd suggest we change the future structure of "AlterOffsetsResult" to *KafkaFuture<Map<TopicPartition, KafkaFuture<Void>>>* This is because we will have a hierarchy of two-layers of errors since we need to find out the group coordinator first and then issue the commit offset request (see e.g. the ListConsumerGroupOffsetsResult which exclude partitions that have errors, or the DeleteMembersResult as part of KIP-345). If the discover-coordinator returns non-triable error, we would set it on the first layer of the KafkaFuture, and the per-partition error would be set on the second layer of the KafkaFuture. Guozhang On Tue, Aug 13, 2019 at 9:36 AM Colin McCabe <cmcc...@apache.org> wrote: > Hi Mickael, > > Considering that KIP-496, which adds a way of deleting consumer offsets > from AdminClient, looks like it is going to get in, this seems like > functionality we should definitely have. > > For alterConsumerGroupOffsets, is the intention to ignore partitions that > are not specified in the map? If so, we should specify that in the JavaDoc. > > isolationLevel seems like it should be an enum rather than a string. The > existing enum is in org.apache.kafka.common.requests, so we should probably > create a new one which is public in org.apache.kafka.clients.admin. > > best, > Colin > > > On Mon, Mar 25, 2019, at 06:10, Mickael Maison wrote: > > Bumping this thread once again > > > > Ismael, have I answered your questions? > > While this has received a few non-binding +1s, no committers have > > voted yet. If you have concerns or questions, please let me know. > > > > Thanks > > > > On Mon, Feb 11, 2019 at 11:51 AM Mickael Maison > > <mickael.mai...@gmail.com> wrote: > > > > > > Bumping this thread as it's been a couple of weeks. > > > > > > On Tue, Jan 22, 2019 at 2:26 PM Mickael Maison < > mickael.mai...@gmail.com> wrote: > > > > > > > > Thanks Ismael for the feedback. I think your point has 2 parts: > > > > - Having the reset functionality in the AdminClient: > > > > The fact we have a command line tool illustrate that this operation > is > > > > relatively common. I seems valuable to be able to perform this > > > > operation directly via a proper API in addition of the CLI tool. > > > > > > > > - Sending an OffsetCommit directly instead of relying on > KafkaConsumer: > > > > The KafkaConsumer requires a lot of stuff to commit offsets. Its > group > > > > cannot change so you need to start a new Consumer every time, that > > > > creates new connections and overal sends more requests. Also there > are > > > > already a bunch of AdminClient APIs that have logic very close to > > > > what needs to be done to send a commit request, keeping the code > small > > > > and consistent. > > > > > > > > I've updated the KIP with these details and moved the 2nd part to > > > > "Proposed changes" as it's more an implementation detail. > > > > > > > > I hope this answers your question > > > > > > > > On Mon, Jan 21, 2019 at 7:41 PM Ismael Juma <isma...@gmail.com> > wrote: > > > > > > > > > > The KIP doesn't discuss the option of using KafkaConsumer directly > as far > > > > > as I can tell. We have tried to avoid having the same > functionality in > > > > > multiple clients so it would be good to explain why this is > necessary here > > > > > (not saying it isn't). > > > > > > > > > > Ismael > > > > > > > > > > On Mon, Jan 21, 2019, 10:29 AM Mickael Maison < > mickael.mai...@gmail.com > > > > > wrote: > > > > > > > > > > > Thanks Ryanne for the feedback, all suggestions sounded good, > I've > > > > > > updated the KIP accordingly. > > > > > > > > > > > > On Mon, Jan 21, 2019 at 3:43 PM Ryanne Dolan < > ryannedo...@gmail.com> > > > > > > wrote: > > > > > > > > > > > > > > +1 (non-binding) > > > > > > > > > > > > > > But I suggest: > > > > > > > > > > > > > > - drop "get" from getOffset, getTimestamp. > > > > > > > > > > > > > > - add to the motivation section why this is better than > constructing a > > > > > > > KafkaConsumer and using seek(), commit() etc. > > > > > > > > > > > > > > - add some rejected alternatives. > > > > > > > > > > > > > > Ryanne > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Mon, Jan 21, 2019, 7:57 AM Dongjin Lee <dong...@apache.org > wrote: > > > > > > > > > > > > > > > We have +4 non-binding for this vote. Is there any committer > who is > > > > > > > > interested in this issue? > > > > > > > > > > > > > > > > Thanks, > > > > > > > > Dongjin > > > > > > > > > > > > > > > > On Mon, Jan 21, 2019 at 10:33 PM Andrew Schofield < > > > > > > > > andrew_schofi...@live.com> > > > > > > > > wrote: > > > > > > > > > > > > > > > > > +1 (non-binding). Thanks for the KIP. > > > > > > > > > > > > > > > > > > On 21/01/2019, 12:45, "Eno Thereska" < > eno.there...@gmail.com> > > > > > > wrote: > > > > > > > > > > > > > > > > > > +1 (non binding). Thanks. > > > > > > > > > > > > > > > > > > On Mon, Jan 21, 2019 at 12:30 PM Mickael Maison < > > > > > > > > > mickael.mai...@gmail.com> > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > Bumping this thread. Considering this KIP is > relatively straigh > > > > > > > > > > forward, can we get some votes or feedback if you > think it's > > > > > > not? > > > > > > > > > > Thanks > > > > > > > > > > > > > > > > > > > > On Tue, Jan 8, 2019 at 5:40 PM Edoardo Comar < > > > > > > edoco...@gmail.com> > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > +1 (non-binding) > > > > > > > > > > > Thanks Mickael! > > > > > > > > > > > > > > > > > > > > > > On Tue, 8 Jan 2019 at 17:39, Patrik Kleindl < > > > > > > pklei...@gmail.com> > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > +1 (non-binding) > > > > > > > > > > > > Thanks, sounds very helpful > > > > > > > > > > > > Best regards > > > > > > > > > > > > Patrik > > > > > > > > > > > > > > > > > > > > > > > > > Am 08.01.2019 um 18:10 schrieb Mickael Maison < > > > > > > > > > > mickael.mai...@gmail.com > > > > > > > > > > > > >: > > > > > > > > > > > > > > > > > > > > > > > > > > Hi all, > > > > > > > > > > > > > > > > > > > > > > > > > > I'd like to start the vote on KIP-396: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://eur04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcwiki.apache.org%2Fconfluence%2Fpages%2Fviewpage.action%3FpageId%3D97551484&data=02%7C01%7C%7C47c103e1919142c35d7c08d67f9e4c5d%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C636836715187389495&sdata=ihLaSXvB8C%2BK%2F%2BWjVDqKXgUJoRDmwfIi7FvFLRzmFe4%3D&reserved=0 > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > > > > > "When the people fear their government, there is > tyranny; > > > > > > when > > > > > > > > the > > > > > > > > > > > government fears the people, there is liberty." > [Thomas > > > > > > > > Jefferson] > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > > *Dongjin Lee* > > > > > > > > > > > > > > > > *A hitchhiker in the mathematical world.* > > > > > > > > *github: <http://goog_969573159/>github.com/dongjinleekr > > > > > > > > <https://github.com/dongjinleekr>linkedin: > > > > > > kr.linkedin.com/in/dongjinleekr > > > > > > > > <https://kr.linkedin.com/in/dongjinleekr>speakerdeck: > > > > > > > > speakerdeck.com/dongjin > > > > > > > > <https://speakerdeck.com/dongjin>* > > > > > > > > > > > > > > > > > > > > > > > > > -- -- Guozhang
