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 26, 2015 at 2:03 PM, Joel Koshy <jjkosh...@gmail.com> wrote: > 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零 say I雋 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靖 want to allow user to use > > > > close()/close(timeout) in callback. That零 why I preferred blocking. > > > > > > > > That said, I do not have strong opinion over the options, so I雋 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 > > > > >
