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
