Note to all: I have included bounding commitSync() and committed() in this KIP.
On Sun, Mar 11, 2018 at 5:05 PM, Richard Yu <yohan.richard...@gmail.com> wrote: > Hi all, > > I updated the KIP where overloading position() is now the favored approach. > Bounding position() using requestTimeoutMs has been listed as rejected. > > Any thoughts? > > On Tue, Mar 6, 2018 at 6:00 PM, Guozhang Wang <wangg...@gmail.com> wrote: > >> I agree that adding the overloads is most flexible. But going for that >> direction we'd do that for all the blocking call that I've listed above, >> with this timeout value covering the end-to-end waiting time. >> >> >> Guozhang >> >> On Tue, Mar 6, 2018 at 10:02 AM, Ted Yu <yuzhih...@gmail.com> wrote: >> >> > bq. The most flexible option is to add overloads to the consumer >> > >> > This option is flexible. >> > >> > Looking at the tail of SPARK-18057, Spark dev voiced the same choice. >> > >> > +1 for adding overload with timeout parameter. >> > >> > Cheers >> > >> > On Mon, Mar 5, 2018 at 2:42 PM, Jason Gustafson <ja...@confluent.io> >> > wrote: >> > >> > > @Guozhang I probably have suggested all options at some point or >> another, >> > > including most recently, the current KIP! I was thinking that >> practically >> > > speaking, the request timeout defines how long the user is willing to >> > wait >> > > for a response. The consumer doesn't really have a complex send >> process >> > > like the producer for any of these APIs, so I wasn't sure how much >> > benefit >> > > there would be from having more granular control over timeouts (in the >> > end, >> > > KIP-91 just adds a single timeout to control the whole send). That >> said, >> > it >> > > might indeed be better to avoid overloading the config as you suggest >> > since >> > > at least it avoids inconsistency with the producer's usage. >> > > >> > > The most flexible option is to add overloads to the consumer so that >> > users >> > > can pass the timeout directly. I'm not sure if that is more or less >> > > annoying than a new config, but I've found config timeouts a little >> > > constraining in practice. For example, I could imagine users wanting >> to >> > > wait longer for an offset commit operation than a position lookup; if >> the >> > > latter isn't timely, users can just pause the partition and continue >> > > fetching on others. If you cannot commit offsets, however, it might be >> > > safer for an application to wait availability of the coordinator than >> > > continuing. >> > > >> > > -Jason >> > > >> > > On Sun, Mar 4, 2018 at 10:14 PM, Guozhang Wang <wangg...@gmail.com> >> > wrote: >> > > >> > > > Hello Richard, >> > > > >> > > > Thanks for the proposed KIP. I have a couple of general comments: >> > > > >> > > > 1. I'm not sure if piggy-backing the timeout exception on the >> > > > existing requestTimeoutMs configured in "request.timeout.ms" is a >> good >> > > > idea >> > > > since a) it is a general config that applies for all types of >> requests, >> > > and >> > > > 2) using it to cover all the phases of an API call, including >> network >> > > round >> > > > trip and potential metadata refresh is shown to not be a good idea, >> as >> > > > illustrated in KIP-91: >> > > > >> > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP- >> > > > 91+Provide+Intuitive+User+Timeouts+in+The+Producer >> > > > >> > > > In fact, I think in KAFKA-4879 which is aimed for the same issue as >> > > > KAFKA-6608, >> > > > Jason has suggested we use a new config for the API. Maybe this >> would >> > be >> > > a >> > > > more intuitive manner than reusing the request.timeout.ms config. >> > > > >> > > > >> > > > 2. Besides the Consumer.position() call, there are a couple of more >> > > > blocking calls today that could result in infinite blocking: >> > > > Consumer.commitSync() and Consumer.committed(), should they be >> > considered >> > > > in this KIP as well? >> > > > >> > > > 3. There are a few other APIs that are today relying on >> > > request.timeout.ms >> > > > already for breaking the infinite blocking, namely >> > > Consumer.partitionFor(), >> > > > Consumer.OffsetAndTimestamp() and Consumer.listTopics(), if we are >> > making >> > > > the other blocking calls to be relying a new config as suggested in >> 1) >> > > > above, should we also change the semantics of these API functions >> for >> > > > consistency? >> > > > >> > > > >> > > > Guozhang >> > > > >> > > > >> > > > >> > > > >> > > > On Sun, Mar 4, 2018 at 11:13 AM, Richard Yu < >> > yohan.richard...@gmail.com> >> > > > wrote: >> > > > >> > > > > Hi all, >> > > > > >> > > > > I would like to discuss a potential change which would be made to >> > > > > KafkaConsumer: >> > > > > https://cwiki.apache.org/confluence/pages/viewpage. >> > > > action?pageId=75974886 >> > > > > >> > > > > Thanks, >> > > > > Richard Yu >> > > > > >> > > > >> > > > >> > > > >> > > > -- >> > > > -- Guozhang >> > > > >> > > >> > >> >> >> >> -- >> -- Guozhang >> > >
