Yeah I think there are two possible approaches: 1. You get the versions in the metadata request and cache them and invalidate that cache if you get a version mismatch error (basically as we do with leadership information). 2. You check each connection
I think combining metadata request and version check only makes sense in (1), right? If it is (2) I don't see how you save anything and the requests don't really make sense because you're mixing cluster wide state about partitions with info about the answering broker. -Jay On Tue, Mar 15, 2016 at 4:25 PM, Magnus Edenhill <mag...@edenhill.se> wrote: > Hey Jay, > > as discussed earlier it is not safe to cache/relay a broker's version or > its supported API versions, > by the time the client connects the broker might have upgraded to another > version which effectively > makes this information useless in a cached form. > > The complexity of querying for protocol verion is very implementation > dependent and > hard to generalize on, I dont foresee any bigger problems adding support > for an extra protocol version > querying state in librdkafka, but other client devs should chime in. > There are already post-connect,pre-operation states for dealing with SSL > and SASL. > > The reason for putting the API versioning stuff in the Metadata request is > that it is already used > for bootstrapping a client and/or connection and thus saves us a round-trip > (and possibly a state). > > > For how this will be used; I can't speak for other client devs but aim to > make a mapping between > the features my client exposes to a set of specific APIs and their minimum > version.. > E.g.: Balanced consumer groups requires JoinGroup >= V0, LeaveGroup >= V0, > SyncGroup >= V0, and so on. > If those requirements can be fullfilled then the feature is enabled, > otherwise an error is returned to the user. > > /Magnus > > > 2016-03-15 23:35 GMT+01:00 Jay Kreps <j...@confluent.io>: > >> Hey Ashish, >> >> Can you expand in the proposal on how this would be used by clients? >> This proposal only has one slot for api versions, though in fact there >> is potentially a different version on each broker. I think the >> proposal is that every single time the client establishes a connection >> it would then need to issue a metadata request on that connection to >> check supported versions. Is that correct? >> >> The point of merging version information with metadata request was >> that the client wouldn't have to manage this additional state for each >> connection, but rather the broker would gather the information and >> give a summary of all brokers in the cluster. (Managing the state >> doesn't seem complex but actually since the full state machine for a >> request is something like begin connecting=>connection complete=>begin >> sending request=>do work sending=>await response=>do work reading >> response adding to the state machine around this is not as simple as >> it seems...you can see the code in the java client around this). >> >> It sounds like in this proposal you are proposing merging with the >> metadata request but not summarizing across the cluster? Can you >> explain the thinking vs a separate request? >> >> It would really be good if the KIP can summarize the whole interaction >> and how clients will work. >> >> -Jay >> >> On Tue, Mar 15, 2016 at 3:24 PM, Ashish Singh <asi...@cloudera.com> wrote: >> > Magnus and I had a brief discussion following the KIP call. KIP-35 >> > < >> https://cwiki.apache.org/confluence/display/KAFKA/KIP-35+-+Retrieving+protocol+version >> > >> > wiki has been updated accordingly. Please review the KIP and vote on the >> > corresponding vote thread. >> > >> > On Mon, Mar 14, 2016 at 11:27 PM, Ashish Singh <asi...@cloudera.com> >> wrote: >> > >> >> I think there is a bit of misunderstanding going on here regarding >> >> protocol documentation and its versioning. It could be that I am the one >> >> who misunderstood it, please correct me if so. >> >> >> >> Taking Gwen's example. >> >> >> >> 1. 0.10.0 (protocol v4) is released with current KIP-35 >> >> 2. On trunk, modify produce requests and bump to v5 >> >> 3. On trunk, we modify metadata requests and bump to v6 >> >> 4. Now we decide that the metadata change fixes a super critical issue >> and >> >> want to backport the change. What's the protocol version of the next >> >> release of 0.10.0 - which supports v6 protocol only partially? >> >> >> >> As per my understanding, this will be v7. When we say a broker is on >> >> ApiVersion 7, we do not necessarily mean that it also supports >> ApiVersion >> >> up to v7. A broker on ApiVersion v7 should probably mean, please refer >> v7 >> >> of protocol documentation to find out supported protocol versions of >> this >> >> broker. >> >> >> >> I just added an example on the KIP wiki to elaborate more on protocol >> >> documentation versioning. Below is the excerpt. >> >> >> >> For instance say we have two brokers, BrokerA has ApiVersion 4 and >> BrokerB >> >> has ApiVersion 5. This means we should have protocol documentations for >> >> ApiVersions 4 and 5. Say we have the following as protocol documentation >> >> for these two versions. >> >> >> >> Sample Protocol Documentation V4 >> >> Version: 4 // Comes from ApiVersion >> >> REQ_A_0: ... >> >> REQ_A_1: ... >> >> RESP_A_0: ... >> >> RESP_A_1: ... >> >> >> >> Sample Protocol Documentation V5 >> >> Version: 5 // Comes from ApiVersion >> >> REQ_A_1: ... >> >> REQ_A_2: ... >> >> RESP_A_1: ... >> >> RESP_A_2: ... >> >> >> >> All a client needs to know to be able to successfully communicate with a >> >> broker is what is the supported ApiVersion of the broker. Say via some >> >> mechanism, discussed below, client gets to know that BrokerA has >> ApiVersion >> >> 4 and BrokerB has ApiVersion 5. With that information, and the available >> >> protocol documentations for those ApiVersions client can deduce what >> >> protocol versions does the broker supports. In this case client will >> deduce >> >> that it can use v0 and v1 of REQ_A and RESP_A while talking to BrokerA, >> >> while it can use v1 and v2 of REQ_A and RESP_A while talking to BrokerB. >> >> >> >> On Mon, Mar 14, 2016 at 10:50 PM, Ewen Cheslack-Postava < >> e...@confluent.io >> >> > wrote: >> >> >> >>> Yeah, Gwen's example is a good one. And it doesn't even have to be >> thought >> >>> of in terms of the implementation -- you can think of the protocol >> itself >> >>> as effectively being possible to branch and have changes cherry-picked. >> >>> Given the way some changes interact and that only some may be feasible >> to >> >>> backport, this may be important. >> >>> >> >>> Similarly, it's difficult to make that definition . In practice, we >> >>> sometimes branch and effectively merge the protocol -- i.e. we develop >> 2 >> >>> KIPs with independent changes at the same time. If you force a linear >> >>> model, you also *force* the ordering of implementation, which will be a >> >>> pretty serious constraint in a lot of cases. Two protocol-changing KIPs >> >>> may >> >>> occur near in time, but one may be a much larger change. >> >>> >> >>> Finally, it might be worth noting that from a client developer's >> >>> perspective, the linear order may not be all that intuitive when we >> pile >> >>> on >> >>> a bunch of protocol changes in one release. They probably don't >> actually >> >>> care about that global protocol version. They'll care more about the >> types >> >>> of things Dana was talking about previously: LZ4 support (which isn't >> even >> >>> a protocol change, but an important feature clients might need to know >> >>> about!), Kafka-backed offset storage (requires 2 protocol changes), >> etc. >> >>> While we want to encourage supporting all features, we should be >> realistic >> >>> about how client developers tackle feature development and limited >> >>> bandwidth. They are probably more feature driven than version driven. >> >>> >> >>> This is what Gwen was saying I had mentioned. The idea of features is >> >>> actually separate from what has been described so far and *does* >> require a >> >>> mapping to protocol versions, but also allows you to capture more than >> >>> that >> >>> and at more flexible granularity (single request type protocol version >> >>> bump >> >>> or the whole set of requests could change). The idea isn't quite the >> same >> >>> as browser feature detection, but that's my frame of reference for it >> (see >> >>> https://developer.mozilla.org/en-US/docs/Browser_Feature_Detection), >> the >> >>> process of trying to sort out supported features and protocols based on >> >>> browser version IDs (sort of equivalent to broker implementation >> versions >> >>> here) is a huge mess. Going entirely the other route (say, only >> enabling a >> >>> feature in CSS3 if *all* CSS3 features are implemented) is really >> >>> restrictive. >> >>> >> >>> I don't have a concrete proposal right now, but something like >> "features" >> >>> that sit somewhere between a global protocol version number and only >> >>> individual request versions more accurately captures what we want to >> >>> express, in my opinion. >> >>> >> >>> -Ewen >> >>> >> >>> On Mon, Mar 14, 2016 at 6:45 PM, Gwen Shapira <g...@confluent.io> >> wrote: >> >>> >> >>> > Jay, >> >>> > >> >>> > Ewen had a good example: >> >>> > >> >>> > 1. 0.10.0 (protocol v4) is released with current KIP-35 >> >>> > 2. On trunk, modify produce requests and bump to v5 >> >>> > 3. On trunk, we modify metadata requests and bump to v6 >> >>> > 4. Now we decide that the metadata change fixes a super critical >> issue >> >>> and >> >>> > want to backport the change. What's the protocol version of the next >> >>> > release of 0.10.0 - which supports v6 protocol only partially? >> >>> > >> >>> > Gwen >> >>> > >> >>> > On Mon, Mar 14, 2016 at 6:38 PM, Jay Kreps <j...@confluent.io> wrote: >> >>> > >> >>> > > Hey Dana, >> >>> > > >> >>> > > I am actually thinking about it differently. Basically I think you >> are >> >>> > > imagining a world in which the Kafka code is the source of truth, >> and >> >>> > > the Kafka developers make random changes that inflict pain on you >> at >> >>> > > will. The protocol documentation is basically just some >> semi-accurate >> >>> > > description of what the code does. It sounds like this isn't too >> far >> >>> > > from the actual world. :-) In that world I agree that the best we >> >>> > > could do would be to assign some id to versions (the md5 of the >> Kafka >> >>> > > jar, maybe) and put in various checks around that in clients to >> try to >> >>> > > keep things working. >> >>> > > >> >>> > > But imagine a different approach where we try to really treat the >> >>> > > protocol as a document and treat that as the source of truth. We >> try >> >>> > > to make this document cover what is and isn't specified and make it >> >>> > > cover enough to support client implementations and a given Kafka >> >>> > > version covers some range of protocols explicitly. There is a >> version >> >>> > > of this document for each protocol version. The code implements the >> >>> > > protocol rather than vice versa. >> >>> > > >> >>> > > So in other words protocol changes are totally ordered and separate >> >>> > > from code development (we might develop them together but the >> protocol >> >>> > > assignment would come when you checked in the new protocol version >> >>> > > which would happen with your code). >> >>> > > >> >>> > > This was really the intention with the protocol originally (though >> we >> >>> > > were doing it on a per-api basis), but I think that understanding >> was >> >>> > > not shared by the full team and we have not done a great job of >> >>> > > important things like documentation or explaining how this are >> >>> > > supposed to work so we fall back on the "the protocol is whatever >> the >> >>> > > code does" thing. >> >>> > > >> >>> > > Does that make sense? In that sense think one of the more important >> >>> > > things we could get out of this would not be more versioning >> features >> >>> > > so much as clear docs and processes around protocol versioning. >> >>> > > >> >>> > > -Jay >> >>> > > >> >>> > > On Mon, Mar 14, 2016 at 6:22 PM, Dana Powers < >> dana.pow...@gmail.com> >> >>> > > wrote: >> >>> > > > Is a linear protocol int consistent with the current release >> model? >> >>> It >> >>> > > > seems like that would break down w/ the multiple release branches >> >>> that >> >>> > > are >> >>> > > > all simultaneously maintained? Or is it implicit that no patch >> >>> release >> >>> > > can >> >>> > > > ever bump the protocol int? Or maybe the protocol int gets some >> >>> extra >> >>> > > > "wiggle" on minor / major releases to create unallocated version >> >>> ints >> >>> > > that >> >>> > > > could be used on future patch releases / backports? >> >>> > > > >> >>> > > > I think the protocol version int does make sense for folks >> deploying >> >>> > from >> >>> > > > trunk. >> >>> > > > >> >>> > > > -Dana >> >>> > > > >> >>> > > > On Mon, Mar 14, 2016 at 6:13 PM, Jay Kreps <j...@confluent.io> >> >>> wrote: >> >>> > > > >> >>> > > >> Yeah I think that is the point--we have a proposal for a new >> >>> protocol >> >>> > > >> versioning scheme and a vote on it but it doesn't actually >> describe >> >>> > > >> how versioning will work yet! I gave my vague impression based >> on >> >>> this >> >>> > > >> thread, but I want to make sure that is correct and get it >> written >> >>> > > >> down before we adopt it. >> >>> > > >> >> >>> > > >> -Jay >> >>> > > >> >> >>> > > >> On Mon, Mar 14, 2016 at 5:58 PM, Gwen Shapira < >> g...@confluent.io> >> >>> > > wrote: >> >>> > > >> > On Mon, Mar 14, 2016 at 5:31 PM, Jay Kreps <j...@confluent.io> >> >>> > wrote: >> >>> > > >> > >> >>> > > >> >> Couple of missing things: >> >>> > > >> >> >> >>> > > >> >> This KIP doesn't have a proposal on versioning it just gives >> >>> > > different >> >>> > > >> >> options, it'd be good to get a concrete proposal in the KIP. >> >>> Here >> >>> > is >> >>> > > my >> >>> > > >> >> understanding of what we are proposing (can someone sanity >> check >> >>> > and >> >>> > > if >> >>> > > >> >> correct, update the kip): >> >>> > > >> >> >> >>> > > >> >> 1. We will augment the existing api_version field in the >> >>> header >> >>> > > with >> >>> > > >> a >> >>> > > >> >> protocol_version that will begin at some initial value and >> >>> > > increment >> >>> > > >> by >> >>> > > >> >> 1 >> >>> > > >> >> every time we make a changes to any of the api_versions >> >>> > (question: >> >>> > > >> >> including internal apis?). >> >>> > > >> >> >> >>> > > >> > >> >>> > > >> > Jay, this part was not in the KIP and was never discussed. >> >>> > > >> > Are you proposing adding this? Or is it just an assumption you >> >>> made? >> >>> > > >> > >> >>> > > >> > >> >>> > > >> > >> >>> > > >> >> 2. The protocol_version will be added to the metadata >> request >> >>> > > >> >> 3. We will also add a string that this proposal is calling >> >>> > > >> VersionString >> >>> > > >> >> which will describe the build of kafka in some way. The >> >>> clients >> >>> > > >> should >> >>> > > >> >> not >> >>> > > >> >> under any circumstances do anything with this string other >> >>> than >> >>> > > >> print it >> >>> > > >> >> out to the user. >> >>> > > >> >> >> >>> > > >> >> One thing I'm not sure about: I think currently metadata >> sits in >> >>> > the >> >>> > > >> client >> >>> > > >> >> for 10 mins by default. Say a client bootstraps and then a >> >>> server >> >>> > is >> >>> > > >> >> downgraded to an earlier version, won't the client's metadata >> >>> > version >> >>> > > >> >> indicate that that client handles a version it doesn't >> actually >> >>> > > handle >> >>> > > >> any >> >>> > > >> >> more? We need to document how clients will handle this. >> >>> > > >> >> >> >>> > > >> >> Here are some comments on other details: >> >>> > > >> >> >> >>> > > >> >> 1. As a minor thing I think we should avoid naming the >> fields >> >>> > > >> VersionId >> >>> > > >> >> and VersionString which sort of implies they are both used >> >>> for >> >>> > > >> >> versioning. >> >>> > > >> >> I think we should call them something like ProtocolVersion >> >>> and >> >>> > > >> >> BuildDescription, with BuildDescription being totally >> >>> > unspecified >> >>> > > >> other >> >>> > > >> >> than that it is some kind of human readable string >> >>> describing a >> >>> > > >> >> particular >> >>> > > >> >> Kafka build. We really don't want a client attempting to >> use >> >>> > this >> >>> > > >> >> string in >> >>> > > >> >> any way as that would always be the wrong thing to do in >> the >> >>> > > >> versioning >> >>> > > >> >> scheme we are proposing, you should always use the >> protocol >> >>> > > version. >> >>> > > >> >> 2. Does making the topics field in the metadata request >> >>> nullable >> >>> > > >> >> actually make sense? We have a history of wanting to add >> >>> magical >> >>> > > >> values >> >>> > > >> >> rather than fields. Currently topics=[a] means give me >> >>> > information >> >>> > > >> about >> >>> > > >> >> topic a, topics=[] means give me information about all >> >>> topics, >> >>> > > and we >> >>> > > >> >> are >> >>> > > >> >> proposing topics=null would mean don't give me topics. I >> >>> don't >> >>> > > have a >> >>> > > >> >> strong opinion. >> >>> > > >> >> 3. I prefer Jason's proposal on using a conservative >> metadata >> >>> > > version >> >>> > > >> >> versus the empty response hack. However I think that may >> >>> > actually >> >>> > > >> >> exacerbate the downgrade scenario I described. >> >>> > > >> >> 4. I agree with Jason that we should really look at the >> >>> details >> >>> > of >> >>> > > >> the >> >>> > > >> >> implementation so we know it works--implementing server >> >>> support >> >>> > > >> without >> >>> > > >> >> actually trying it is kind of risky. >> >>> > > >> >> >> >>> > > >> >> As a meta comment: I'd really like to encourage us to think >> of >> >>> the >> >>> > > >> protocol >> >>> > > >> >> as a document that includes the following things: >> >>> > > >> >> >> >>> > > >> >> - The binary format, error codes, etc >> >>> > > >> >> - The request/response interaction >> >>> > > >> >> - The semantics of each request in different cases >> >>> > > >> >> - Instructions on how to use this to implement a client >> >>> > > >> >> >> >>> > > >> >> This document is versioned with the protocol number and is >> the >> >>> > > source of >> >>> > > >> >> truth for the protocol. >> >>> > > >> >> >> >>> > > >> >> Part of any protocol change needs to be an update to the >> >>> > > instructions on >> >>> > > >> >> how to use that part of the protocol. We should be >> opinionated. >> >>> If >> >>> > > there >> >>> > > >> >> are two options there should be a reason, and then we need to >> >>> > > document >> >>> > > >> both >> >>> > > >> >> and say exactly when to use each. >> >>> > > >> >> >> >>> > > >> >> I think we also need to get a "how to" document on protocol >> >>> changes >> >>> > > >> just so >> >>> > > >> >> people know what they need to do to add a new protocol >> feature. >> >>> > > >> >> >> >>> > > >> >> -Jay >> >>> > > >> >> >> >>> > > >> >> On Mon, Mar 14, 2016 at 4:51 PM, Jason Gustafson < >> >>> > ja...@confluent.io >> >>> > > > >> >>> > > >> >> wrote: >> >>> > > >> >> >> >>> > > >> >> > > >> >>> > > >> >> > > Are you suggesting this as a solution for the problem >> where >> >>> a >> >>> > > KIP-35 >> >>> > > >> >> > aware >> >>> > > >> >> > > client sends a higher version of metadata req, say v2, >> to a >> >>> > > KIP-35 >> >>> > > >> >> aware >> >>> > > >> >> > > broker that only supports up to v1. >> >>> > > >> >> > >> >>> > > >> >> > >> >>> > > >> >> > Yes, that's right. In that case, the client first sends v1, >> >>> finds >> >>> > > out >> >>> > > >> >> that >> >>> > > >> >> > the broker supports v2, and then sends v2 (if it has any >> >>> reason >> >>> > to >> >>> > > do >> >>> > > >> >> so). >> >>> > > >> >> > >> >>> > > >> >> > We had a bit of discussion on such scenarios, and it >> seemed to >> >>> > be a >> >>> > > >> >> chicken >> >>> > > >> >> > > and egg problem that is hard to avoid. Your suggestion >> >>> > definitely >> >>> > > >> makes >> >>> > > >> >> > > sense, however it falls under the purview of clients. >> >>> > > >> >> > >> >>> > > >> >> > >> >>> > > >> >> > That basically means clients should figure it out for >> >>> themselves? >> >>> > > >> Might >> >>> > > >> >> be >> >>> > > >> >> > nice to have a better answer. >> >>> > > >> >> > >> >>> > > >> >> > KIP-35 only aims on adding support for getting the version >> >>> info >> >>> > > from a >> >>> > > >> >> > > broker. This definitely can be utilized by our clients. >> >>> > However, >> >>> > > >> that >> >>> > > >> >> can >> >>> > > >> >> > > follow KIP-35 changes. Does this sound reasonable to you? >> >>> > > >> >> > >> >>> > > >> >> > >> >>> > > >> >> > It may be OK, but I'm a little concerned about offering a >> >>> feature >> >>> > > >> that we >> >>> > > >> >> > don't support ourselves. Sometimes it's not until >> >>> implementation >> >>> > > that >> >>> > > >> we >> >>> > > >> >> > find out whether it really works as expected. And if we're >> >>> > > eventually >> >>> > > >> >> > planning to support it, I feel we should think through >> some of >> >>> > the >> >>> > > >> cases >> >>> > > >> >> a >> >>> > > >> >> > bit more. For example, the upgrade and downgrade cases that >> >>> > Becket >> >>> > > >> >> > mentioned earlier. It doesn't feel too great to support >> this >> >>> > > feature >> >>> > > >> >> unless >> >>> > > >> >> > we can offer guidance on how to use it. >> >>> > > >> >> > >> >>> > > >> >> > -Jason >> >>> > > >> >> > >> >>> > > >> >> > >> >>> > > >> >> > >> >>> > > >> >> > On Mon, Mar 14, 2016 at 4:20 PM, Ashish Singh < >> >>> > asi...@cloudera.com >> >>> > > > >> >>> > > >> >> wrote: >> >>> > > >> >> > >> >>> > > >> >> > > Hi Jason, >> >>> > > >> >> > > >> >>> > > >> >> > > On Mon, Mar 14, 2016 at 4:04 PM, Jason Gustafson < >> >>> > > >> ja...@confluent.io> >> >>> > > >> >> > > wrote: >> >>> > > >> >> > > >> >>> > > >> >> > > > Perhaps clients should always send the oldest version >> of >> >>> the >> >>> > > >> metadata >> >>> > > >> >> > > > request which supports KIP-35 when initially >> connecting to >> >>> > the >> >>> > > >> >> cluster. >> >>> > > >> >> > > > Depending on the versions in the response, it can >> upgrade >> >>> to >> >>> > a >> >>> > > >> more >> >>> > > >> >> > > recent >> >>> > > >> >> > > > version. Then maybe we don't need the empty response >> hack? >> >>> > > >> >> > > > >> >>> > > >> >> > > Are you suggesting this as a solution for the problem >> where >> >>> a >> >>> > > KIP-35 >> >>> > > >> >> > aware >> >>> > > >> >> > > client sends a higher version of metadata req, say v2, >> to a >> >>> > > KIP-35 >> >>> > > >> >> aware >> >>> > > >> >> > > broker that only supports up to v1. >> >>> > > >> >> > > >> >>> > > >> >> > > We had a bit of discussion on such scenarios, and it >> seemed >> >>> to >> >>> > > be a >> >>> > > >> >> > chicken >> >>> > > >> >> > > and egg problem that is hard to avoid. Your suggestion >> >>> > definitely >> >>> > > >> makes >> >>> > > >> >> > > sense, however it falls under the purview of clients. >> >>> > > >> >> > > >> >>> > > >> >> > > > >> >>> > > >> >> > > > One thing that's not clear to me is whether the >> ultimate >> >>> goal >> >>> > > of >> >>> > > >> this >> >>> > > >> >> > KIP >> >>> > > >> >> > > > is to have our clients support multiple broker >> versions. >> >>> It >> >>> > > would >> >>> > > >> be >> >>> > > >> >> a >> >>> > > >> >> > > > little weird to have this feature if our own clients >> don't >> >>> > use >> >>> > > it. >> >>> > > >> >> > > > >> >>> > > >> >> > > KIP-35 only aims on adding support for getting the >> version >> >>> info >> >>> > > >> from a >> >>> > > >> >> > > broker. This definitely can be utilized by our clients. >> >>> > However, >> >>> > > >> that >> >>> > > >> >> can >> >>> > > >> >> > > follow KIP-35 changes. Does this sound reasonable to you? >> >>> > > >> >> > > >> >>> > > >> >> > > > >> >>> > > >> >> > > > -Jason >> >>> > > >> >> > > > >> >>> > > >> >> > > > On Mon, Mar 14, 2016 at 3:34 PM, Ashish Singh < >> >>> > > >> asi...@cloudera.com> >> >>> > > >> >> > > wrote: >> >>> > > >> >> > > > >> >>> > > >> >> > > > > On Mon, Mar 14, 2016 at 2:12 PM, Ismael Juma < >> >>> > > ism...@juma.me.uk >> >>> > > >> > >> >>> > > >> >> > > wrote: >> >>> > > >> >> > > > > >> >>> > > >> >> > > > > > On Mon, Mar 14, 2016 at 8:45 PM, Gwen Shapira < >> >>> > > >> g...@confluent.io >> >>> > > >> >> > >> >>> > > >> >> > > > wrote: >> >>> > > >> >> > > > > > > >> >>> > > >> >> > > > > > > > I don't see how it helps. If the client is >> >>> > > communicating >> >>> > > >> >> with a >> >>> > > >> >> > > > > broker >> >>> > > >> >> > > > > > > that >> >>> > > >> >> > > > > > > > does not support KIP-35, that broker will >> simply >> >>> > close >> >>> > > the >> >>> > > >> >> > > > > connection. >> >>> > > >> >> > > > > > If >> >>> > > >> >> > > > > > > > the broker supports KIP-35, then it will >> provide >> >>> the >> >>> > > >> broker >> >>> > > >> >> > > > version. >> >>> > > >> >> > > > > I >> >>> > > >> >> > > > > > > > don't envisage a scenario where a broker does >> not >> >>> > > support >> >>> > > >> >> > KIP-35, >> >>> > > >> >> > > > but >> >>> > > >> >> > > > > > > > implements the new behaviour of sending an >> empty >> >>> > > >> response. Do >> >>> > > >> >> > > you? >> >>> > > >> >> > > > > > > > >> >>> > > >> >> > > > > > > > Are you sure about that? Per KIP-35, the broker >> >>> > > supplies >> >>> > > >> the >> >>> > > >> >> > > > version >> >>> > > >> >> > > > > in >> >>> > > >> >> > > > > > > response to Metadata request, not in response to >> >>> > anything >> >>> > > >> else. >> >>> > > >> >> > > > > > > If the client sends producer request version 42 >> >>> > > >> (accidentally >> >>> > > >> >> or >> >>> > > >> >> > > due >> >>> > > >> >> > > > to >> >>> > > >> >> > > > > > > premature upgrade) to KIP-35-compactible broker >> - we >> >>> > > want to >> >>> > > >> >> see >> >>> > > >> >> > an >> >>> > > >> >> > > > > empty >> >>> > > >> >> > > > > > > packet and not a connection close. >> >>> > > >> >> > > > > > > Sending a broker version was deemed impractical >> >>> IIRC. >> >>> > > >> >> > > > > > > >> >>> > > >> >> > > > > > >> >>> > > >> >> > > > > > OK, so this is a different case than the one Ashish >> >>> > > described >> >>> > > >> >> > > ("client >> >>> > > >> >> > > > > that >> >>> > > >> >> > > > > > wants to support broker versions that do not >> provide >> >>> > broker >> >>> > > >> >> version >> >>> > > >> >> > > in >> >>> > > >> >> > > > > > metadata and broker versions that provides version >> >>> info >> >>> > in >> >>> > > >> >> > > metadata"). >> >>> > > >> >> > > > > So, >> >>> > > >> >> > > > > > you are suggesting that if a client is >> communicating >> >>> > with a >> >>> > > >> >> broker >> >>> > > >> >> > > that >> >>> > > >> >> > > > > > implements KIP-35 and it receives an empty >> response, >> >>> it >> >>> > > will >> >>> > > >> >> assume >> >>> > > >> >> > > > that >> >>> > > >> >> > > > > > the broker doesn't support the request version and >> it >> >>> > won't >> >>> > > >> try >> >>> > > >> >> to >> >>> > > >> >> > > > parse >> >>> > > >> >> > > > > > the response? I think it would be good to explain >> this >> >>> > > kind of >> >>> > > >> >> > thing >> >>> > > >> >> > > in >> >>> > > >> >> > > > > > detail in the KIP. >> >>> > > >> >> > > > > > >> >>> > > >> >> > > > > Actually even in this case and the case I mentioned, >> >>> > closing >> >>> > > >> >> > connection >> >>> > > >> >> > > > > should be fine. Lets think about possible reasons >> that >> >>> > could >> >>> > > >> lead >> >>> > > >> >> to >> >>> > > >> >> > > this >> >>> > > >> >> > > > > issue. >> >>> > > >> >> > > > > >> >>> > > >> >> > > > > 1. Client has incorrect mapping of supported >> protocols >> >>> for >> >>> > a >> >>> > > >> broker >> >>> > > >> >> > > > > version. >> >>> > > >> >> > > > > 2. Client misread broker version from metadata >> response. >> >>> > > >> >> > > > > 3. Client constructed unsupported protocol version by >> >>> > > mistake. >> >>> > > >> >> > > > > >> >>> > > >> >> > > > > In all the above cases irrespective of what broker >> does, >> >>> > > client >> >>> > > >> >> will >> >>> > > >> >> > > keep >> >>> > > >> >> > > > > sending wrong request version. >> >>> > > >> >> > > > > >> >>> > > >> >> > > > > At this point, I think sending an empty packet >> instead >> >>> of >> >>> > > >> closing >> >>> > > >> >> > > > > connection is a nice to have and not mandatory >> >>> requirement. >> >>> > > >> Like in >> >>> > > >> >> > the >> >>> > > >> >> > > > > above case, a client can catch parsing error and be >> sure >> >>> > that >> >>> > > >> there >> >>> > > >> >> > is >> >>> > > >> >> > > > > something wrong in the protocol version it is >> sending. >> >>> > > However, >> >>> > > >> a >> >>> > > >> >> > > generic >> >>> > > >> >> > > > > connection close does not really provide any >> >>> information on >> >>> > > >> >> probable >> >>> > > >> >> > > > cause. >> >>> > > >> >> > > > > >> >>> > > >> >> > > > > What do you guys suggest? >> >>> > > >> >> > > > > >> >>> > > >> >> > > > > > >> >>> > > >> >> > > > > > Ismael >> >>> > > >> >> > > > > > >> >>> > > >> >> > > > > >> >>> > > >> >> > > > > >> >>> > > >> >> > > > > >> >>> > > >> >> > > > > -- >> >>> > > >> >> > > > > >> >>> > > >> >> > > > > Regards, >> >>> > > >> >> > > > > Ashish >> >>> > > >> >> > > > > >> >>> > > >> >> > > > >> >>> > > >> >> > > >> >>> > > >> >> > > >> >>> > > >> >> > > >> >>> > > >> >> > > -- >> >>> > > >> >> > > >> >>> > > >> >> > > Regards, >> >>> > > >> >> > > Ashish >> >>> > > >> >> > > >> >>> > > >> >> > >> >>> > > >> >> >> >>> > > >> >> >>> > > >> >>> > >> >>> >> >>> >> >>> >> >>> -- >> >>> Thanks, >> >>> Ewen >> >>> >> >> >> >> >> >> >> >> -- >> >> >> >> Regards, >> >> Ashish >> >> >> > >> > >> > >> > -- >> > >> > Regards, >> > Ashish >>
