Re: [VOTE] KIP-266: Add TimeoutException for KafkaConsumer#position

2018-06-11 Thread Ted Yu
+1 Original message From: Ismael Juma Date: 6/11/18 5:43 PM (GMT-08:00) To: dev Subject: Re: [VOTE] KIP-266: Add TimeoutException for KafkaConsumer#position Sounds good to me. Ismael On Mon, Jun 11, 2018 at 5:38 PM Jason Gustafson wrote: > Hey All, > > I want

Re: [VOTE] KIP-266: Add TimeoutException for KafkaConsumer#position

2018-06-11 Thread Ismael Juma
Sounds good to me. Ismael On Mon, Jun 11, 2018 at 5:38 PM Jason Gustafson wrote: > Hey All, > > I wanted to get to some closure on this issue before the release. I think > the config `default.api.timeout.ms` sounds good to me. Guozhang and I had > actually discussed using this name before we sa

Re: [VOTE] KIP-266: Add TimeoutException for KafkaConsumer#position

2018-06-11 Thread Guozhang Wang
Re: special handling JoinGroup request so that we can use reasonable request.timeout.ms, sounds great to me :) Guozhang On Mon, Jun 11, 2018 at 5:38 PM, Jason Gustafson wrote: > Hey All, > > I wanted to get to some closure on this issue before the release. I think > the config `default.api.tim

Re: [VOTE] KIP-266: Add TimeoutException for KafkaConsumer#position

2018-06-11 Thread Jason Gustafson
Hey All, I wanted to get to some closure on this issue before the release. I think the config `default.api.timeout.ms` sounds good to me. Guozhang and I had actually discussed using this name before we saw Colin's comment. As for the default value, the main reason that the current ` request.timeo

Re: [VOTE] KIP-266: Add TimeoutException for KafkaConsumer#position

2018-06-08 Thread Colin McCabe
On Wed, Jun 6, 2018, at 13:10, Guozhang Wang wrote: > The reason that I'm hesitant to use the term "timeout" is that it's being > over-used for multiple semantics: request RPC timeout, consumer session > heartbeat liveness "timeout", and API blocking timeout. We can argue that > in English both of

Re: [VOTE] KIP-266: Add TimeoutException for KafkaConsumer#position

2018-06-06 Thread Guozhang Wang
The reason that I'm hesitant to use the term "timeout" is that it's being over-used for multiple semantics: request RPC timeout, consumer session heartbeat liveness "timeout", and API blocking timeout. We can argue that in English both of them are indeed called a "timeout" value, but personally I'm

Re: [VOTE] KIP-266: Add TimeoutException for KafkaConsumer#position

2018-06-05 Thread Ted Yu
I see where the 0.5 in your previous response came about. The reason I wrote 'request.timeout.ms + 15000' was that I treat this value in place of the default.block.ms According to your earlier description: bq. request.timeout.ms controls something different: the amount of time we're willing to wa

Re: [VOTE] KIP-266: Add TimeoutException for KafkaConsumer#position

2018-06-05 Thread Colin McCabe
On Tue, Jun 5, 2018, at 16:35, Ted Yu wrote: > bq. could probably come up with a good default > > That's the difficult part. > > bq. max(1000, 0.5 * request.timeout.ms) > > Looking at some existing samples: > In tests/kafkatest/tests/connect/templates/connect-distributed.properties , > we have:

Re: [VOTE] KIP-266: Add TimeoutException for KafkaConsumer#position

2018-06-05 Thread Ted Yu
bq. could probably come up with a good default That's the difficult part. bq. max(1000, 0.5 * request.timeout.ms) Looking at some existing samples: In tests/kafkatest/tests/connect/templates/connect-distributed.properties , we have: request.timeout.ms=3 Isn't the above formula putting an

Re: [VOTE] KIP-266: Add TimeoutException for KafkaConsumer#position

2018-06-05 Thread Colin McCabe
I don't think it can be fixed. The RPC duration is something that you might reasonably want to tune. For example, if it's too low, you might not be able to make progress at all on a heavily loaded server. We could probably come up with a good default, however. rpc.timeout.ms could be set to

Re: [VOTE] KIP-266: Add TimeoutException for KafkaConsumer#position

2018-06-05 Thread Ted Yu
bq. we must make the API timeout longer than the RPC timeout I agree with the above. How about adding a fixed duration on top of request.timeout.ms ? Thanks On Tue, Jun 5, 2018 at 4:18 PM, Colin McCabe wrote: > As Jason noted earlier, request.timeout.ms controls something different: > the amo

Re: [VOTE] KIP-266: Add TimeoutException for KafkaConsumer#position

2018-06-05 Thread Colin McCabe
As Jason noted earlier, request.timeout.ms controls something different: the amount of time we're willing to wait for an RPC to complete. Empirically, RPCs sometimes hang for a long time. If the API timeout is the same as the RPC timeout, we are not robust against this failure condition. The

Re: [VOTE] KIP-266: Add TimeoutException for KafkaConsumer#position

2018-06-05 Thread Ted Yu
bq. we were already doing with request.timeout.ms I would vote for using existing config. Any new config parameter needs to go thru long process of digestion: documentation, etc in order for users to understand and familiarize. The existing config would have lower mismatch of impedance. Cheers

Re: [VOTE] KIP-266: Add TimeoutException for KafkaConsumer#position

2018-06-05 Thread Jason Gustafson
Thanks for the comments. I'm not sure I understand why we want to avoid the term "timeout." Semantically, that's what it is. If we don't want another timeout config, we could avoid it and hard-code a reasonable default or try to wrap the behavior into one of the other timeouts (which is sort of wha

Re: [VOTE] KIP-266: Add TimeoutException for KafkaConsumer#position

2018-06-05 Thread Dhruvil Shah
I agree that using `default.timeout.ms` could cause confusion since we already have other timeout configurations in the consumer. +1 for using `default.block.ms`. Thanks, Dhruvil On Tue, Jun 5, 2018 at 11:48 AM, Bill Bejeck wrote: > Hi Jason, > > At first, I thought the same name between the p

Re: [VOTE] KIP-266: Add TimeoutException for KafkaConsumer#position

2018-06-05 Thread Bill Bejeck
Hi Jason, At first, I thought the same name between the producer and the consumer was ideal. But your comment makes me realize consistent names with different semantics is even more confusing. I'm +1 for not using `max.block.ms`. I like Guozhang's suggestion of ` default.block.ms` for the name.

Re: [VOTE] KIP-266: Add TimeoutException for KafkaConsumer#position

2018-06-05 Thread Guozhang Wang
Hi Jason, Yeah I agree that "max.block.ms" makes people thinking of the producer's config with the same name, but their semantics are different. On the other hand, I'm a bit concerned with the reusing of the term `timeout` as we already have `session.timeout.ms` and `request.timeout.ms` in Consum

Re: [VOTE] KIP-266: Add TimeoutException for KafkaConsumer#position

2018-06-05 Thread Jason Gustafson
Hey All, One more minor follow-up. As I was reviewing the change mentioned above, I felt the name `max.block.ms` was a little bit misleading since it only applies to methods which do not have an explicit timeout. A clearer name given its usage might be `default.timeout.ms`. It is the default timeo

Re: [VOTE] KIP-266: Add TimeoutException for KafkaConsumer#position

2018-05-31 Thread Dong Lin
Thanks for the KIP! I am in favor of the option 1. +1 as well. On Thu, May 31, 2018 at 6:00 PM, Jason Gustafson wrote: > Thanks everyone for the feedback. I've updated the KIP and added > KAFKA-6979. > > -Jason > > On Wed, May 30, 2018 at 3:50 PM, Guozhang Wang wrote: > > > Thanks Jason. I'm i

Re: [VOTE] KIP-266: Add TimeoutException for KafkaConsumer#position

2018-05-31 Thread Jason Gustafson
Thanks everyone for the feedback. I've updated the KIP and added KAFKA-6979. -Jason On Wed, May 30, 2018 at 3:50 PM, Guozhang Wang wrote: > Thanks Jason. I'm in favor of option 1 as well. > > On Wed, May 30, 2018 at 1:37 PM, Bill Bejeck wrote: > > > For what it's worth I'm +1 on Option 1 and t

Re: [VOTE] KIP-266: Add TimeoutException for KafkaConsumer#position

2018-05-30 Thread Guozhang Wang
Thanks Jason. I'm in favor of option 1 as well. On Wed, May 30, 2018 at 1:37 PM, Bill Bejeck wrote: > For what it's worth I'm +1 on Option 1 and the default value for the > timeout. > > In addition to reasons outlined above by Jason, I think it will help to > reason about consumer behavior (with

Re: [VOTE] KIP-266: Add TimeoutException for KafkaConsumer#position

2018-05-30 Thread Bill Bejeck
For what it's worth I'm +1 on Option 1 and the default value for the timeout. In addition to reasons outlined above by Jason, I think it will help to reason about consumer behavior (with respect to blocking) having the configuration and default value aligned with the producer. -Bill On Wed, May

Re: [VOTE] KIP-266: Add TimeoutException for KafkaConsumer#position

2018-05-30 Thread Ismael Juma
Sounds good to me, On Wed, May 30, 2018 at 12:40 PM Jason Gustafson wrote: > Perhaps one minute? That is the default used by the producer. > > -Jason > > On Wed, May 30, 2018 at 9:50 AM, Ismael Juma wrote: > > > Option 1 sounds good to me provided that we can come up with a good > > default. Wh

Re: [VOTE] KIP-266: Add TimeoutException for KafkaConsumer#position

2018-05-30 Thread Jason Gustafson
Perhaps one minute? That is the default used by the producer. -Jason On Wed, May 30, 2018 at 9:50 AM, Ismael Juma wrote: > Option 1 sounds good to me provided that we can come up with a good > default. What would you suggest? > > Ismael > > On Wed, May 30, 2018 at 9:41 AM Jason Gustafson > wro

Re: [VOTE] KIP-266: Add TimeoutException for KafkaConsumer#position

2018-05-30 Thread Ismael Juma
Option 1 sounds good to me provided that we can come up with a good default. What would you suggest? Ismael On Wed, May 30, 2018 at 9:41 AM Jason Gustafson wrote: > Hi Everyone, > > There remains some inconsistency in the timeout behavior of the consumer > APIs which do not accept a timeout. So

Re: [VOTE] KIP-266: Add TimeoutException for KafkaConsumer#position

2018-05-30 Thread Jason Gustafson
Hi Everyone, There remains some inconsistency in the timeout behavior of the consumer APIs which do not accept a timeout. Some of them block forever (e.g. position()) and some of them use request.timeout.ms (e.g. parititonsFor()). I think we'd probably all agree that blocking forever is not useful

Re: [VOTE] KIP-266: Add TimeoutException for KafkaConsumer#position

2018-05-13 Thread Richard Yu
Hi, With 3 binding votes and 6 non-binding, this KIP would be accepted. Thanks for participating. On Thu, May 10, 2018 at 2:35 AM, Edoardo Comar wrote: > +1 (non-binding) > > On 10 May 2018 at 10:29, zhenya Sun wrote: > > > +1 non-binding > > > > > 在 2018年5月10日,下午5:19,Manikumar 写道: > > > > >

Re: [VOTE] KIP-266: Add TimeoutException for KafkaConsumer#position

2018-05-10 Thread Edoardo Comar
+1 (non-binding) On 10 May 2018 at 10:29, zhenya Sun wrote: > +1 non-binding > > > 在 2018年5月10日,下午5:19,Manikumar 写道: > > > > +1 (non-binding). > > Thanks. > > > > On Thu, May 10, 2018 at 2:33 PM, Mickael Maison < > mickael.mai...@gmail.com> > > wrote: > > > >> +1 (non binding) > >> Thanks > >>

Re: [VOTE] KIP-266: Add TimeoutException for KafkaConsumer#position

2018-05-10 Thread zhenya Sun
+1 non-binding > 在 2018年5月10日,下午5:19,Manikumar 写道: > > +1 (non-binding). > Thanks. > > On Thu, May 10, 2018 at 2:33 PM, Mickael Maison > wrote: > >> +1 (non binding) >> Thanks >> >> On Thu, May 10, 2018 at 9:39 AM, Rajini Sivaram >> wrote: >>> Hi Richard, Thanks for the KIP. >>> >>> +1 (bi

Re: [VOTE] KIP-266: Add TimeoutException for KafkaConsumer#position

2018-05-10 Thread Manikumar
+1 (non-binding). Thanks. On Thu, May 10, 2018 at 2:33 PM, Mickael Maison wrote: > +1 (non binding) > Thanks > > On Thu, May 10, 2018 at 9:39 AM, Rajini Sivaram > wrote: > > Hi Richard, Thanks for the KIP. > > > > +1 (binding) > > > > Regards, > > > > Rajini > > > > On Wed, May 9, 2018 at 10:54

Re: [VOTE] KIP-266: Add TimeoutException for KafkaConsumer#position

2018-05-10 Thread Mickael Maison
+1 (non binding) Thanks On Thu, May 10, 2018 at 9:39 AM, Rajini Sivaram wrote: > Hi Richard, Thanks for the KIP. > > +1 (binding) > > Regards, > > Rajini > > On Wed, May 9, 2018 at 10:54 PM, Guozhang Wang wrote: > >> +1 from me, thanks! >> >> >> Guozhang >> >> On Wed, May 9, 2018 at 10:46 AM, Ja

Re: [VOTE] KIP-266: Add TimeoutException for KafkaConsumer#position

2018-05-10 Thread Rajini Sivaram
Hi Richard, Thanks for the KIP. +1 (binding) Regards, Rajini On Wed, May 9, 2018 at 10:54 PM, Guozhang Wang wrote: > +1 from me, thanks! > > > Guozhang > > On Wed, May 9, 2018 at 10:46 AM, Jason Gustafson > wrote: > > > Thanks for the KIP, +1 (binding). > > > > One small correction: the KIP

Re: [VOTE] KIP-266: Add TimeoutException for KafkaConsumer#position

2018-05-09 Thread Guozhang Wang
+1 from me, thanks! Guozhang On Wed, May 9, 2018 at 10:46 AM, Jason Gustafson wrote: > Thanks for the KIP, +1 (binding). > > One small correction: the KIP mentions that close() will be deprecated, but > we do not want to do this because it is needed by the Closeable interface. > We only want t

Re: [VOTE] KIP-266: Add TimeoutException for KafkaConsumer#position

2018-05-09 Thread Jason Gustafson
Thanks for the KIP, +1 (binding). One small correction: the KIP mentions that close() will be deprecated, but we do not want to do this because it is needed by the Closeable interface. We only want to deprecate close(long, TimeUnit) in favor of close(Duration). -Jason On Tue, May 8, 2018 at 12:4

Re: [VOTE] KIP-266: Add TimeoutException for KafkaConsumer#position

2018-05-08 Thread khaireddine Rezgui
+1 2018-05-07 20:35 GMT+01:00 Bill Bejeck : > +1 > > Thanks, > Bill > > On Fri, May 4, 2018 at 7:21 PM, Richard Yu > wrote: > > > Hi all, I would like to bump this thread since discussion in the KIP > > appears to be reaching its conclusion. > > > > > > > > On Thu, Mar 15, 2018 at 3:30 PM, Richa

Re: [VOTE] KIP-266: Add TimeoutException for KafkaConsumer#position

2018-05-07 Thread Bill Bejeck
+1 Thanks, Bill On Fri, May 4, 2018 at 7:21 PM, Richard Yu wrote: > Hi all, I would like to bump this thread since discussion in the KIP > appears to be reaching its conclusion. > > > > On Thu, Mar 15, 2018 at 3:30 PM, Richard Yu > wrote: > > > Hi all, > > > > Since there does not seem to be t

Re: [VOTE] KIP-266: Add TimeoutException for KafkaConsumer#position

2018-05-04 Thread Richard Yu
Hi all, I would like to bump this thread since discussion in the KIP appears to be reaching its conclusion. On Thu, Mar 15, 2018 at 3:30 PM, Richard Yu wrote: > Hi all, > > Since there does not seem to be too much discussion in KIP-266, I will be > starting a voting thread. > Here is the link

[VOTE] KIP-266: Add TimeoutException for KafkaConsumer#position

2018-03-15 Thread Richard Yu
Hi all, Since there does not seem to be too much discussion in KIP-266, I will be starting a voting thread. Here is the link to KIP-266 for reference: https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=75974886 Recently, I have made some updates to the KIP. To reiterate, I have in