[CANCEL] [VOTE] KIP-15 add a close with timeout to new producer

2015-03-27 Thread Jiangjie Qin
Hi folks, It looks everyone is on the same page but just had different preference. >From what I can tell, it seems the option with least concerns is logging an error and exiting if when close()/close(timeout) is called from callback. And we will document it crystal clear. Just in order to make pro

Re: [VOTE] KIP-15 add a close with timeout to new producer

2015-03-26 Thread Joel Koshy
Yes it is killing the thread (and effectively the producer as well) but I think that is appropriate since calling close with a timeout from callback is fundamentally a programming error - specifically an illegal argument (in the context of the callback). There are three options as I see it (in dec

Re: [VOTE] KIP-15 add a close with timeout to new producer

2015-03-26 Thread Jay Kreps
Hmm, but won't the impact of throwing the exception just be killing the sender thread? i.e. the user won't see it unless they check the logs, which is the same as logging the error. Is there a problem with just logging an error and then blocking for the amount of time requested? -Jay On Thu, Mar

Re: [VOTE] KIP-15 add a close with timeout to new producer

2015-03-26 Thread Joel Koshy
Talked to Jiangjie offline - actually looking at the code, we could just extend java.lang.Error. Alternately we could throw an IllegalArgumentException and though we catch Exception, we could catch that explicitly and rethrow to cause the sender to just exit. On Thu, Mar 26, 2015 at 01:29:41PM -07

Re: [VOTE] KIP-15 add a close with timeout to new producer

2015-03-26 Thread Jay Kreps
Hey guys, I think there are really two choices: 1. Blocking for the time the user requested 2. Blocking indefinitely irrespective of what the user requested When we were discussing I thought we were talking about (1) but I think really you were proposing (2). (1) seems defensible. After all you

Re: [VOTE] KIP-15 add a close with timeout to new producer

2015-03-26 Thread Guozhang Wang
I was previously preferring logging + exist, but Jiangjie had a point that by doing this we are effectively silently changing the close(>0) call to close(0) call although we log an error to it. The problem is that users may just think the close(>0) call exit normally and do not check the logs. Guo

Re: [VOTE] KIP-15 add a close with timeout to new producer

2015-03-26 Thread Jiangjie Qin
Hi Neha, I totally agree that from program behavior point of view, blocking is not a good idea. I think the ultimate question here is whether we define calling close()/close(timeout) from callback as a legal usage or not. If it is a legal usage, logging a warning and exit makes perfect sense, w

Re: [VOTE] KIP-15 add a close with timeout to new producer

2015-03-25 Thread Neha Narkhede
> > We have agreed that we will have an error log to inform user about this > mis-usage. The options differ in the way how we can force user to take a > look at that error log. Since we have to detect the problem in order to log an appropriate error message, we have a way to tell if the user is d

Re: [VOTE] KIP-15 add a close with timeout to new producer

2015-03-25 Thread Jiangjie Qin
Hi Jay, The reason we keep the blocking behavior if close() or close(timeout) is called from callback is discussed in another thread. I copy/paste the message here: It looks that the problem we want to solve and the purpose we want to achieve is: If user uses close() in callback, we want to let u

Re: [VOTE] KIP-15 add a close with timeout to new producer

2015-03-25 Thread Jay Kreps
Wait, actually, why would the thread block forever? I would understand throwing an error and failing immediately (fail fast) and I would understand logging an error and blocking for the time they specified (since that is what they asked for), but the logging an error and putatively blocking forever

Re: [VOTE] KIP-15 add a close with timeout to new producer

2015-03-25 Thread Jay Kreps
+1 -Jay On Tue, Mar 24, 2015 at 2:43 PM, Guozhang Wang wrote: > +1. > > On Tue, Mar 24, 2015 at 2:15 PM, Jiangjie Qin > wrote: > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-15+-+Add+a+close+method+with+a+timeout+in+the+producer > > > > As a short summary, the new interface

Re: [VOTE] KIP-15 add a close with timeout to new producer

2015-03-24 Thread Guozhang Wang
+1. On Tue, Mar 24, 2015 at 2:15 PM, Jiangjie Qin wrote: > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-15+-+Add+a+close+method+with+a+timeout+in+the+producer > > As a short summary, the new interface will be as following: > Close(Long Timeout, TimeUnit timeUnit) > > 1. When timeou

[VOTE] KIP-15 add a close with timeout to new producer

2015-03-24 Thread Jiangjie Qin
https://cwiki.apache.org/confluence/display/KAFKA/KIP-15+-+Add+a+close+method+with+a+timeout+in+the+producer As a short summary, the new interface will be as following: Close(Long Timeout, TimeUnit timeUnit) 1. When timeout > 0, it will try to wait up to timeout for the sender thread to compl