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 -0700, Jay Kreps wrote: > 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 asked us to block for that long and we > did, we logged a warning so hopefully you will notice and correct it. > > (2) seems odd. There are many areas where we log an error but we never do > sleep(INFINITY) to draw attention to the problem, right? Changing the > blocking time to infinite is arguably as weird as changing it to 0, no? > > I don't want to prolong the discussion too long but I do think it is a bit > weird. > > I think that likely very few people will call close from within a callback > so it probably isn't a huge issue one way or another. > > -Jay > > On Thu, Mar 26, 2015 at 11:03 AM, Guozhang Wang <wangg...@gmail.com> wrote: > > > 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. > > > > Guozhang > > > > On Thu, Mar 26, 2015 at 6:31 AM, Jiangjie Qin <j...@linkedin.com.invalid> > > wrote: > > > > > 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, > > we > > > just need to document that in this case close() is same as close(0). > > > > > > On the other hand, if we define this as an illegal usage and want users > > to > > > correct it, blocking has merit in some cases. > > > Let¹s say I¹m writing a unit test for exit-on-send-faiure and call > > close() > > > in callback, if we detect the mis-usage of close() and replace it with > > > close(0). User might never know that they were not using the right > > > interface in the callback, because the unit test will pass just with an > > > error log which might not even be printed. So we are indulging user to > > use > > > a wrong interface in this case. > > > > > > My previous assumption was that we don¹t want to allow user to use > > > close()/close(timeout) in callback. That¹s why I preferred blocking. > > > > > > That said, I do not have strong opinion over the options, so I¹m happy > > > with whichever way we decide to go. > > > > > > Thanks. > > > > > > Jiangjie (Becket) Qin > > > > > > On 3/25/15, 9:02 PM, "Neha Narkhede" <n...@confluent.io> wrote: > > > > > > >> > > > >> 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 doing something that will > > > >cause their application to block indefinitely, which can't be ideal > > > >behavior in any case I can imagine. > > > > > > > >My concern is that this might add to this long list > > > ><http://ingest.tips/2015/03/26/what-is-confusing-about-kafka/> of > > > >confusing > > > >Kafka behaviors, so I'd vote to log an appropriate error message and > > exit. > > > > > > > >On Wed, Mar 25, 2015 at 10:04 AM, Jiangjie Qin > > <j...@linkedin.com.invalid > > > > > > > >wrote: > > > > > > > >> 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 user be aware that > > they > > > >> should use close(0) instead of close() in the callback. > > > >> > > > >> 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. > > > >> There are two scenarios: > > > >> 1. User does not expect the program to exit. > > > >> 2. User expect the program to exit. > > > >> > > > >> For scenario 1), blocking will probably delay the discovery of the > > > >> problem. Calling close(0) exposes the problem quicker. In this > > scenario > > > >> producer just encounter a send failure when running normally. > > > >> For scenario 2), blocking will expose the problem quick. Calling > > > >>close(-1) > > > >> might hide the problem. This scenario might include: a) Unit test for > > a > > > >> send failure. b) Message sent during a close() call from a user > > thread. > > > >> > > > >> So as a summary table: > > > >> > > > >> Scenario 1) Scenario 2) > > > >> > > > >> Blocking Delay problem discovery Guaranteed problem > > discovery > > > >> > > > >> Close(-1) Immediate problem discovery Problem might be hidden > > > >> > > > >> > > > >> Personally I prefer blocking because it seems providing more > > guarantees > > > >> and safer. > > > >> > > > >> Thanks. > > > >> > > > >> Jiangjie (Becket) Qin > > > >> > > > >> > > > >> > > > >> On 3/25/15, 9:20 AM, "Jay Kreps" <jay.kr...@gmail.com> wrote: > > > >> > > > >> >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 if they only specified a wait time of say 15 ms just > > > >> >seems > > > >> >weird, right? There is no other error condition where we > > intentionally > > > >> >block forever as far as I know. > > > >> > > > > >> >Sorry to keep going around and around on this minor point I just want > > > >>to > > > >> >clarify that that is what you mean. > > > >> > > > > >> >-Jay > > > >> > > > > >> >On Wed, Mar 25, 2015 at 9:17 AM, Jay Kreps <jay.kr...@gmail.com> > > > wrote: > > > >> > > > > >> >> +1 > > > >> >> > > > >> >> -Jay > > > >> >> > > > >> >> On Tue, Mar 24, 2015 at 2:43 PM, Guozhang Wang <wangg...@gmail.com > > > > > > >> >>wrote: > > > >> >> > > > >> >>> +1. > > > >> >>> > > > >> >>> On Tue, Mar 24, 2015 at 2:15 PM, Jiangjie Qin > > > >> >>><j...@linkedin.com.invalid> > > > >> >>> wrote: > > > >> >>> > > > >> >>> > > > > >> >>> > > > > >> >>> > > > >> >>> > > > >> > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-15+-+Add+a+close+m > > > >> >>>ethod+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 complete all the requests, then join the sender > > thread. > > > >>If > > > >> >>>the > > > >> >>> > sender thread is not able to finish work before timeout, the > > > >>method > > > >> >>> force > > > >> >>> > close the producer by fail all the incomplete requests and join > > > >>the > > > >> >>> sender > > > >> >>> > thread. > > > >> >>> > 2. When timeout = 0, it will be a non-blocking call, just > > > >>initiate > > > >> >>> the > > > >> >>> > force close and DOES NOT join the sender thread. > > > >> >>> > > > > >> >>> > If close(timeout) is called from callback, an error message will > > > >>be > > > >> >>> logged > > > >> >>> > and the producer sender thread will block forever. > > > >> >>> > > > > >> >>> > > > > >> >>> > > > >> >>> > > > >> >>> -- > > > >> >>> -- Guozhang > > > >> >>> > > > >> >> > > > >> >> > > > >> > > > >> > > > > > > > > > > > >-- > > > >Thanks, > > > >Neha > > > > > > > > > > > > -- > > -- Guozhang > >
