kafka-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jason Gustafson <ja...@confluent.io>
Subject Re: [VOTE] KIP-266: Add TimeoutException for KafkaConsumer#position
Date Tue, 05 Jun 2018 22:17:41 GMT
Thanks for the comments. I'm not sure I understand why we want to avoid the
term "timeout." Semantically, that's what it is. If we don't want another
timeout config, we could avoid it and hard-code a reasonable default or try
to wrap the behavior into one of the other timeouts (which is sort of what
we were already doing with request.timeout.ms). But I'm not too sure
calling it something else addresses the problem.

-Jason

On Tue, Jun 5, 2018 at 2:59 PM, Dhruvil Shah <dhruvil@confluent.io> wrote:

> I agree that using `default.timeout.ms` could cause confusion since we
> already have other timeout configurations in the consumer.
>
> +1 for using `default.block.ms`.
>
> Thanks,
> Dhruvil
>
> On Tue, Jun 5, 2018 at 11:48 AM, Bill Bejeck <bbejeck@gmail.com> wrote:
>
> > Hi Jason,
> >
> > At first, I thought the same name between the producer and the consumer
> was
> > ideal.
> >
> > But your comment makes me realize consistent names with different
> semantics
> > is even more confusing.
> >
> > I'm +1 for not using `max.block.ms`.  I like Guozhang's suggestion of `
> > default.block.ms` for the name.
> >
> > Thanks,
> > Bill
> >
> > On Tue, Jun 5, 2018 at 1:33 PM, Guozhang Wang <wangguoz@gmail.com>
> wrote:
> >
> > > Hi Jason,
> > >
> > > Yeah I agree that "max.block.ms" makes people thinking of the
> producer's
> > > config with the same name, but their semantics are different.
> > >
> > > On the other hand, I'm a bit concerned with the reusing of the term
> > > `timeout` as we already have `session.timeout.ms` and `
> > request.timeout.ms`
> > > in ConsumerConfig.. How about using the name `default.api.block.ms` or
> > > simply `default.block.ms`?
> > >
> > >
> > >
> > > Guozhang
> > >
> > >
> > > On Tue, Jun 5, 2018 at 8:57 AM, Jason Gustafson <jason@confluent.io>
> > > wrote:
> > >
> > > > Hey All,
> > > >
> > > > One more minor follow-up. As I was reviewing the change mentioned
> > above,
> > > I
> > > > felt the name `max.block.ms` was a little bit misleading since it
> only
> > > > applies to methods which do not have an explicit timeout. A clearer
> > name
> > > > given its usage might be `default.timeout.ms`. It is the default
> > timeout
> > > > for any blocking API which does not have a timeout. I'm leaning
> toward
> > > > using this name since the current one seems likely to cause
> confusion.
> > > Any
> > > > thoughts?
> > > >
> > > > Thanks,
> > > > Jason
> > > >
> > > >
> > > > On Thu, May 31, 2018 at 6:09 PM, Dong Lin <lindong28@gmail.com>
> wrote:
> > > >
> > > > > Thanks for the KIP! I am in favor of the option 1.
> > > > >
> > > > > +1 as well.
> > > > >
> > > > > On Thu, May 31, 2018 at 6:00 PM, Jason Gustafson <
> jason@confluent.io
> > >
> > > > > wrote:
> > > > >
> > > > > > Thanks everyone for the feedback. I've updated the KIP and added
> > > > > > KAFKA-6979.
> > > > > >
> > > > > > -Jason
> > > > > >
> > > > > > On Wed, May 30, 2018 at 3:50 PM, Guozhang Wang <
> wangguoz@gmail.com
> > >
> > > > > wrote:
> > > > > >
> > > > > > > Thanks Jason. I'm in favor of option 1 as well.
> > > > > > >
> > > > > > > On Wed, May 30, 2018 at 1:37 PM, Bill Bejeck <
> bbejeck@gmail.com>
> > > > > wrote:
> > > > > > >
> > > > > > > > For what it's worth I'm +1 on Option 1 and the default
value
> > for
> > > > the
> > > > > > > > timeout.
> > > > > > > >
> > > > > > > > In addition to reasons outlined above by Jason, I
think it
> will
> > > > help
> > > > > to
> > > > > > > > reason about consumer behavior (with respect to blocking)
> > having
> > > > the
> > > > > > > > configuration and default value aligned with the producer.
> > > > > > > >
> > > > > > > > -Bill
> > > > > > > >
> > > > > > > > On Wed, May 30, 2018 at 3:43 PM, Ismael Juma <
> > ismael@juma.me.uk>
> > > > > > wrote:
> > > > > > > >
> > > > > > > > > Sounds good to me,
> > > > > > > > >
> > > > > > > > > On Wed, May 30, 2018 at 12:40 PM Jason Gustafson
<
> > > > > jason@confluent.io
> > > > > > >
> > > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > > Perhaps one minute? That is the default
used by the
> > producer.
> > > > > > > > > >
> > > > > > > > > > -Jason
> > > > > > > > > >
> > > > > > > > > > On Wed, May 30, 2018 at 9:50 AM, Ismael
Juma <
> > > > ismael@juma.me.uk>
> > > > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > > > Option 1 sounds good to me provided
that we can come up
> > > with
> > > > a
> > > > > > good
> > > > > > > > > > > default. What would you suggest?
> > > > > > > > > > >
> > > > > > > > > > > Ismael
> > > > > > > > > > >
> > > > > > > > > > > On Wed, May 30, 2018 at 9:41 AM Jason
Gustafson <
> > > > > > > jason@confluent.io>
> > > > > > > > > > > wrote:
> > > > > > > > > > >
> > > > > > > > > > > > Hi Everyone,
> > > > > > > > > > > >
> > > > > > > > > > > > There remains some inconsistency
in the timeout
> > behavior
> > > of
> > > > > the
> > > > > > > > > > consumer
> > > > > > > > > > > > APIs which do not accept a timeout.
Some of them
> block
> > > > > forever
> > > > > > > > (e.g.
> > > > > > > > > > > > position()) and some of them use
request.timeout.ms
> > > (e.g.
> > > > > > > > > > > > parititonsFor()).
> > > > > > > > > > > > I think we'd probably all agree
that blocking forever
> > is
> > > > not
> > > > > > > useful
> > > > > > > > > > > > behavior and using request.timeout.ms
has always
> been
> > a
> > > > hack
> > > > > > > since
> > > > > > > > > it
> > > > > > > > > > > > controls a separate concern. I
think there are
> > basically
> > > > two
> > > > > > > > options
> > > > > > > > > to
> > > > > > > > > > > > address this:
> > > > > > > > > > > >
> > > > > > > > > > > > 1. We can add max.block.ms to
match the producer and
> > use
> > > > it
> > > > > as
> > > > > > > the
> > > > > > > > > > > default
> > > > > > > > > > > > timeout when a timeout is not
explicitly provided.
> This
> > > > will
> > > > > > fix
> > > > > > > > the
> > > > > > > > > > > > indefinite blocking behavior and
avoid conflating
> > > > > > > > request.timeout.ms
> > > > > > > > > .
> > > > > > > > > > > > 2. We can deprecate the methods
which don't accept a
> > > > timeout.
> > > > > > > > > > > >
> > > > > > > > > > > > I'm leaning toward the first solution
because I think
> > we
> > > > want
> > > > > > to
> > > > > > > > push
> > > > > > > > > > > users
> > > > > > > > > > > > to specifying timeouts through
configuration rather
> > than
> > > in
> > > > > > code
> > > > > > > > > (Jay's
> > > > > > > > > > > > original argument). I think the
overloads are still
> > > useful
> > > > > for
> > > > > > > > > advanced
> > > > > > > > > > > > usage (e.g. in kafka streams),
but we should give
> users
> > > an
> > > > > easy
> > > > > > > > > option
> > > > > > > > > > > with
> > > > > > > > > > > > reasonable default behavior.
> > > > > > > > > > > >
> > > > > > > > > > > > If that sounds ok, I'd propose
we add it to this KIP
> > and
> > > > fix
> > > > > it
> > > > > > > > now.
> > > > > > > > > > This
> > > > > > > > > > > > gives users an easy way to get
the benefit of the
> > > > > improvements
> > > > > > > from
> > > > > > > > > > this
> > > > > > > > > > > > KIP without changing any code.
> > > > > > > > > > > >
> > > > > > > > > > > > Thanks,
> > > > > > > > > > > > Jason
> > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > > > On Sun, May 13, 2018 at 7:58 PM,
Richard Yu <
> > > > > > > > > > yohan.richard.yu@gmail.com>
> > > > > > > > > > > > wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > > Hi,
> > > > > > > > > > > > >
> > > > > > > > > > > > > With 3 binding votes and
6 non-binding, this KIP
> > would
> > > be
> > > > > > > > accepted.
> > > > > > > > > > > > >
> > > > > > > > > > > > > Thanks for participating.
> > > > > > > > > > > > >
> > > > > > > > > > > > > On Thu, May 10, 2018 at 2:35
AM, Edoardo Comar <
> > > > > > > > edocomar@gmail.com
> > > > > > > > > >
> > > > > > > > > > > > wrote:
> > > > > > > > > > > > >
> > > > > > > > > > > > > > +1 (non-binding)
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > On 10 May 2018 at 10:29,
zhenya Sun <
> > token01@126.com
> > > >
> > > > > > wrote:
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > > +1 non-binding
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > 在 2018年5月10日,下午5:19,Manikumar
<
> > > > > > manikumar.reddy@gmail.com
> > > > > > > >
> > > > > > > > > 写道:
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > +1 (non-binding).
> > > > > > > > > > > > > > > > Thanks.
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > On Thu, May
10, 2018 at 2:33 PM, Mickael
> > Maison <
> > > > > > > > > > > > > > > mickael.maison@gmail.com>
> > > > > > > > > > > > > > > > wrote:
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > >> +1 (non
binding)
> > > > > > > > > > > > > > > >> Thanks
> > > > > > > > > > > > > > > >>
> > > > > > > > > > > > > > > >> On Thu,
May 10, 2018 at 9:39 AM, Rajini
> > Sivaram
> > > <
> > > > > > > > > > > > > > > rajinisivaram@gmail.com>
> > > > > > > > > > > > > > > >> wrote:
> > > > > > > > > > > > > > > >>> Hi
Richard, Thanks for the KIP.
> > > > > > > > > > > > > > > >>>
> > > > > > > > > > > > > > > >>> +1
(binding)
> > > > > > > > > > > > > > > >>>
> > > > > > > > > > > > > > > >>> Regards,
> > > > > > > > > > > > > > > >>>
> > > > > > > > > > > > > > > >>> Rajini
> > > > > > > > > > > > > > > >>>
> > > > > > > > > > > > > > > >>> On
Wed, May 9, 2018 at 10:54 PM, Guozhang
> > Wang
> > > <
> > > > > > > > > > > > wangguoz@gmail.com
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > > >> wrote:
> > > > > > > > > > > > > > > >>>
> > > > > > > > > > > > > > > >>>>
+1 from me, thanks!
> > > > > > > > > > > > > > > >>>>
> > > > > > > > > > > > > > > >>>>
> > > > > > > > > > > > > > > >>>>
Guozhang
> > > > > > > > > > > > > > > >>>>
> > > > > > > > > > > > > > > >>>>
On Wed, May 9, 2018 at 10:46 AM, Jason
> > > > Gustafson <
> > > > > > > > > > > > > > jason@confluent.io>
> > > > > > > > > > > > > > > >>>>
wrote:
> > > > > > > > > > > > > > > >>>>
> > > > > > > > > > > > > > > >>>>>
Thanks for the KIP, +1 (binding).
> > > > > > > > > > > > > > > >>>>>
> > > > > > > > > > > > > > > >>>>>
One small correction: the KIP mentions
> that
> > > > > close()
> > > > > > > > will
> > > > > > > > > be
> > > > > > > > > > > > > > > >> deprecated,
> > > > > > > > > > > > > > > >>>>
but
> > > > > > > > > > > > > > > >>>>>
we do not want to do this because it is
> > > needed
> > > > by
> > > > > > the
> > > > > > > > > > > Closeable
> > > > > > > > > > > > > > > >>>>
interface.
> > > > > > > > > > > > > > > >>>>>
We only want to deprecate close(long,
> > > TimeUnit)
> > > > > in
> > > > > > > > favor
> > > > > > > > > of
> > > > > > > > > > > > > > > >>>>>
close(Duration).
> > > > > > > > > > > > > > > >>>>>
> > > > > > > > > > > > > > > >>>>>
-Jason
> > > > > > > > > > > > > > > >>>>>
> > > > > > > > > > > > > > > >>>>>
On Tue, May 8, 2018 at 12:43 AM,
> > khaireddine
> > > > > > Rezgui <
> > > > > > > > > > > > > > > >>>>>
khaireddine120@gmail.com> wrote:
> > > > > > > > > > > > > > > >>>>>
> > > > > > > > > > > > > > > >>>>>>
+1
> > > > > > > > > > > > > > > >>>>>>
> > > > > > > > > > > > > > > >>>>>>
2018-05-07 20:35 GMT+01:00 Bill Bejeck <
> > > > > > > > > bbejeck@gmail.com
> > > > > > > > > > >:
> > > > > > > > > > > > > > > >>>>>>
> > > > > > > > > > > > > > > >>>>>>>
+1
> > > > > > > > > > > > > > > >>>>>>>
> > > > > > > > > > > > > > > >>>>>>>
Thanks,
> > > > > > > > > > > > > > > >>>>>>>
Bill
> > > > > > > > > > > > > > > >>>>>>>
> > > > > > > > > > > > > > > >>>>>>>
On Fri, May 4, 2018 at 7:21 PM, Richard
> > Yu
> > > <
> > > > > > > > > > > > > > > >>>>
yohan.richard.yu@gmail.com
> > > > > > > > > > > > > > > >>>>>>
> > > > > > > > > > > > > > > >>>>>>>
wrote:
> > > > > > > > > > > > > > > >>>>>>>
> > > > > > > > > > > > > > > >>>>>>>>
Hi all, I would like to bump this
> thread
> > > > since
> > > > > > > > > > discussion
> > > > > > > > > > > in
> > > > > > > > > > > > > the
> > > > > > > > > > > > > > > >>>>
KIP
> > > > > > > > > > > > > > > >>>>>>>>
appears to be reaching its conclusion.
> > > > > > > > > > > > > > > >>>>>>>>
> > > > > > > > > > > > > > > >>>>>>>>
> > > > > > > > > > > > > > > >>>>>>>>
> > > > > > > > > > > > > > > >>>>>>>>
On Thu, Mar 15, 2018 at 3:30 PM,
> Richard
> > > Yu
> > > > <
> > > > > > > > > > > > > > > >>>>>>
yohan.richard.yu@gmail.com>
> > > > > > > > > > > > > > > >>>>>>>>
wrote:
> > > > > > > > > > > > > > > >>>>>>>>
> > > > > > > > > > > > > > > >>>>>>>>>
Hi all,
> > > > > > > > > > > > > > > >>>>>>>>>
> > > > > > > > > > > > > > > >>>>>>>>>
Since there does not seem to be too
> > much
> > > > > > > discussion
> > > > > > > > > in
> > > > > > > > > > > > > > > >> KIP-266,
I
> > > > > > > > > > > > > > > >>>>>>
will
> > > > > > > > > > > > > > > >>>>>>>
be
> > > > > > > > > > > > > > > >>>>>>>>>
starting a voting thread.
> > > > > > > > > > > > > > > >>>>>>>>>
Here is the link to KIP-266 for
> > > reference:
> > > > > > > > > > > > > > > >>>>>>>>>
> > > > > > > > > > > > > > > >>>>>>>>>
https://cwiki.apache.org/
> > > > > > > confluence/pages/viewpage
> > > > > > > > .
> > > > > > > > > > > > > > > >>>>>>>>
action?pageId=75974886
> > > > > > > > > > > > > > > >>>>>>>>>
> > > > > > > > > > > > > > > >>>>>>>>>
Recently, I have made some updates to
> > the
> > > > > KIP.
> > > > > > To
> > > > > > > > > > > > reiterate,
> > > > > > > > > > > > > I
> > > > > > > > > > > > > > > >>>>
have
> > > > > > > > > > > > > > > >>>>>>>>>
included KafkaConsumer's commitSync,
> > > > > > > > > > > > > > > >>>>>>>>>
poll, and committed in the KIP. (we
> > will
> > > be
> > > > > > > adding
> > > > > > > > > to a
> > > > > > > > > > > > > > > >>>>>>>
TimeoutException
> > > > > > > > > > > > > > > >>>>>>>>>
to them as well, in a similar manner
> > > > > > > > > > > > > > > >>>>>>>>>
to what we will be doing for
> > position())
> > > > > > > > > > > > > > > >>>>>>>>>
> > > > > > > > > > > > > > > >>>>>>>>>
Thanks,
> > > > > > > > > > > > > > > >>>>>>>>>
Richard Yu
> > > > > > > > > > > > > > > >>>>>>>>>
> > > > > > > > > > > > > > > >>>>>>>>>
> > > > > > > > > > > > > > > >>>>>>>>
> > > > > > > > > > > > > > > >>>>>>>
> > > > > > > > > > > > > > > >>>>>>
> > > > > > > > > > > > > > > >>>>>>
> > > > > > > > > > > > > > > >>>>>>
> > > > > > > > > > > > > > > >>>>>>
--
> > > > > > > > > > > > > > > >>>>>>
Ingénieur en informatique
> > > > > > > > > > > > > > > >>>>>>
> > > > > > > > > > > > > > > >>>>>
> > > > > > > > > > > > > > > >>>>
> > > > > > > > > > > > > > > >>>>
> > > > > > > > > > > > > > > >>>>
> > > > > > > > > > > > > > > >>>>
--
> > > > > > > > > > > > > > > >>>>
-- Guozhang
> > > > > > > > > > > > > > > >>>>
> > > > > > > > > > > > > > > >>
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > >
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > --
> > > > > > > > > > > > > > "When the people fear
their government, there is
> > > > tyranny;
> > > > > > > when
> > > > > > > > > the
> > > > > > > > > > > > > > government fears the
people, there is liberty."
> > > [Thomas
> > > > > > > > > Jefferson]
> > > > > > > > > > > > > >
> > > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > --
> > > > > > > -- Guozhang
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> > >
> > >
> > > --
> > > -- Guozhang
> > >
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message