Hi Jason, Thanks for the feedback
1. Yes listOffsets() should be able to retrieve earliest, latest and by-timestamp offsets. I agree, it's better to avoid exposing magic values. I've updated the KIP to use an OffsetSpec object as suggested. 2. Yes let's expose the leader epoch in ListOffsetsResultInfo. 3. Yes mapping it to a friendlier error would be nice but I'm not sure if we can do that because UNKNOWN_MEMBER_ID is also returned if the group is dead. On Tue, Aug 6, 2019 at 6:38 PM Jason Gustafson <ja...@confluent.io> wrote: > > Thanks for the KIP. This makes sense to me. Just a couple small comments: > > 1. Can the listOffsets API be used to get the start and end offsets? In the > consumer, we use separate APIs for this: `beginningOffsets` and > `endOffsets` to avoid the need for sentinels. An alternative would be to > introduce an `OffsetSpec` (or maybe `OffsetQuery`) object to customize the > query. For example: > > public ListOffsetsResult listOffsets(Map<TopicPartition, OffsetSpec> > partitionOffsetSpecs) > > The benefit is that we can avoid sentinel values and we have an extension > point for additional query options in the future. What do you think? > > 2. The ListOffset response includes the leader epoch corresponding to the > offset that was found. This is useful for finer-grained reasoning about the > log. We expose this in the consumer in the OffsetAndTimestamp object which > is returned from `offsetsForTimes`. Does it make sense to add this to > `ListOffsetsResultInfo` as well? > > 3. If the group is still active, the call to reset offsets will fail. > Currently this would result in an UNKNOWN_MEMBER_ID error. I think it would > make sense to map this exception to a friendlier error before raising to > the user. For example, `NonEmptyGroupException` or something like that. > > -Jason > > > > > > On Tue, Aug 6, 2019 at 9:33 AM Mickael Maison <mickael.mai...@gmail.com> > wrote: > > > Hi Colin, > > > > Thank you for taking a look! > > I agree, being able to set consumer group offsets via the AdminClient > > would be really useful, hence I created this KIP. > > > > With the total absence of binding votes, I guessed I needed to make > > some changes. Do you mean you preferred the previous naming > > (commitConsumerGroupOffsets) over "resetConsumerGroupOffsets"? > > > > Thanks > > > > On Mon, Aug 5, 2019 at 8:26 PM Colin McCabe <cmcc...@apache.org> wrote: > > > > > > I think it would be useful to have this in AdminClient. Especially if > > we implement KIP-496: Administrative API to delete consumer offsets. It > > would be odd to have a way to delete consumer offsets in AdminClient, but > > not to create them. What do you think? > > > > > > best, > > > Colin > > > > > > > > > On Sun, Aug 4, 2019, at 09:27, Mickael Maison wrote: > > > > Hi, > > > > > > > > In an attempt to unblock this KIP, I've made some adjustments: > > > > I've renamed the commitConsumerGroupOffsets() methods to > > > > resetConsumerGroupOffsets() to reduce confusion. That should better > > > > highlight the differences with the regular commit() operation from the > > > > Consumer API. I've also added some details to the motivation section. > > > > > > > > So we have +5 non binding votes and 0 binding votes > > > > > > > > On Mon, Mar 25, 2019 at 1:10 PM Mickael Maison < > > mickael.mai...@gmail.com> 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>* > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
