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
