Hi Colin,

Thanks for raising these 2 valid points. I've updated the KIP accordingly.

On Tue, Aug 13, 2019 at 9:50 PM Guozhang Wang <wangg...@gmail.com> wrote:
>
> 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&amp;data=02%7C01%7C%7C47c103e1919142c35d7c08d67f9e4c5d%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C636836715187389495&amp;sdata=ihLaSXvB8C%2BK%2F%2BWjVDqKXgUJoRDmwfIi7FvFLRzmFe4%3D&amp;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

Reply via email to