Re: [VOTE] KIP-520: Augment Consumer.committed(partition) to allow multiple partitions

2019-09-13 Thread Guozhang Wang
Thanks everyone for your votes. +1 from myself (binding). I'm closing this voting thread now with the following tally: binding +1: 4 (Bill, Matthias, Jason, Guozhang) non-binding +1: 6 (Boyang, Mickael, Kamal, Bruno, Manna, Tom) Thanks, Guozhang On Thu, Sep 12, 2019 at 11:02 AM Jason

Re: [VOTE] KIP-520: Augment Consumer.committed(partition) to allow multiple partitions

2019-09-12 Thread Jason Gustafson
Hi Guozhang, That's a fair point. I think originally `committed` just hit the local cache. For EOS, we changed it to send a request on every call since the updated value came from outside the consumer. Given that, I think it is reasonable to push users toward the batch API. +1 -Jason On Thu,

Re: [VOTE] KIP-520: Augment Consumer.committed(partition) to allow multiple partitions

2019-09-12 Thread Tom Bentley
+1 (non-binding). Thanks! On Thu, Sep 12, 2019 at 9:58 AM M. Manna wrote: > Gushing, > > Good kip. +1 (binding) from me. > > Thanks, > MAnna > > On Thu, 12 Sep 2019 at 09:55, Bruno Cadonna wrote: > > > Guozhang, Thanks for the KIP. > > > > +1 (non-binding) > > > > Best, > > Bruno > > > > On

Re: [VOTE] KIP-520: Augment Consumer.committed(partition) to allow multiple partitions

2019-09-12 Thread M. Manna
Gushing, Good kip. +1 (binding) from me. Thanks, MAnna On Thu, 12 Sep 2019 at 09:55, Bruno Cadonna wrote: > Guozhang, Thanks for the KIP. > > +1 (non-binding) > > Best, > Bruno > > On Wed, Sep 11, 2019 at 9:17 AM Kamal Chandraprakash > wrote: > > > > Thanks for the KIP! > > > > LGTM, +1

Re: [VOTE] KIP-520: Augment Consumer.committed(partition) to allow multiple partitions

2019-09-12 Thread Bruno Cadonna
Guozhang, Thanks for the KIP. +1 (non-binding) Best, Bruno On Wed, Sep 11, 2019 at 9:17 AM Kamal Chandraprakash wrote: > > Thanks for the KIP! > > LGTM, +1 (non-binding). > > On Wed, Sep 11, 2019 at 3:23 AM Matthias J. Sax > wrote: > > > I don't have a strong preference. So I am also fine to

Re: [VOTE] KIP-520: Augment Consumer.committed(partition) to allow multiple partitions

2019-09-11 Thread Kamal Chandraprakash
Thanks for the KIP! LGTM, +1 (non-binding). On Wed, Sep 11, 2019 at 3:23 AM Matthias J. Sax wrote: > I don't have a strong preference. So I am also fine to deprecate the > existing methods. Let's see what Jason thinks. > > Can you update the KIP to reflect the semantics of the return `Map`

Re: [VOTE] KIP-520: Augment Consumer.committed(partition) to allow multiple partitions

2019-09-10 Thread Matthias J. Sax
I don't have a strong preference. So I am also fine to deprecate the existing methods. Let's see what Jason thinks. Can you update the KIP to reflect the semantics of the return `Map` (ie, does only contain entries for partitions with committed offsets, and does not contain `null` values)? +1

Re: [VOTE] KIP-520: Augment Consumer.committed(partition) to allow multiple partitions

2019-09-10 Thread Guozhang Wang
Hi Jason / Matthias, I understand your concerns now. Just to clarify my main motivation on deprecating the old APIs is not only for aesthetics (confess I personally do have preference on succinct APIs), but to urge people to use the batched API for better latency when possible --- as stated in

Re: [VOTE] KIP-520: Augment Consumer.committed(partition) to allow multiple partitions

2019-09-10 Thread Matthias J. Sax
Thanks for the KIP Guozhang. > Another reason is that other functions of KafkaConsumers do not have those > overloaded functions to be consistent I tend to agree with Jason about keeping the existing methods. Your argument does not seem to hold. I just checked the `Consumer` API, and it's mix of

Re: [VOTE] KIP-520: Augment Consumer.committed(partition) to allow multiple partitions

2019-09-10 Thread Jason Gustafson
> I feel it not worth making committed to have both plurals and singulars. Not sure I agree. If we had started with these new APIs from the beginning, that may have been better, but we already have exposed the singular APIs and users are depending on them. Not sure it's worth breaking

Re: [VOTE] KIP-520: Augment Consumer.committed(partition) to allow multiple partitions

2019-09-10 Thread Guozhang Wang
Thanks Jason! On Tue, Sep 10, 2019 at 9:07 AM Jason Gustafson wrote: > Hi Guozhang, > > I think the motivation for the new API makes sense. I've wanted something > like this in the past. That said, do you think there is a substantial > benefit from deprecating the old API? I can still see it

Re: [VOTE] KIP-520: Augment Consumer.committed(partition) to allow multiple partitions

2019-09-10 Thread Jason Gustafson
Hi Guozhang, I think the motivation for the new API makes sense. I've wanted something like this in the past. That said, do you think there is a substantial benefit from deprecating the old API? I can still see it being convenient in some cases and it's no real cost to maintain. Also, just a

Re: [VOTE] KIP-520: Augment Consumer.committed(partition) to allow multiple partitions

2019-09-10 Thread Bill Bejeck
Thanks for the KIP Guozhang. +1 (binding) -Bill On Tue, Sep 10, 2019 at 9:09 AM Mickael Maison wrote: > +1 (non-binding), thanks Guozhang > > On Tue, Sep 10, 2019 at 1:14 AM Boyang Chen > wrote: > > > > Hey Guozhang, > > > > LGTM, +1 (non-binding) > > > > On Mon, Sep 9, 2019 at 5:07 PM

Re: [VOTE] KIP-520: Augment Consumer.committed(partition) to allow multiple partitions

2019-09-10 Thread Mickael Maison
+1 (non-binding), thanks Guozhang On Tue, Sep 10, 2019 at 1:14 AM Boyang Chen wrote: > > Hey Guozhang, > > LGTM, +1 (non-binding) > > On Mon, Sep 9, 2019 at 5:07 PM Guozhang Wang wrote: > > > Hello folks, > > > > I've created a new KIP allowing consumer.committed to take a set of > > partitions

Re: [VOTE] KIP-520: Augment Consumer.committed(partition) to allow multiple partitions

2019-09-09 Thread Boyang Chen
Hey Guozhang, LGTM, +1 (non-binding) On Mon, Sep 9, 2019 at 5:07 PM Guozhang Wang wrote: > Hello folks, > > I've created a new KIP allowing consumer.committed to take a set of > partitions instead of just one partition to allow batching effects of such > requests (the protocol already allows