Thanks Guozhang for the summary. If everyone agrees to the strategy here, which include:
1. No affection to 2.5 release, maintaining the same crashing logic for new Producer API talking to old clients 2. Add an internal flag to silently downgrade protocol for Kafka Streams case only I will go ahead to make the corresponding changes to unblock. Boyang On Mon, Mar 30, 2020 at 11:22 AM Guozhang Wang <wangg...@gmail.com> wrote: > Hello Colin, > > Thanks for the context you provided with CreateAclsRequest to make the > point for compatibility policy, and I think they make sense -- given > KIP-447 is primarily motivated for Streams clients, we should still > consider it's potential compatibility pitfall for non-Streams clients. > > About the resolution here, personally I do not think that punt this KIP for > the next release is a good idea; instead I think we can make the change for > Streams only while keeping non-Streams to still throw --- Boyang have > proposed with the internal flag do to so, and I'd want to add that we > should emphasize on docs when UnsupportedVersion is thrown on producer it > might already be too late (as shown in the example in KIP wiki, in such a > consumer -> process -> producer pattern when producer throws some > processing may already been done with irreversible side-effects) so that > users are aware. > > > Guozhang > > > On Sun, Mar 29, 2020 at 9:07 PM Colin McCabe <cmcc...@apache.org> wrote: > > > Thanks for the explanations, all. It seems to me that the proposed > silent > > downgrade change leaves non-streams applications with no way to safely > use > > KIP-447. If they use it, it will sometimes be safe, but sometimes not, > > depending on what they are doing. > > > > This doesn't seem acceptable. People should be able to rely on EOS when > > it exists. > > > > I agree that it is frustrating that the UnsupportedVersionException > > happens when the applications have already set up many things, rather > than > > being accessible when the application first starts up. But this does not > > fundamentally change the compatibility policy. It is difficult > (actually, > > it's impossible) to work around the lack of CreateAclsRequest or > > DeleteAclsRequest. But this doesn't motivate us to return fake results > for > > the corresponding AdminClient calls when the RPCs are not present. Some > > things can't be faked. > > > > It seems like the right thing to do here would be to expose the KIP-447 > > functionality as a feature flag. Then, applications can test if that > > feature flag is present before they perform a long setup process. They > can > > also decide if they want to provide a fallback path or not. > > > > Of course, feature flags won't make 2.5, so we need some more short-term > > solution here. Either we punt this KIP to the next release, or we find > > some way to make it safe for everyone, not just Streams (possibly by only > > enabling it for streams?) > > > > best, > > Colin > > > > > > On Sat, Mar 28, 2020, at 01:31, Matthias J. Sax wrote: > > > I agree with Guozhang that the issue only arises for the > > > `read-process-write` pattern and thus we should guard at the read level > > > already and thus auto-downgrading the write path commitTx request seems > > > fine. > > > > > > Note that for producer-only and consumer-only apps the auto-downgrade > is > > > totally fine (and actually desired, especially for the consumer to > > > guarantee backward compatibility as there is not public API change) for > > > the corresponding request. For `read-process-write` with no > exactly-once > > > or the existing exactly-once pattern auto-downgrading is also fine for > > > both clients. > > > > > > The only issue I see atm might be, that the consumer side guard depends > > > on an internal config and there is not public API to enable this guard. > > > Hence, for non-Streams users there might be a gap: If they don't > upgrade > > > their brokers first but switch to the new exactly-once pattern they > > > loose exactly-once protection without any warning... > > > > > > I am frankly not sure how many users would actually build exactly-once > > > apps without using Streams and might be affected? > > > > > > To address the issue I consider the following two options: > > > > > > - educate users about this internal consumer config > > > - make the consumer config public > > > > > > > > > Thoughts? > > > > > > -Matthias > > > > > > > > > > > > > > > On 3/27/20 10:12 PM, Guozhang Wang wrote: > > > > Hello Colin, Ismael, > > > > > > > > Thanks for your feedbacks, they are quite helpful. Just to provide > some > > > > context here about OffsetFetch: > > > > > > > > 1) When building the offset fetch request, we used to auto > "downgrade" > > the > > > > request by falling back the requireStable flag when broker supporter > > > > version is < 7. > > > > > > > > > > > https://github.com/apache/kafka/blob/trunk/clients/src/main/java/org/apache/kafka/common/requests/OffsetFetchRequest.java#L82 > > > > > > > > 2) The newly introduced config value "eos-beta" in KIP-447 requires > > brokers > > > > to be on version 2.5 or later, and if broker is not on newer version, > > we > > > > need to let the client to fail fast; in order to do so an internal > > config > > > > from consumer is exposed to Streams, which allows the build function > to > > > > throw the UnsupportedVersionException instead of auto downgrading > when > > > > building the offset-fetch that requireStable flag. This is only > turned > > on > > > > by Streams so that if broker is not on newer version when "eos-beta" > is > > > > enabled on the client, we will fail fast . > > > > > > > > In that sense, a new client would not use old RPC talking to the > > brokers -- > > > > the key point is that, upon starting a task the offset fetch is > firstly > > > > sent before we start processing records, so that's where we should > > stop the > > > > client right away instead of silently reading unstable offsets which > > would > > > > already break the fencing guarantees if producers were not fenced on > > txn > > > > coordinator. In other words, we think that letting the overloaded > > > > producer#sendOffsetsToTxn to throw UnsupportedVersion exception is > > already > > > > too late, and hence we should not relying on that API to let clients > > detect > > > > if brokers are on older versions. > > > > > > > > For KIP-98, however, letting producer#initTxn to throw > > UnsupportedVersion > > > > makes total sense since that's the first API triggered before > > producers try > > > > to send any records. > > > > > > > > 3) With that in mind, I think letting the overloaded > > > > producer#sendOffsetsToTxn to not throw UnsupportedVersion is > > appropriate, > > > > since KIP-447 only applies to a consume -> process -> produce > scenario > > and > > > > does not apply to a producer-only scenario for transactional > messaging. > > > > Instead, the consumer API calls that trigger earlier would detect > older > > > > brokers. A side benefit of this is that, on the caller side (a.k.a > > Streams) > > > > we do not need to first detect the broker version and then decide > which > > > > overloaded `producer#sendOffsetsToTxn` to call based on that, which > > also > > > > means that we may eventually deprecate the old overloaded function > one > > day. > > > > > > > > > > > > Please let me know if you have any questions regarding this > rationale, > > and > > > > we can continue the discussion from there. > > > > > > > > > > > > Guozhang > > > > > > > > > > > > > > > > On Fri, Mar 27, 2020 at 9:21 PM Ismael Juma <ism...@juma.me.uk> > wrote: > > > > > > > >> I'm a bit puzzled. We added this feature because we thought it was > > useful. > > > >> And now we are saying that you don't know if you can rely on it > since > > the > > > >> downgrade happens silently. Can you provide more context on the > > OffsetFetch > > > >> downgrade? What is the implication of that? > > > >> > > > >> Ismael > > > >> > > > >> On Fri, Mar 27, 2020 at 7:30 PM Boyang Chen < > > reluctanthero...@gmail.com> > > > >> wrote: > > > >> > > > >>> Thanks Colin, I think the point of this change is to make the new > > client > > > >>> experience better while working with old brokers, when the upgrade > is > > > >>> happening on client side only. For Streams, it is very common to > have > > > >> more > > > >>> advanced client version. On code level, the path to call > transaction > > > >> commit > > > >>> is better to be unified instead of diverged with try-catch. As a > > matter > > > >> of > > > >>> fact, we have already implemented the downgrade of the OffsetFetch > > > >> protocol > > > >>> to adapt this broker version incompatibility. New transaction > commit > > > >>> protocol thus behaves inconsistent with what we did with other > > protocols. > > > >>> > > > >>> In terms of the impact for this last minute change, silent > downgrade > > has > > > >>> one downside which is the customized EOS applications could not > > alarm the > > > >>> user of semantic violation anymore by simply crashing. To be > honest, > > I'm > > > >>> not sure how severe this would affect customized community usages, > > as we > > > >>> don't have user stats for that yet :) > > > >>> > > > >>> Boyang > > > >>> > > > >>> On Fri, Mar 27, 2020 at 6:32 PM Colin McCabe <cmcc...@apache.org> > > wrote: > > > >>> > > > >>>> On Fri, Mar 27, 2020, at 18:29, Colin McCabe wrote: > > > >>>>> On Fri, Mar 27, 2020, at 16:06, Boyang Chen wrote: > > > >>>>>> Hey there, > > > >>>>>> > > > >>>>>> we would like to address an improvement on the > > > >>>>>> *Producer#sendOffsetsToTransaction(offsets, > > > >>>>>> groupMetadata) *API. Previously we assume the calling of this > > > >>> function > > > >>>>>> would crash the app if broker is not on version 2.5.0 or higher. > > As > > > >>>> Streams > > > >>>>>> side change was rolling out, the disadvantage of this behavior > > > >>> becomes > > > >>>>>> obvious, since on the application level it is hard to detect > > broker > > > >>>> version > > > >>>>>> and do separate call paths for old and new transaction commit > > > >>>> protocols. > > > >>>>> > > > >>>>> Hi Boyang, > > > >>>>> > > > >>>>> Applications are never supposed to detect broker versions, but > they > > > >> may > > > >>>>> be required to detect the presence or absence of a feature. That > > > >> would > > > >>>>> happen if a new feature was introduced that couldn't be emulated > > with > > > >>>>> some combination of the features that previously existed. > > > >>>>> > > > >>>>> That was the case for the original EOS work. For example, > > > >>>>> initTransactions will throw UnsupportedVersionException if the > > > >> producer > > > >>>>> tries to use it, but the broker doesn't support it. > > > >>>>> > > > >>>>>> > > > >>>>>> Thus, we are planning to change the behavior from crashing to > > > >>> silently > > > >>>>>> downgrade to the older transactional offset commit protocol by > > > >>>> resetting > > > >>>>>> the consumer group metadata fields. Details in this PR > > > >>>>>> <https://github.com/apache/kafka/pull/8375>. > > > >>>>>> > > > >>>>> > > > >>>>> The big question is will using the old client code with the old > RPC > > > >> be > > > >>>>> less safe than using the old client code with the old RPC? If it > > > >> will, > > > >>>>> there's a strong case to be made that streams should catch the > > > >>>>> UnsupportedVersionException and fall back to its old behavior. > > > >>>> > > > >>>> Sorry, that should read "new client code with old RPC". > Basically, > > are > > > >>> we > > > >>>> making things worse for users with older brokers and newer > clients? > > > >>>> > > > >>>> Colin > > > >>>> > > > >>>>> > > > >>>>> best, > > > >>>>> Colin > > > >>>>> > > > >>>>>> > > > >>>>>> Let me know if you have any questions, thanks! > > > >>>>>> > > > >>>>>> On Thu, Mar 26, 2020 at 12:45 PM Matthias J. Sax < > > mj...@apache.org > > > >>> > > > >>>> wrote: > > > >>>>>> > > > >>>>>>> One more change for KIP-447. > > > >>>>>>> > > > >>>>>>> Currently, Kafka Streams collects task-level metrics called > > > >>>>>>> "commit-latency-[max|avg]". However, with KIP-447 tasks don't > get > > > >>>>>>> committed individually any longer, and thus this metrics do not > > > >>> make > > > >>>>>>> sense going forward. > > > >>>>>>> > > > >>>>>>> Therefore, we propose to remove those metrics in 2.6 release. > > > >>>>>>> Deprecation does not make much sense, as there is just nothing > to > > > >>> be > > > >>>>>>> recorded in a useful way any longer. > > > >>>>>>> > > > >>>>>>> I also updated the upgrade path section, as the upgrade path is > > > >>>> actually > > > >>>>>>> simpler as described originally. > > > >>>>>>> > > > >>>>>>> > > > >>>>>>> If there are any concerns, please let us know! > > > >>>>>>> > > > >>>>>>> > > > >>>>>>> -Matthias > > > >>>>>>> > > > >>>>>>> > > > >>>>>>> On 3/5/20 12:56 PM, Matthias J. Sax wrote: > > > >>>>>>>> There is one more change to this KIP for the upgrade path of > > > >>> Kafka > > > >>>>>>>> Streams applications: > > > >>>>>>>> > > > >>>>>>>> We cannot detect broker versions reliable, and thus, we need > > > >>> users > > > >>>> to > > > >>>>>>>> manually opt-in to the feature. Thus, we need to add a third > > > >>> value > > > >>>> for > > > >>>>>>>> configuration parameter `processing.guarantee` that we call > > > >>>>>>>> `exactly_once_beta` -- specifying this config will enable the > > > >>>> producer > > > >>>>>>>> per thread design. > > > >>>>>>>> > > > >>>>>>>> I updated the KIP accordingly. Please let us know if there are > > > >>> any > > > >>>>>>>> concerns. > > > >>>>>>>> > > > >>>>>>>> > > > >>>>>>>> -Matthias > > > >>>>>>>> > > > >>>>>>>> On 2/11/20 4:44 PM, Guozhang Wang wrote: > > > >>>>>>>>> Boyang, > > > >>>>>>>> > > > >>>>>>>>> Thanks for the update. This change makes sense to me. > > > >>>>>>>> > > > >>>>>>>>> Guozhang > > > >>>>>>>> > > > >>>>>>>>> On Tue, Feb 11, 2020 at 11:37 AM Boyang Chen > > > >>>>>>>>> <reluctanthero...@gmail.com> wrote: > > > >>>>>>>> > > > >>>>>>>>>> Hey there, > > > >>>>>>>>>> > > > >>>>>>>>>> we are adding a small change to the KIP-447 public API. The > > > >>>>>>>>>> default value of > > > >>>>>>>>>> ` > transaction.abort.timed.out.transaction.cleanup.interval.ms > > > >> ` > > > >>>>>>>>>> shall be changed from 1 minute to 10 seconds. The goal here > > > >> is > > > >>> to > > > >>>>>>>>>> trigger the expired transaction more frequently in order to > > > >>>>>>>>>> reduce the consumer pending offset fetch wait time. > > > >>>>>>>>>> > > > >>>>>>>>>> Let me know if you have further questions, thanks! > > > >>>>>>>>>> > > > >>>>>>>>>> > > > >>>>>>>>>> On Wed, Jan 8, 2020 at 3:44 PM Boyang Chen > > > >>>>>>>>>> <reluctanthero...@gmail.com> wrote: > > > >>>>>>>>>> > > > >>>>>>>>>>> Thanks Guozhang for another review! I have addressed all > the > > > >>>>>>>>>>> javadoc changes for PendingTransactionException in the KIP. > > > >>>>>>>>>>> For > > > >>>>>>>>>> FENCED_INSTANCE_ID > > > >>>>>>>>>>> the only thrown place would be on the new send offsets API, > > > >>>>>>>>>>> which is also addressed. > > > >>>>>>>>>>> > > > >>>>>>>>>>> Thanks Matthias for the vote! As we have 3 binding votes > > > >>>>>>>>>>> (Guozhang, > > > >>>>>>>>>> Jason, > > > >>>>>>>>>>> and Matthias), the KIP is officially accepted and prepared > > > >> to > > > >>>>>>>>>>> ship in > > > >>>>>>>>>> 2.5. > > > >>>>>>>>>>> > > > >>>>>>>>>>> Still feel free to put more thoughts on either discussion > or > > > >>>>>>>>>>> voting > > > >>>>>>>>>> thread > > > >>>>>>>>>>> to refine the KIP! > > > >>>>>>>>>>> > > > >>>>>>>>>>> > > > >>>>>>>>>>> On Wed, Jan 8, 2020 at 3:15 PM Matthias J. Sax > > > >>>>>>>>>>> <matth...@confluent.io> wrote: > > > >>>>>>>>>>> > > > >>>>>>>>>>>> I just re-read the KIP. Overall I am +1 as well. > > > >>>>>>>>>>>> > > > >>>>>>>>>>> > > > >>>>>>>>>>>> Some minor comments (also apply to the Google design doc): > > > >>>>>>>>>>>> > > > >>>>>>>>>>>> 1) As 2.4 was release, references should be updated to > 2.5. > > > >>>>>>>>>>>> > > > >>>>>>>>>>>> Addressed > > > >>>>>>>>>>> > > > >>>>>>>>>>>> > > > >>>>>>>>>>>> > > > >>>>>>>>>>>>> 2) About the upgrade path, the KIP says: > > > >>>>>>>>>>>> > > > >>>>>>>>>>>> 2a) > > > >>>>>>>>>>>> > > > >>>>>>>>>>>>> Broker must be upgraded to 2.4 first. This means the > > > >>>>>>>>>>>> `inter.broker.protocol.version` (IBP) has to be set to the > > > >>>>>>>>>>>> latest. Any produce request with higher version will > > > >>>>>>>>>>>> automatically get fenced > > > >>>>>>>>>> because > > > >>>>>>>>>>>> of no support. > > > >>>>>>>>>>>> > > > >>>>>>>>>>>> From my understanding, this is not correct? After a broker > > > >> is > > > >>>>>>>>>>>> updated to the new binaries, it should accept new > requests, > > > >>>>>>>>>>>> even if IBP was not bumped yet? > > > >>>>>>>>>>>> > > > >>>>>>>>>>>> Your understanding was correct, after some offline > > > >> discussion > > > >>>>>>>>>>>> we should > > > >>>>>>>>>>> not worry about IBP in this case. > > > >>>>>>>>>>> > > > >>>>>>>>>>>> 2b) > > > >>>>>>>>>>>> > > > >>>>>>>>>>>> About the two rolling bounces for KS apps and the > statement > > > >>>>>>>>>>>> > > > >>>>>>>>>>>>> one should never allow task producer and thread producer > > > >>>>>>>>>>>>> under the > > > >>>>>>>>>> same > > > >>>>>>>>>>>> application group > > > >>>>>>>>>>>> > > > >>>>>>>>>>>> In the second rolling bounce, we might actually mix both > > > >> (ie, > > > >>>>>>>>>>>> per-task and per-thread producers) but this is fine as > > > >>>>>>>>>>>> explained in the KIP. The only case we cannot allow is, > old > > > >>>>>>>>>>>> per-task producers (without consumer generation fencing) > to > > > >>>>>>>>>>>> be mixed with per-thread producers (that rely solely on > > > >>>>>>>>>>>> consumer generation fencing). > > > >>>>>>>>>>>> > > > >>>>>>>>>>>> Does this sound correct? > > > >>>>>>>>>>>> > > > >>>>>>>>>>>> Correct, that's the purpose of doing 2 rolling bounce, > > > >> where > > > >>>>>>>>>>>> the first > > > >>>>>>>>>>> one is to guarantee everyone's opt-in for generation > > > >> fencing. > > > >>>>>>>>>>> > > > >>>>>>>>>>>> > > > >>>>>>>>>>>> 3) We should also document how users can use KS 2.5 > > > >>>>>>>>>>>> applications against older brokers -- for this case, we > > > >> need > > > >>>>>>>>>>>> to stay on per-task producers and cannot use the new > > > >> fencing > > > >>>>>>>>>>>> mechanism. Currently, the KIP only describe a single way > > > >> how > > > >>>>>>>>>>>> users could make this work: by setting (and keeping) > > > >>>>>>>>>>>> UPGRADE_FROM config to 2.4 (what might not be an ideal > > > >>>>>>>>>>>> solution and might also not be clear by itself that people > > > >>>>>>>>>>>> would need to do > > > >>>>>>>>>> this)? > > > >>>>>>>>>>>> > > > >>>>>>>>>>>> > > > >>>>>>>>>>>> Yes exactly, at the moment we are actively working on a > > > >> plan > > > >>>>>>>>>>>> to acquire > > > >>>>>>>>>>> broker's IBP during stream start-up and initialize based > off > > > >>>>>>>>>>> that information, so that user doesn't need to keep > > > >>>>>>>>>>> UPGRADE_FROM simply for working with > > > >>>>>>>>>> old > > > >>>>>>>>>>> brokers. > > > >>>>>>>>>>> > > > >>>>>>>>>>>> > > > >>>>>>>>>>>> -Matthias > > > >>>>>>>>>>>> > > > >>>>>>>>>>>> > > > >>>>>>>>>>>> > > > >>>>>>>>>>>> On 9/18/19 4:41 PM, Boyang Chen wrote: > > > >>>>>>>>>>>>> Bump this thread to see if someone could also review! > > > >>>>>>>>>>>>> > > > >>>>>>>>>>>>> On Mon, Sep 9, 2019 at 5:00 PM Boyang Chen < > > > >>>>>>>>>> reluctanthero...@gmail.com> > > > >>>>>>>>>>>>> wrote: > > > >>>>>>>>>>>>> > > > >>>>>>>>>>>>>> Thank you Jason! Addressed the comments. > > > >>>>>>>>>>>>>> > > > >>>>>>>>>>>>>> Thank you Guozhang for explaining. I will document the > > > >>>>>>>>>>>>>> timeout > > > >>>>>>>>>> setting > > > >>>>>>>>>>>>>> reasoning in the design doc. > > > >>>>>>>>>>>>>> > > > >>>>>>>>>>>>>> > > > >>>>>>>>>>>>>> On Mon, Sep 9, 2019 at 1:49 PM Guozhang Wang > > > >>>>>>>>>>>>>> <wangg...@gmail.com> > > > >>>>>>>>>>>> wrote: > > > >>>>>>>>>>>>>> > > > >>>>>>>>>>>>>>> On Fri, Sep 6, 2019 at 6:33 PM Boyang Chen < > > > >>>>>>>>>>>> reluctanthero...@gmail.com> > > > >>>>>>>>>>>>>>> wrote: > > > >>>>>>>>>>>>>>> > > > >>>>>>>>>>>>>>>> Thanks Guozhang, I have polished the design doc to > > > >>>>>>>>>>>>>>>> make it sync > > > >>>>>>>>>> with > > > >>>>>>>>>>>>>>>> current KIP. As for overriding default timeout > > > >>>>>>>>>>>>>>>> values, I guess it's > > > >>>>>>>>>>>>>>> already > > > >>>>>>>>>>>>>>>> stated in the KIP to set txn timeout to 10s, are you > > > >>>>>>>>>>>>>>>> suggesting we > > > >>>>>>>>>>>>>>> should > > > >>>>>>>>>>>>>>>> also put down this recommendation on the KIP for > > > >>>>>>>>>>>>>>>> non-stream EOS > > > >>>>>>>>>>>> users? > > > >>>>>>>>>>>>>>>> > > > >>>>>>>>>>>>>>>> My comment is not for changing any produce / consumer > > > >>>>>>>>>>>>>>>> default > > > >>>>>>>>>> config > > > >>>>>>>>>>>>>>> values, but for the Streams configs, to make sure that > > > >>>>>>>>>>>>>>> our overridden config values respect the above rules. > > > >>>>>>>>>>>>>>> That is, we check > > > >>>>>>>>>>>> the > > > >>>>>>>>>>>>>>> actual value used in the config if they are ever > > > >>>>>>>>>>>>>>> overridden by > > > >>>>>>>>>> users, > > > >>>>>>>>>>>> and > > > >>>>>>>>>>>>>>> if the above were not true we can log a warning that it > > > >>>>>>>>>>>>>>> may be risky > > > >>>>>>>>>>>> to > > > >>>>>>>>>>>>>>> encounter some unnecessary rebalances. > > > >>>>>>>>>>>>>>> > > > >>>>>>>>>>>>>>> Again, this is not something we need to include in the > > > >>>>>>>>>>>>>>> KIP since it > > > >>>>>>>>>>>> is not > > > >>>>>>>>>>>>>>> part of public APIs, just to emphasize that the > > > >>>>>>>>>>>>>>> internal > > > >>>>>>>>>>>> implementation > > > >>>>>>>>>>>>>>> can have some safety guard like this. > > > >>>>>>>>>>>>>>> > > > >>>>>>>>>>>>>>> Guozhang > > > >>>>>>>>>>>>>>> > > > >>>>>>>>>>>>>>> > > > >>>>>>>>>>>>>>> > > > >>>>>>>>>>>>>>>> Boyang > > > >>>>>>>>>>>>>>>> > > > >>>>>>>>>>>>>>>> On Thu, Sep 5, 2019 at 8:43 PM Guozhang Wang > > > >>>>>>>>>>>>>>>> <wangg...@gmail.com> > > > >>>>>>>>>>>>>>> wrote: > > > >>>>>>>>>>>>>>>> > > > >>>>>>>>>>>>>>>>> Hello Boyang, > > > >>>>>>>>>>>>>>>>> > > > >>>>>>>>>>>>>>>>> Just realized one thing about timeout > > > >>>>>>>>>>>>>>>>> configurations that we > > > >>>>>>>>>> should > > > >>>>>>>>>>>>>>>>> consider including in this KIP as well: > > > >>>>>>>>>>>>>>>>> > > > >>>>>>>>>>>>>>>>> 1) In Producer we have: max.block.ms (default value > > > >>>>>>>>>>>>>>>>> 60sec), request.timeout (30sec), > > > >>>>>>>>>>>>>>>>> delivery.timeout.ms (120sec), transaction.timeout > > > >>>>>>>>>> (60sec) > > > >>>>>>>>>>>>>>>>> 2) In Consumer we have: session.timeout (10sec), > > > >>>>>>>>>>>>>>>>> request.timeout > > > >>>>>>>>>>>>>>> (30sec), > > > >>>>>>>>>>>>>>>>> default.api.timeout.ms (60sec). > > > >>>>>>>>>>>>>>>>> > > > >>>>>>>>>>>>>>>>> Within a transaction (i.e. after we've beginTxn), > > > >>>>>>>>>>>>>>>>> we could > > > >>>>>>>>>>>> potentially > > > >>>>>>>>>>>>>>>> call > > > >>>>>>>>>>>>>>>>> consumer blocking APIs that depend on > > > >>>>>>>>>>>>>>>>> default.api.timeout.ms, and > > > >>>>>>>>>>>>>>> call > > > >>>>>>>>>>>>>>>>> producer blocking APIs that depend on max.block.ms. > > > >>>>>>>>>>>>>>>>> Also, if the > > > >>>>>>>>>>>>>>> user is > > > >>>>>>>>>>>>>>>>> following a consumer->producer pattern, then it > > > >>>>>>>>>>>>>>>>> could be kicked > > > >>>>>>>>>> and > > > >>>>>>>>>>>>>>>> fenced > > > >>>>>>>>>>>>>>>>> either by txn or by consumer group session. > > > >>>>>>>>>>>>>>>>> > > > >>>>>>>>>>>>>>>>> So we want to make sure that in the caller, e.g. > > > >>>>>>>>>>>>>>>>> Kafka Streams: > > > >>>>>>>>>>>>>>>>> > > > >>>>>>>>>>>>>>>>> 1) transaction.timeout < max.block.ms 2) > > > >>>>>>>>>>>>>>>>> transaction.timeout < delivery.timeout.ms 3) > > > >>>>>>>>>>>>>>>>> transaction.timeout < default.api.timeout.ms 4) > > > >>>>>>>>>>>>>>>>> transaction.timeout ~= default.api.timeout.ms (I > > > >>>>>>>>>>>>>>>>> think this is > > > >>>>>>>>>>>>>>>> already > > > >>>>>>>>>>>>>>>>> mentioned in the KIP, just wanted to bring this up > > > >>>>>>>>>>>>>>>>> again) > > > >>>>>>>>>>>>>>>>> > > > >>>>>>>>>>>>>>>>> We do not need to override the default since not > > > >>>>>>>>>>>>>>>>> every users are > > > >>>>>>>>>>>>>>>> following > > > >>>>>>>>>>>>>>>>> the consumer -> producer pattern, but in cases like > > > >>>>>>>>>>>>>>>>> Streams where > > > >>>>>>>>>> it > > > >>>>>>>>>>>>>>> is > > > >>>>>>>>>>>>>>>>> indeed the case, we should override the default > > > >>>>>>>>>>>>>>>>> values to obey the > > > >>>>>>>>>>>>>>> above > > > >>>>>>>>>>>>>>>>> rules. > > > >>>>>>>>>>>>>>>>> > > > >>>>>>>>>>>>>>>>> Guozhang > > > >>>>>>>>>>>>>>>>> > > > >>>>>>>>>>>>>>>>> > > > >>>>>>>>>>>>>>>>> > > > >>>>>>>>>>>>>>>>> On Thu, Sep 5, 2019 at 5:47 PM Guozhang Wang > > > >>>>>>>>>>>>>>>>> <wangg...@gmail.com> > > > >>>>>>>>>>>>>>> wrote: > > > >>>>>>>>>>>>>>>>> > > > >>>>>>>>>>>>>>>>>> Thanks Boyang, I'm +1 on the KIP. > > > >>>>>>>>>>>>>>>>>> > > > >>>>>>>>>>>>>>>>>> Could you also update the detailed design doc > > > >>>>>>>>>>>>>>>>>> > > > >>>>>>>>>>>>>>>>> > > > >>>>>>>>>>>>>>>> > > > >>>>>>>>>>>>>>> > > > >>>>>>>>>>>> > > > >>>>>>>>>> > > > >>>> > > https://docs.google.com/document/d/1LhzHGeX7_Lay4xvrEXxfciuDWATjpUXQh > > > >>>>>>>> rEIkph9qRE/edit > > > >>>>>>>>>>>>>>>>> > > > >>>>>>>>>> > > > >>>>>>>> which > > > >>>>>>>>>>>>>>>>>> seems a bit out-dated with the latest proposal? > > > >>>>>>>>>>>>>>>>>> > > > >>>>>>>>>>>>>>>>>> > > > >>>>>>>>>>>>>>>>>> Guozhang > > > >>>>>>>>>>>>>>>>>> > > > >>>>>>>>>>>>>>>>>> On Wed, Sep 4, 2019 at 10:45 AM Boyang Chen < > > > >>>>>>>>>>>>>>>> reluctanthero...@gmail.com> > > > >>>>>>>>>>>>>>>>>> wrote: > > > >>>>>>>>>>>>>>>>>> > > > >>>>>>>>>>>>>>>>>>> Hey all, > > > >>>>>>>>>>>>>>>>>>> > > > >>>>>>>>>>>>>>>>>>> I would like to start the vote for KIP-447 < > > > >>>>>>>>>>>>>>>>>>> > > > >>>>>>>>>>>>>>>>> > > > >>>>>>>>>>>>>>>> > > > >>>>>>>>>>>>>>> > > > >>>>>>>>>>>> > > > >>>>>>>>>> > > > >>>> > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-447%3A+Producer > > > >>>>>>>> +scalability+for+exactly+once+semantics > > > >>>>>>>>>>>>>>>>>>>> > > > >>>>>>>>>> > > > >>>>>>>> . > > > >>>>>>>>>>>>>>>>>>> This is a very important step to improve Kafka > > > >>>>>>>>>>>>>>>>>>> Streams > > > >>>>>>>>>> scalability > > > >>>>>>>>>>>>>>> in > > > >>>>>>>>>>>>>>>>>>> exactly-once semantics, by avoiding linearly > > > >>>>>>>>>>>>>>>>>>> increasing number > > > >>>>>>>>>> of > > > >>>>>>>>>>>>>>>>>>> producers with topic partition increases. > > > >>>>>>>>>>>>>>>>>>> > > > >>>>>>>>>>>>>>>>>>> Thanks, Boyang > > > >>>>>>>>>>>>>>>>>>> > > > >>>>>>>>>>>>>>>>>> > > > >>>>>>>>>>>>>>>>>> > > > >>>>>>>>>>>>>>>>>> -- -- Guozhang > > > >>>>>>>>>>>>>>>>>> > > > >>>>>>>>>>>>>>>>> > > > >>>>>>>>>>>>>>>>> > > > >>>>>>>>>>>>>>>>> -- -- Guozhang > > > >>>>>>>>>>>>>>>>> > > > >>>>>>>>>>>>>>>> > > > >>>>>>>>>>>>>>> > > > >>>>>>>>>>>>>>> > > > >>>>>>>>>>>>>>> -- -- Guozhang > > > >>>>>>>>>>>>>>> > > > >>>>>>>>>>>>>> > > > >>>>>>>>>>>>> > > > >>>>>>>>>>>> > > > >>>>>>>>>>>> > > > >>>>>>>>>> > > > >>>>>>>> > > > >>>>>>>> > > > >>>>>>>> > > > >>>>>>> > > > >>>>>>> > > > >>>>>> > > > >>>> > > > >>> > > > >> > > > > > > > > > > > > > > > > > Attachments: > > > * signature.asc > > > > > -- > -- Guozhang >
