My point was only that the way people would probably end up using the request for all versions as a proxy for determining the broker version. For example, if the JoinGroup v0 is present, then it must be 0.9. Or if you do it in terms of features, then you need to check 5 or 6 different request types to verify that the coordination protocol was supported. So a single version seemed simpler and closer to what (I guessed) clients would actually be able to code against. That said, it doesn't prevent clients from having to track a big version matrix (i.e. you still need to know which api versions are supported for each broker version), so maybe it doesn't help as much as I hoped. And as pointed out in the discussion, it doesn't allow backporting, which may or may not be an issue depending on whether we want to support that. If we don't, then I'd say the single version still gets edge in terms of simplicity.
Anyway, I'm not particularly strong in favor of either approach, but I think what's concerning Gwen (and myself) is that we're going down to the wire with a significant decision and the KIP itself seems different every time we look at it. And still a little incomplete. It would be nice, for example, if the rejected alternatives explained why each alternative was rejected. It would also be nice to discuss the way the current client versioning works and why it is inadequate for third-party clients. Thanks, Jason 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 > > > > > > > > > > > > > > >
