Ismael, Thank you, I have fixed the javadoc typo.
Producers use a default close timeout of Long.MAX_VALUE, but with a default request timeout of 30 seconds, close would complete in ~30 seconds. The consumer's 5 minutes request timeout is clearly too long as the default close timeout, but if that is improved in a future release as Ismael suggested, perhaps that could become 30 seconds to be consistent with producer request timeout. And then we could make the consumer's default close timeout use request timeout as the upper bound. So shall we use hard-coded 30 second timeout for now, but change to use request timeout when the default request timeout is reduced? On Fri, Jan 6, 2017 at 10:22 AM, Ismael Juma <ism...@juma.me.uk> wrote: > Hi, > > I am definitely in favour of aiming for good defaults. One concern I have > is that `30 seconds` is a hardcoded value and not based on any config. It > is true that users can call the `close` method that takes an explicit > timeout if necessary, but it would be nice if the no-args `close` would use > a timeout inferred from an existing config. From that perspective, I liked > Rajini's idea to rely on request timeout. The issue is that the request > timeout is a bit too large by default (5 minutes) due to some > implementation details (it would be great to improve this in a future > release). > > Rajini, a typo in the the javadoc for the consumer close method in the KIP: > it says "force close the producer". > > Ismael > > On Fri, Jan 6, 2017 at 2:42 AM, Jason Gustafson <ja...@confluent.io> > wrote: > > > Hey Gwen, > > > > I'm not super strong on this, but I think the case for a longer timeout > as > > the default behavior is weaker for the consumer. For the producer, it > means > > we might lose messages that the application tried to send. For the > > consumer, it means we might lose offset commits, which means duplicates > > later on. But if we're not attempting coordinator rediscovery on > connection > > failures or retrying offset commits anyway, does the extra time help that > > much? I'm not sure, but I'd rather have a reasonable bound on the > shutdown > > time as the default behavior and let users who want to wait longer > provide > > their own timeout. I've seen a few too many cases in the mail lists where > > users complain about services taking too long to shutdown. The other > issue, > > as Jay pointed out, is that the current behavior of close() has an > > effectively small timeout, so this changes the behavior of existing code, > > which seems best to avoid. > > > > -Jason > > > > On Thu, Jan 5, 2017 at 3:09 PM, Gwen Shapira <g...@confluent.io> wrote: > > > > > I hate going back and forth on this, but KafkaProducer.close() (with > > > no timeout) is equivalent to close(Long.MAX_VALUE, > > > TimeUnit.MILLISECONDS), while the KafkaConsumer.close() is equivalent > > > to close(30*1000,TimeUnit.MILLISECONDS). > > > > > > Isn't this kind of inconsistency best to avoid? > > > > > > On Thu, Jan 5, 2017 at 2:08 PM, Rajini Sivaram < > rajinisiva...@gmail.com> > > > wrote: > > > > Thank you, Ismael. I have sent another one. Hopefully that will > appear > > in > > > > its own thread. > > > > > > > > Rajini > > > > > > > > On Thu, Jan 5, 2017 at 9:30 PM, Ismael Juma <ism...@juma.me.uk> > wrote: > > > > > > > >> Thanks Rajini. This seems to be happening a lot lately: Gmail is > > showing > > > >> the vote message in the discuss thread. > > > >> > > > >> Ismael > > > >> > > > >> On Thu, Jan 5, 2017 at 9:23 PM, Rajini Sivaram < > > rajinisiva...@gmail.com > > > > > > > >> wrote: > > > >> > > > >> > Hi all, > > > >> > > > > >> > I would like to start the voting process for *KIP-102 - Add close > > with > > > >> > timeout for consumers:* > > > >> > > > > >> > https://cwiki.apache.org/confluence/display/KAFKA/KIP- > > > >> > 102+-+Add+close+with+timeout+for+consumers > > > >> > > > > >> > > > > >> > This KIP adds a new close method with a timeout for consumers > > similar > > > to > > > >> > the close method in the producer. As described in the discussion > > > thread > > > >> > <http://mail-archives.apache.org/mod_mbox/kafka-dev/201612. > > > mbox/%3cCAG_+ > > > >> > n9us5ohthwmyai9pz4s2j62fmils2ufj8oie9jpmyaf...@mail.gmail.com > %3e>, > > > >> > the changes are only in the close code path and hence the impact > is > > > not > > > >> too > > > >> > big. The existing close() method without a timeout will use a > > default > > > >> > timeout of 30 seconds. > > > >> > > > > >> > Thank you... > > > >> > > > > >> > Regards, > > > >> > > > > >> > Rajini > > > >> > > > > >> > > > > > > > > > > > > -- > > > Gwen Shapira > > > Product Manager | Confluent > > > 650.450.2760 | @gwenshap > > > Follow us: Twitter | blog > > > > > >
