"I think I would make this approach work by looking at the released server version documentation for each version that I am trying to support and test against*, manually identify the expected "protocol vectors" each supports, store that as a map of vectors to "broker versions", check each vector at runtime until I find a match, and write code compatibility checks from there."
How is this better than a global version ID? On Thu, Mar 17, 2016 at 1:31 PM, Dana Powers <dana.pow...@gmail.com> wrote: > I like it. I think the revised KIP-35 approach is simple, easy to > understand, and workable. I think it will solve the main problem that I > have had, and I assume other 3rd party client devs have had as well, and > should open the door to more backwards-compatible drivers and less > fracturing of client development. > > As much as I think a single broker version could be easier to work with, > the conference call discussion convinced me that my thinking of broker > version is not totally aligned with the server development workflow and the > design ideas of the folks that have thought about this much more deeply > than I have. > > I think I would make this approach work by looking at the released server > version documentation for each version that I am trying to support and test > against*, manually identify the expected "protocol vectors" each supports, > store that as a map of vectors to "broker versions", check each vector at > runtime until I find a match, and write code compatibility checks from > there. I admit that I am not sure whether this would work if I was trying > to support trunk deploys where the "fallback" vector may not be as clearly > defined. But I do think that if a client is targeting trunk it probably > isn't also targeting backwards compatibility, and so it likely would have > little reason to use this API anyways (unless for nicer error messages if > particular apis are unsupported on the target broker). > > I think Magnus has much deeper insight into this than I do, and I wish I > had time to test a patch this weekend. Hopefully I will be able to get to > that next week. But I'm confident that if it works for librdkafka, it will > work for most if not all other 3rd party clients. > > -Dana > > * FWIW, every push / pull request to kafka-python gets automatically tested > against a number of public kafka releases via travis-ci. Currently we test > against 0.8.0, 0.8.1.1, 0.8.2.2, and 0.9.0.1 . > > On Thu, Mar 17, 2016 at 12:28 PM, Magnus Edenhill <mag...@edenhill.se> > wrote: > > > I dont really see how first checking if the broker supports a certain set > > of API versions > > is so much more complex than calling the same set of API versions during > > operation? > > The only difference is that without the check we might only make it > halfway > > through, > > i.e., being able to join a group, consume messages, but then fail to > commit > > them > > > > Another concrete example is support for KIP-31 and KIP-32: > > How will a client with support for 0.8.0-0.10.0 know when to use the new > > Message format? > > If a client uses the old Message format (v0) the broker will need to > > convert message formats > > in the slow path which will increase the broker CPU usage and decrease > its > > performance. > > Sure, clients can expose a configuration property to let the user enable > > this but most > > users will miss this (there are a lot of config properties already). > > > > > > > > > > 2016-03-17 19:43 GMT+01:00 Gwen Shapira <g...@confluent.io>: > > > > > My problem with per-api is that not all combinations are supported. So > > the > > > client will need to figure out which "safe" combination to revert to - > > > which is a lot like global numbers, but less linear... > > > > > > > > > On Thu, Mar 17, 2016 at 11:29 AM, Joel Koshy <jjkosh...@gmail.com> > > wrote: > > > > > > > @Gwen - yes it did come back to per-API versions - I think that > > happened > > > > toward the end of the KIP hangout. I already said this, but > personally > > > I'm > > > > also in favor of per-API versions. A global/aggregate API version > also > > > > works but that is just a proxy to a vector of per-API versions and at > > the > > > > end of the day a client implementation will need to somehow derive > > *some* > > > > per-API version vector to talk to the broker. The other challenges > > (that > > > > I'm sure can be solved) with the global API version are automatically > > > > computing it in the first place or if set manually, automatically > > > verifying > > > > that it was correctly bumped; then there are the nuances of API > > evolution > > > > across multiple branches and thereby the need for some convention to > > > manage > > > > trees of aggregate API versions. > > > > > > > > Jason and Jay had brought up (earlier in this thread) some practical > > > > difficulties in using per-API versions by client implementations > that I > > > > think is worth quoting verbatim to make sure people are aware of the > > > > implications: > > > > > > > > Maybe you could try to capture feature > > > > > compatibility by checking the versions for a subset of request > types? > > > For > > > > > example, to ensure that you can use the new consumer API, you check > > > that > > > > > the group coordinator request is present, the offset commit request > > > > version > > > > > is greater than 2, the offset fetch request is greater than 1, and > > the > > > > join > > > > > group request is present. And to ensure compatibility with KIP-32, > > > maybe > > > > > you only need to check the appropriate versions of the fetch and > > > produce > > > > > requests. That sounds kind of complicated to keep track of and you > > > > probably > > > > > end up trying to handle combinations which aren't even possible in > > > > > practice. > > > > > > > > > > > > > Unless I'm missing something I don't recollect any other significant > > > > drawbacks to per-API versions or are there any? > > > > > > > > For the above concern I think indicating support for certain features > > > > ultimately has to be accomplished one way or the other by documenting > > > what > > > > per-API request version ranges need to be used. This is obviously > more > > > > tedious for more complex client-server interactions such as the new > > > > consumer's rebalance process; but not so much for other features such > > as > > > > KIP-32; or reading throttle time in produce/fetch responses that were > > > > delayed due to quota violations. > > > > > > > > On Thu, Mar 17, 2016 at 1:50 AM, Magnus Edenhill <mag...@edenhill.se > > > > > > wrote: > > > > > > > > > I'm sorry for the confusion but I was referring to a simplified > > version > > > > of > > > > > the KIP > > > > > that I'd failed to document on the wiki yet, my bad! > > > > > > > > > > The now updated KIP leaves the existing MetadataRequest untouched > and > > > > > instead > > > > > adds a specific API to query the broker for supported APIs: > > > > > ApiVersionQueryRequest. > > > > > > > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-35+-+Retrieving+protocol+version > > > > > > > > > > With all the new protocol additions of the upcoming 0.10.0 release > it > > > is > > > > > imperative > > > > > to get this support in now, so let's be constructive, pragmatic and > > > most > > > > > importantly quick > > > > > in commenting on this latest proposal so we can start a voting > thread > > > > ASAP. > > > > > > > > > > Ashish will have the patch ready within a day and librdkafka will > be > > > used > > > > > to verify the functionality, > > > > > we should have a technical green light before the weekend. > Additional > > > > > client devs > > > > > are very welcome to join in. > > > > > > > > > > I'll send a message to kafka-clients to pull in any 3rd party > client > > > devs > > > > > that have missed > > > > > the recent uprise in discussion. > > > > > > > > > > > > > > > /Magnus > > > > > > > > > > 2016-03-17 5:21 GMT+01:00 Ewen Cheslack-Postava <e...@confluent.io > >: > > > > > > > > > > > I'm a fan of separating feature support/versions into a separate > > > > request > > > > > > from metadata -- they're different requests and I don't see how > the > > > > > current > > > > > > model of metadata can properly map to the request version info we > > > need. > > > > > The > > > > > > complexity of support differing across brokers seems like > something > > > > > clients > > > > > > need to handle regardless of any solution we come up with here > > > (current > > > > > > auto detection approaches are vulnerable to similar issues). > > > > > > > > > > > > More importantly -- has this KIP ever made it over to the > > > kafka-clients > > > > > > list, or have we reached out to other client library developers? > My > > > > brief > > > > > > scan of the list suggests it hasn't, but maybe I missed > something. > > > > We've > > > > > > got Magnus & Dana participating, which is great and accounts for > > some > > > > > > really important clients, but have we reached out as broadly as > we > > > can > > > > to > > > > > > other client developers to get feedback? We're largely operating > in > > > an > > > > > echo > > > > > > chamber, and some of the current challenges might trace back to > > that > > > > mode > > > > > > of operation -- the Java client, which has the benefit of > > > development, > > > > > > testing, and release alongside the broker, doesn't follow the > same > > > > > > compatibility and release approach as most other clients. > > > > > > > > > > > > I'm happy to help with that outreach and bring feedback back into > > > this > > > > > > discussion, but I think this will end up being longer process > than > > we > > > > can > > > > > > resolve in the next few days... > > > > > > > > > > > > -Ewen > > > > > > > > > > > > > > > > > > On Wed, Mar 16, 2016 at 5:22 PM, Gwen Shapira <g...@confluent.io > > > > > > wrote: > > > > > > > > > > > > > I was a bit surprised to discover that the latest KIP includes > an > > > > array > > > > > > of > > > > > > > versions per request. Especially since we spent an hour > > discussing > > > > > global > > > > > > > APIs and their tradeoffs. > > > > > > > > > > > > > > I'm pointing this out in case anyone else missed that - to make > > > sure > > > > we > > > > > > are > > > > > > > all on the same page in this discussion. > > > > > > > > > > > > > > Gwen > > > > > > > > > > > > > > On Tue, Mar 15, 2016 at 10:41 PM, Ashish Singh < > > > asi...@cloudera.com> > > > > > > > wrote: > > > > > > > > > > > > > > > Sent incomplete message by mistake. > > > > > > > > > > > > > > > > I do not have a preference for (1) or (2). (1) basically > makes > > us > > > > > view > > > > > > > > supported versions for all brokers as a part of cluster wide > > > state, > > > > > > > however > > > > > > > > (2) lets client deal with individual brokers. From usage > > > > perspective, > > > > > > > > clients would need to choose a certain feature or not based > on > > > > > minimal > > > > > > > > supported version across all brokers they are talking to. In > > (1) > > > > this > > > > > > > state > > > > > > > > will be maintained and provided by server, however in (2) the > > > onus > > > > > will > > > > > > > be > > > > > > > > on client to maintain the state. What do you suggest will be > > the > > > > > right > > > > > > > > thing to do here? > > > > > > > > > > > > > > > > > > > > > > > > On Tue, Mar 15, 2016 at 10:27 PM, Ashish Singh < > > > > asi...@cloudera.com> > > > > > > > > wrote: > > > > > > > > > > > > > > > > > I do not have a preference for 1 or 2. 1 basically makes us > > > view > > > > > > > > supported > > > > > > > > > versions for all brokers > > > > > > > > > > > > > > > > > > On Tuesday, March 15, 2016, Jay Kreps <j...@confluent.io> > > > wrote: > > > > > > > > > > > > > > > > > > > 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 > > > > > > > > > > <javascript:;>> 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 > > > > > > > > > <javascript:;>>: > > > > > > > > > > > > > > > > > > > > > >> 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 > > > > > > > > > > <javascript:;>> 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 > > > > > > > > > > <javascript:;>> > > > > > > > > > > >> 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 <javascript:;> > > > > > > > > > > >> >> > 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 > > > > > > > > > > <javascript:;>> > > > > > > > > > > >> 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 > > > > > > > > > > <javascript:;>> 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 <javascript:;>> > > > > > > > > > > >> >>> > > 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 > > > > > > > > > > <javascript:;>> > > > > > > > > > > >> >>> 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 <javascript:;>> > > > > > > > > > > >> >>> > > wrote: > > > > > > > > > > >> >>> > > >> > On Mon, Mar 14, 2016 at 5:31 PM, Jay > Kreps > > < > > > > > > > > > > j...@confluent.io <javascript:;>> > > > > > > > > > > >> >>> > 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 <javascript:;> > > > > > > > > > > >> >>> > > > > > > > > > > > > > >> >>> > > >> >> 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 <javascript:;> > > > > > > > > > > >> >>> > > > > > > > > > > > > > >> >>> > > >> >> wrote: > > > > > > > > > > >> >>> > > >> >> > > > > > > > > > > > >> >>> > > >> >> > > Hi Jason, > > > > > > > > > > >> >>> > > >> >> > > > > > > > > > > > > >> >>> > > >> >> > > On Mon, Mar 14, 2016 at 4:04 PM, > Jason > > > > > > > Gustafson < > > > > > > > > > > >> >>> > > >> ja...@confluent.io <javascript:;>> > > > > > > > > > > >> >>> > > >> >> > > 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 <javascript:;>> > > > > > > > > > > >> >>> > > >> >> > > wrote: > > > > > > > > > > >> >>> > > >> >> > > > > > > > > > > > > > >> >>> > > >> >> > > > > On Mon, Mar 14, 2016 at 2:12 PM, > > > > Ismael > > > > > > > Juma < > > > > > > > > > > >> >>> > > ism...@juma.me.uk <javascript:;> > > > > > > > > > > >> >>> > > >> > > > > > > > > > > > >> >>> > > >> >> > > wrote: > > > > > > > > > > >> >>> > > >> >> > > > > > > > > > > > > > > >> >>> > > >> >> > > > > > On Mon, Mar 14, 2016 at 8:45 > PM, > > > > Gwen > > > > > > > > Shapira > > > > > > > > > < > > > > > > > > > > >> >>> > > >> g...@confluent.io <javascript:;> > > > > > > > > > > >> >>> > > >> >> > > > > > > > > > > > >> >>> > > >> >> > > > 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 > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > > > Ashish 🎤h > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > > > > > > > > > > Regards, > > > > > > > > Ashish > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > Thanks, > > > > > > Ewen > > > > > > > > > > > > > > > > > > > > >
