Magnus, If we go with release version as protocol version (which I agree is much more user-friendly) - what will be the release version on trunk? 0.10.0-SNAPSHOT? How will clients handle the fact that some 0.10.0-SNAPSHOT will have different protocol than others (because we modify the protocol multiple times between releases)?
Gwen On Thu, Mar 10, 2016 at 1:52 PM, Magnus Edenhill <mag...@edenhill.se> wrote: > Hi all, > > sorry for joining late in the game, the carribean got in the way. > > My thoughts: > > There is no way around the chicken&egg problem, so the sooner we can > add protocol versioning functionality the better and we'll add heuristics > in clients to > handle the migration period (e.g, what Dana has done in kafka-python). > The focus at this point should be to mitigate the core issue (allow clients > to know what is supported) > in the least intrusive way. Hopefully we can redesign the protocol in the > future to add proper > response headers, etc. > > I'm with Data that reusing the broker version as a protocol version will > work just fine and > saves us from administrating another version. > From a client's perspective an explicit protocol version doesn't really add > any value. > I'd rather maintain a mapping of actual broker versions to supported > protocol requests rather than > some independent protocol version that still needs to be translated to a > broker version for > proper code maintainability / error messages / etc. > > > Thus my suggestion is in line with some of the previous speakers, that is > is to keep things > simple and bump the MetadataRequest version to 1 by adding a VersionString > ("0.9.1.0") > and VersionInt (0x00090100) field to the response. > These fields return version information for the current connection's broker > only, not for other broker's > in the cluster: > Providing version information for other brokers doesn't really serve any > purpose: > a) the information is cached by the responding broker so it might be > outdated ( = cant be trusted) > b) by the time the client connects to a given broker it might have upgraded > > This means that a client (that is interested in protocol versioning) will > need to query each > connection's version any way. Since MetadataRequets are typically already > sent on connection set up > this seems to be the proper place to put it. > > The MetadataRequest semantics should also be extended to allow asking only > for cluster and version information, > but not the topic list since this might have negative performance impact on > large clusters with many topics. > One way to achieve this would be to provide one single Null topic in the > request (length=-1). > > Sending a new Metadata V1 request to an old broker will cause the > connection to be closed and > the client will need to use this as a heuristic to downgrade its protocol > ambitions to an older version > (either by some default value or by user configuration). > > > /Magnus > > > 2016-03-10 20:04 GMT+01:00 Ashish Singh <asi...@cloudera.com>: > >> @Magnus, >> >> Does the latest suggestion sound OK to you. I am planning to update PR >> based on latest suggestion. >> >> On Mon, Mar 7, 2016 at 10:58 AM, Ashish Singh <asi...@cloudera.com> wrote: >> >> > >> > >> > On Fri, Mar 4, 2016 at 5:46 PM, Jay Kreps <j...@confluent.io> wrote: >> > >> >> Hey Ashish, >> >> >> >> Both good points. >> >> >> >> I think the issue with the general metadata request is the same as the >> >> issue with a version-specific metadata request from the other >> >> proposal--basically it's a chicken and egg problem, to find out anything >> >> about the cluster you have to be able to communicate something in a >> format >> >> the server can understand without knowing a priori what version it's >> on. I >> >> guess the question is how can you continue to evolve the metadata >> request >> >> (whether it is the existing metadata or a protocol-version specific >> >> metadata request) given that you need this information to bootstrap you >> >> have to be more careful in how that request evolves. >> >> >> > You are correct. It's just that protocol version request would be very >> > specific to retrieve the protocol versions. Changes to protocol version >> > request itself should be very rare, if at all. However, the general >> > metadata request carries a lot more information and its format is more >> > probable to evolve. This boils down to higher probability of change vs a >> > definite network round-trip for each re/connect. It does sound like, it >> is >> > better to avoid a definite penalty than to avoid a probable rare issue. >> > >> >> >> >> I think deprecation/removal may be okay. Ultimately clients will always >> >> use >> >> the highest possible version of the protocol the server supports so if >> >> we've already deprecated and removed your highest version then you are >> >> screwed and you're going to get an error no matter what, right? >> Basically >> >> there is nothing dynamic you can do in that case. >> >> >> > Sure, this should be expected. Just wanted to make sure deprecation is >> > still on the table. >> > >> >> >> >> -Jay >> >> >> >> On Fri, Mar 4, 2016 at 4:05 PM, Ashish Singh <asi...@cloudera.com> >> wrote: >> >> >> >> > Hello Jay, >> >> > >> >> > The overall approach sounds good. I do realize that this discussion >> has >> >> > gotten too lengthy and is starting to shoot tangents. Maybe a KIP call >> >> will >> >> > help us getting to a decision faster. I do have a few questions >> though. >> >> > >> >> > On Fri, Mar 4, 2016 at 9:52 AM, Jay Kreps <j...@confluent.io> wrote: >> >> > >> >> > > Yeah here is my summary of my take: >> >> > > >> >> > > 1. Negotiating a per-connection protocol actually does add a lot of >> >> > > complexity to clients (many more failure states to get right). >> >> > > >> >> > > 2. Having the client configure the protocol version manually is >> doable >> >> > now >> >> > > but probably a worse state. I suspect this will lead to more not >> less >> >> > > confusion. >> >> > > >> >> > > 3. I don't think the current state is actually that bad. Integrators >> >> > pick a >> >> > > conservative version and build against that. There is a tradeoff >> >> between >> >> > > getting the new features and being compatible with old Kafka >> versions. >> >> > But >> >> > > a large part of this tradeoff is essential since new features aren't >> >> > going >> >> > > to magically appear on old servers, so even if you upgrade your >> client >> >> > you >> >> > > likely aren't going to get the new stuff (since we will end up >> >> > dynamically >> >> > > turning it off). Having client features that are there but don't >> work >> >> > > because you're on an old cluster may actually be a worse experience >> if >> >> > not >> >> > > handled very carefully.. >> >> > > >> >> > > 4. The problems Dana brought up are totally orthogonal to the >> problem >> >> of >> >> > > having per-api versions or overall versions. The problem was that we >> >> > > changed behavior subtly without changing the version. This will be >> an >> >> > issue >> >> > > regardless of whether the version is global or not. >> >> > > >> >> > > 5. Using the broker release as the version is strictly worse than >> >> using a >> >> > > global protocol version (0, 1, 2, ...) that increments any time any >> >> api >> >> > > changes but doesn't increment just because non-protocol code is >> >> changed. >> >> > > The problem with using the broker release version is we want to be >> >> able >> >> > to >> >> > > keep Kafka releasable from any commit which means there isn't as >> >> clear a >> >> > > sequencing of releases as you would think. >> >> > > >> >> > > 6. We need to consider the case of mixed version clusters during the >> >> time >> >> > > period when you are upgrading Kafka. >> >> > > >> >> > > So overall I think this is not a critical thing to do right now, but >> >> if >> >> > we >> >> > > are going to do it we should do it in a way that actually improves >> >> > things. >> >> > > >> >> > > Here would be one proposal for that: >> >> > > a. Add a global protocol version that increments with any api >> version >> >> > > update. Move the documentation so that the docs are by version. This >> >> is >> >> > > basically just a short-hand for a complete set of supported api >> >> versions. >> >> > > b. Include a field in the metadata response for each broker that >> adds >> >> the >> >> > > protocol version. >> >> > > >> >> > There might be an issue here where the metadata request version sent >> by >> >> > client is not supported by broker, an older broker. However, if we are >> >> > clearly stating that a client is not guaranteed to work with an older >> >> > broker then this becomes expected. This will potentially limit us in >> >> terms >> >> > of supporting downgrades though, if we ever want to. >> >> > >> >> > > c. To maintain the protocol version this information will have to >> get >> >> > > propagated with the rest of the broker metadata like host, port, id, >> >> etc. >> >> > > >> >> > > The instructions to clients would be: >> >> > > - By default you build against a single conservative Kafka protocol >> >> > version >> >> > > and we carry that support forward, as today >> >> > > >> >> > If I am getting this correct, this will mean we will never >> >> deprecate/remove >> >> > any protocol version in future. Having some way to deprecate/remove >> >> older >> >> > protocol versions will probably be a good idea. It is possible with >> the >> >> > global protocol version approach, it could be as simple as marking a >> >> > protocol deprecated in protocol doc before removing it. Just want to >> >> make >> >> > sure deprecation is still on the table. >> >> > >> >> > > - If you want to get fancy you can use the protocol version field in >> >> the >> >> > > metadata request to more dynamically chose what features are >> available >> >> > and >> >> > > select api versions appropriately. This is purely optional. >> >> > > >> >> > > -Jay >> >> > > >> >> > > On Thu, Mar 3, 2016 at 9:38 PM, Jason Gustafson <ja...@confluent.io >> > >> >> > > wrote: >> >> > > >> >> > > > I talked with Jay about this KIP briefly this morning, so let me >> >> try to >> >> > > > summarize the discussion (I'm sure he'll jump in if I get anything >> >> > > wrong). >> >> > > > Apologies in advance for the length. >> >> > > > >> >> > > > I think we both share some skepticism that a request with all the >> >> > > supported >> >> > > > versions of all the request APIs is going to be a useful primitive >> >> to >> >> > try >> >> > > > and build client compatibility around. In practice I think people >> >> would >> >> > > end >> >> > > > up checking for particular request versions in order to determine >> if >> >> > the >> >> > > > broker is 0.8 or 0.9 or whatever, and then change behavior >> >> accordingly. >> >> > > I'm >> >> > > > wondering if there's a reasonable way to handle the version >> >> responses >> >> > > that >> >> > > > doesn't amount to that. 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. >> >> > > > >> >> > > > The alternative is to use a single API version. It could be the >> >> Kafka >> >> > > > release version, but then you need to figure out how to handle >> users >> >> > who >> >> > > > are running off of trunk since multiple API versions will >> typically >> >> > > change >> >> > > > between releases. Perhaps it makes more sense to keep a separate >> API >> >> > > > version number which is incremented every time any one of the API >> >> > > versions >> >> > > > increases? This also decouples the protocol from the Kafka >> >> > distribution. >> >> > > > >> >> > > > As far as whether there should be a separate request or not, I get >> >> > > Becket's >> >> > > > point that you would only need to do the version check once when a >> >> > > > connection is established, but another round trip still >> complicates >> >> the >> >> > > > picture quite a bit. Before you just need to send a metadata >> >> request to >> >> > > > bootstrap yourself to the cluster, but now you need to do version >> >> > > > negotiation before you can even do that, and then you need to try >> >> adapt >> >> > > to >> >> > > > the versions reported. Jay brought up the point that you probably >> >> > > wouldn't >> >> > > > design a protocol from scratch to work this way. Using the >> metadata >> >> > > request >> >> > > > would be better if it's possible, but you need a way to handle the >> >> fact >> >> > > > that a broker's version might be stale by the time you connect to >> >> it. >> >> > And >> >> > > > even then you're going to have to deal internally with the >> >> complexity >> >> > > > involved in trying to upgrade/downgrade dynamically, which sounds >> >> to me >> >> > > > like it would have a ton of edge cases. >> >> > > > >> >> > > > Taking a bit of a step back, any solution is probably going to be >> >> > painful >> >> > > > since the Kafka protocol was not designed for this use case. >> >> Currently >> >> > > what >> >> > > > that means for clients that /want/ to support compatibility across >> >> > broker >> >> > > > versions is that they need to have the user tell them the broker >> >> > version >> >> > > > through configuration (e.g. librdkafka has a "protocol.version" >> >> field >> >> > for >> >> > > > this purpose). The only real problem with this in my mind is that >> we >> >> > > don't >> >> > > > have a graceful way to detect request incompatibility, which is >> why >> >> > there >> >> > > > are so many questions on the user list which basically amount to >> the >> >> > > client >> >> > > > hanging because the broker refuses to respond to a request it >> >> doesn't >> >> > > > understand. If you solve this problem, then depending on >> >> configuration >> >> > > > seems totally reasonable and we can skip trying to implement >> request >> >> > > > version negotiation. Magnus's solution in this KIP may seem a >> little >> >> > > hacky, >> >> > > > but it also seems like the only way to do it without changing the >> >> > header. >> >> > > > >> >> > > > The Spark problem mentioned above is interesting and I agree that >> it >> >> > > sucks >> >> > > > for frameworks that need to ship the kafka client library since >> they >> >> > have >> >> > > > to figure out how to bundle multiple versions. Ultimately if we >> >> want to >> >> > > > solve this problem, then it sounds like we need to commit to >> >> > maintaining >> >> > > > compatibility with older versions of Kafka in the client going >> >> forward. >> >> > > > That's a lot bigger decision and it matters less whether the >> broker >> >> > > version >> >> > > > is found through configuration, topic metadata, or a new request >> >> type. >> >> > > > >> >> > > > -Jason >> >> > > > >> >> > > > >> >> > > > On Thu, Mar 3, 2016 at 3:59 PM, Becket Qin <becket....@gmail.com> >> >> > wrote: >> >> > > > >> >> > > > > Hi Ashish, >> >> > > > > >> >> > > > > In approach (1), the clients will still be able to talked to >> >> multiple >> >> > > > > versions of Kafka brokers as long as the clients version is not >> >> > higher >> >> > > > than >> >> > > > > the broker version, right? >> >> > > > > >> >> > > > > From Spark's point of view, it seems the difference is whether >> >> Spark >> >> > > can >> >> > > > > independently update their Kafka clients dependency or not. More >> >> > > > > specifically, consider the following three scenarios: >> >> > > > > A. Spark has some new features that do not rely on clients or >> >> brokers >> >> > > in >> >> > > > a >> >> > > > > new Kafka release. >> >> > > > > B. Spark has some new features that only rely on the clients in >> a >> >> new >> >> > > > Kafka >> >> > > > > release, but not rely on the brokers in a new Kafka release. >> e.g. >> >> New >> >> > > > > client provides a listTopic() method. >> >> > > > > C. Spark has some new features that rely on both the clients and >> >> > > brokers >> >> > > > in >> >> > > > > a new Kafka release. e.g timestamp field. >> >> > > > > >> >> > > > > For A, Spark does not need to update the Kafka dependency >> because >> >> > there >> >> > > > is >> >> > > > > no need and the old clients can talk to both new and old Kafka >> >> > brokers. >> >> > > > > For C, Spark has to wait for broker upgrade anyways. >> >> > > > > So in the above two scenarios, there is not much difference >> >> between >> >> > > > > approach (1) and (2). >> >> > > > > >> >> > > > > B is a tricky scenario. Because it is possible that we introduce >> >> both >> >> > > > > listTopic() and the timestamp field in the same Kafka release, >> >> and we >> >> > > > don't >> >> > > > > know if Spark needs both or only uses listTopic(). >> >> > > > > This indicates the client should work fine if a method is >> >> supported >> >> > and >> >> > > > > should throw exception when a method is not supported. I think >> we >> >> can >> >> > > do >> >> > > > > the following: >> >> > > > > 0. Clients always use its highest request version. The clients >> >> keeps >> >> > a >> >> > > > > static final map recording the minimum required ApiVersion for >> >> each >> >> > > > > request. >> >> > > > > 1. When connect to a broker, the clients always send an >> >> > > ApiVersionRequest >> >> > > > > to the broker. >> >> > > > > 2. The broker replies with the its highest supported ApiVersion. >> >> > > > > 3. Before sending a request, the clients checks the minimum >> >> required >> >> > > > > ApiVersion for that request. If the broker returned ApiVersion >> is >> >> > > higher >> >> > > > > than this minimum required ApiVersion, then we can proceed. >> >> Otherwise >> >> > > we >> >> > > > > throw something like NotSupportedOperationException. >> >> > > > > >> >> > > > > With this approach, scenario B will also work unless Spark calls >> >> some >> >> > > > > function that is not supported by the Kafka broker, which makes >> it >> >> > > become >> >> > > > > scenario C. >> >> > > > > >> >> > > > > Thoughts? >> >> > > > > >> >> > > > > Thanks, >> >> > > > > >> >> > > > > Jiangjie (Becket) Qin >> >> > > > > >> >> > > > > On Thu, Mar 3, 2016 at 11:24 AM, Ashish Singh < >> >> asi...@cloudera.com> >> >> > > > wrote: >> >> > > > > >> >> > > > > > On Wed, Mar 2, 2016 at 8:29 PM, Becket Qin < >> >> becket....@gmail.com> >> >> > > > wrote: >> >> > > > > > >> >> > > > > > > Hi Jason, >> >> > > > > > > >> >> > > > > > > I was thinking that every time when we connect to a broker, >> we >> >> > > first >> >> > > > > send >> >> > > > > > > the version check request. (The version check request itself >> >> > should >> >> > > > be >> >> > > > > > very >> >> > > > > > > simple and never changes across all server releases.) This >> >> does >> >> > add >> >> > > > an >> >> > > > > > > additional round trip, but given reconnect is rare, it is >> >> > probably >> >> > > > > fine. >> >> > > > > > On >> >> > > > > > > the client side, the client will always send request using >> the >> >> > > lowest >> >> > > > > > > supported version across all brokers. That means if a Kafka >> >> > cluster >> >> > > > is >> >> > > > > > > downgrading, we will use the downgraded protocol as soon as >> >> the >> >> > > > client >> >> > > > > > > connected to an older broker. >> >> > > > > > > >> >> > > > > > This sounds interesting and very similar to current >> suggestion. >> >> > > > However, >> >> > > > > > just to make sure I am getting it right, you are suggesting >> >> send a >> >> > > > > separate >> >> > > > > > request only for release version? >> >> > > > > > >> >> > > > > > > >> >> > > > > > > @Ashish, >> >> > > > > > > Can you help me understand the pain points from other open >> >> source >> >> > > > > > projects >> >> > > > > > > that you mentioned a little more? There are two different >> >> levels >> >> > of >> >> > > > > > > requirements: >> >> > > > > > > 1. User wants to know if the client is compatible with the >> >> broker >> >> > > or >> >> > > > > not. >> >> > > > > > > 2. User wants the client and the broker to negotiate the >> >> protocol >> >> > > on >> >> > > > > > their >> >> > > > > > > own. >> >> > > > > > > >> >> > > > > > Not sure which category it falls in, but below is the excerpt >> >> from >> >> > > > Mark, >> >> > > > > a >> >> > > > > > spark dev, who has been trying to upgrade spark kafka >> >> integration >> >> > to >> >> > > > use >> >> > > > > > 0.9 clients. >> >> > > > > > >> >> > > > > > Based on what I understand, users of Kafka need to upgrade >> their >> >> > > > brokers >> >> > > > > to >> >> > > > > > Kafka 0.9.x first, before they upgrade their clients to Kafka >> >> > 0.9.x. >> >> > > > > > >> >> > > > > > > However, that presents a problem to other projects that >> >> integrate >> >> > > > with >> >> > > > > > Kafka (Spark, Flume, Storm, etc.). From here on, I will speak >> >> for >> >> > > > Spark + >> >> > > > > > Kafka, since that's the one I am most familiar with. >> >> > > > > > In the light of compatibility (or the lack thereof) between >> >> 0.8.x >> >> > and >> >> > > > > > 0.9.x, Spark is faced with a problem of what version(s) of >> >> Kafka to >> >> > > be >> >> > > > > > compatible with, and has 2 options (discussed in this PR >> >> > > > > > <https://github.com/apache/spark/pull/11143>): >> >> > > > > > 1. We either upgrade to Kafka 0.9, dropping support for 0.8. >> >> Storm >> >> > > and >> >> > > > > > Flume are already on this path. >> >> > > > > > 2. We introduce complexity in our code to support both 0.8 and >> >> 0.9 >> >> > > for >> >> > > > > the >> >> > > > > > entire duration of our next major release (Apache Spark 2.x). >> >> > > > > > I'd love to hear your thoughts on which option, you recommend. >> >> > > > > > Long term, I'd really appreciate if Kafka could do something >> >> that >> >> > > > doesn't >> >> > > > > > make Spark having to support two, or even more versions of >> >> Kafka. >> >> > > And, >> >> > > > if >> >> > > > > > there is something that I, personally, and Spark project can >> do >> >> in >> >> > > your >> >> > > > > > next release candidate phase to make things easier, please do >> >> let >> >> > us >> >> > > > > know. >> >> > > > > > >> >> > > > > > This issue has made other projects worry about how they are >> >> going >> >> > to >> >> > > > keep >> >> > > > > > up with Kafka releases. Last I heard, take this with a pinch >> of >> >> > salt, >> >> > > > > Spark >> >> > > > > > folks are discussing about using Maven profiles to build >> against >> >> > > > multiple >> >> > > > > > Kafka versions at compile time, etc. Also, there are clients >> who >> >> > are >> >> > > > > > relying on class-loading tricks with custom implementation of >> >> OSGi >> >> > to >> >> > > > > solve >> >> > > > > > such issues. Don't quote me on the stuff I just mentioned, as >> >> this >> >> > is >> >> > > > > what >> >> > > > > > I have heard during casual discussions. The point I am trying >> to >> >> > make >> >> > > > is >> >> > > > > > that Kafka clients are worried about being able to support >> >> multiple >> >> > > > Kafka >> >> > > > > > broker versions. I am sure we all agree on that. >> >> > > > > > >> >> > > > > > I think the second requirement makes more sense from a client >> >> > > > > perspective. >> >> > > > > > First req will just tell them that there is a problem, but no >> >> way >> >> > to >> >> > > > work >> >> > > > > > around it. >> >> > > > > > >> >> > > > > > > >> >> > > > > > > Currently in Kafka the principle we are following is to let >> >> > clients >> >> > > > > stick >> >> > > > > > > to a certain version and server will adapt to the clients >> >> > > > accordingly. >> >> > > > > > > If this KIP doesn't want to break this rule, it seems we >> >> should >> >> > > > simply >> >> > > > > > let >> >> > > > > > > the clients send the ApiVersion it is using to the brokers >> and >> >> > the >> >> > > > > > brokers >> >> > > > > > > will decide whether to accept or reject the clients. This >> >> means >> >> > > user >> >> > > > > have >> >> > > > > > > to upgrade broker before they upgrade clients. This >> satisfies >> >> (1) >> >> > > so >> >> > > > > > that a >> >> > > > > > > newer client will know it does not compatible with an older >> >> > server >> >> > > > > > > immediately. >> >> > > > > > > If this KIP will change that to let the newer clients adapt >> to >> >> > the >> >> > > > > older >> >> > > > > > > brokers, compatibility wise it is a good thing to have. >> With >> >> > this >> >> > > > now >> >> > > > > > > users are able to upgrade clients before they upgrade Kafka >> >> > > brokers. >> >> > > > > This >> >> > > > > > > means user can upgrade clients even before upgrade servers. >> >> This >> >> > > > > > satisfies >> >> > > > > > > (2) as the newer clients can also talk to the older servers. >> >> > > > > > > >> >> > > > > > More importantly, this will allow a client to talk to multiple >> >> > > versions >> >> > > > > of >> >> > > > > > Kafka. >> >> > > > > >> >> > > > > >> >> > > > > > > >> >> > > > > > > If we decide to go with (2). The benefit is that a newer >> >> client >> >> > > won't >> >> > > > > > break >> >> > > > > > > when talking to an older broker. But functionality wise, it >> >> might >> >> > > be >> >> > > > > the >> >> > > > > > > same as an older clients. >> >> > > > > > > In the downgrading case, we probably still have to notify >> all >> >> the >> >> > > > > users. >> >> > > > > > > For example, if application is sending messages with >> timestamp >> >> > and >> >> > > > the >> >> > > > > > > broker got downgraded to an older version that does not >> >> support >> >> > > > > > timestamp. >> >> > > > > > > The clients will suddenly start to throw away timestamps. >> This >> >> > > might >> >> > > > > > affect >> >> > > > > > > the application logic. In this case even if we have clients >> >> > > > > automatically >> >> > > > > > > adapted to a lower version broker, the applications might >> >> still >> >> > > > break. >> >> > > > > > > Hence we still need to notify the users about the case when >> >> the >> >> > > > clients >> >> > > > > > is >> >> > > > > > > newer than the brokers. This is the same for both (1) and >> (2). >> >> > > > > > > Supporting (2) will introduce more complication on the >> client >> >> > side. >> >> > > > And >> >> > > > > > we >> >> > > > > > > may also have to communicate with users about what function >> is >> >> > > > > supported >> >> > > > > > in >> >> > > > > > > the new clients and what is not supported after the protocol >> >> > > > > negotiation >> >> > > > > > > finishes. >> >> > > > > > > >> >> > > > > > Totally agreed, however only if clients want to support >> multiple >> >> > > broker >> >> > > > > > versions. If they want to, then I am sure they are willing to >> >> add >> >> > > some >> >> > > > > > logic on their end. >> >> > > > > > >> >> > > > > > > >> >> > > > > > > Thanks, >> >> > > > > > > >> >> > > > > > > Jiangjie (Becket) Qin >> >> > > > > > > >> >> > > > > > > >> >> > > > > > > >> >> > > > > > > >> >> > > > > > > On Wed, Mar 2, 2016 at 5:58 PM, Dana Powers < >> >> > dana.pow...@gmail.com >> >> > > > >> >> > > > > > wrote: >> >> > > > > > > >> >> > > > > > > > In kafka-python we've been doing something like: >> >> > > > > > > > >> >> > > > > > > > if version >= (0, 9): >> >> > > > > > > > Do cool new stuff >> >> > > > > > > > elif version >= (0, 8, 2): >> >> > > > > > > > Do some older stuff >> >> > > > > > > > .... >> >> > > > > > > > else: >> >> > > > > > > > raise UnsupportedVersionError >> >> > > > > > > > >> >> > > > > > > > This will break if / when the new 0.9 apis are completely >> >> > removed >> >> > > > > from >> >> > > > > > > some >> >> > > > > > > > future release, but should handle intermediate broker >> >> upgrades. >> >> > > > > Because >> >> > > > > > > we >> >> > > > > > > > can't add support for future apis a priori, I think the >> >> best we >> >> > > > could >> >> > > > > > do >> >> > > > > > > > here is throw an error that request protocol version X is >> >> not >> >> > > > > > supported. >> >> > > > > > > > For now that comes through as a broken socket connection, >> so >> >> > > there >> >> > > > is >> >> > > > > > an >> >> > > > > > > > error - just not a super helpful one. >> >> > > > > > > > >> >> > > > > > > > For that reason I'm also in favor of a generic error >> >> response >> >> > > when >> >> > > > a >> >> > > > > > > > protocol req is not recognized. >> >> > > > > > > > >> >> > > > > > > > -Dana >> >> > > > > > > > On Mar 2, 2016 5:38 PM, "Jay Kreps" <j...@confluent.io> >> >> wrote: >> >> > > > > > > > >> >> > > > > > > > > But won't it be the case that what clients end up doing >> >> would >> >> > > be >> >> > > > > > > > something >> >> > > > > > > > > like >> >> > > > > > > > > if(version != 0.8.1) >> >> > > > > > > > > throw new UnsupportedVersionException() >> >> > > > > > > > > which then means the client is broken as soon as we >> >> release a >> >> > > new >> >> > > > > > > server >> >> > > > > > > > > version even though the protocol didn't change. I'm >> >> actually >> >> > > not >> >> > > > > sure >> >> > > > > > > how >> >> > > > > > > > > you could use that information in a forward compatible >> way >> >> > > since >> >> > > > > you >> >> > > > > > > > can't >> >> > > > > > > > > know a priori if you will work with the next release >> until >> >> > you >> >> > > > know >> >> > > > > > if >> >> > > > > > > > the >> >> > > > > > > > > protocol changed. >> >> > > > > > > > > >> >> > > > > > > > > -Jay >> >> > > > > > > > > >> >> > > > > > > > > On Wed, Mar 2, 2016 at 5:28 PM, Jason Gustafson < >> >> > > > > ja...@confluent.io> >> >> > > > > > > > > wrote: >> >> > > > > > > > > >> >> > > > > > > > > > Hey Jay, >> >> > > > > > > > > > >> >> > > > > > > > > > Yeah, I wasn't suggesting that we eliminate request >> API >> >> > > > versions. >> >> > > > > > > > They're >> >> > > > > > > > > > definitely needed on the broker to support >> >> compatibility. I >> >> > > was >> >> > > > > > just >> >> > > > > > > > > saying >> >> > > > > > > > > > that if a client wants to support multiple broker >> >> versions >> >> > > > (e.g. >> >> > > > > > 0.8 >> >> > > > > > > > and >> >> > > > > > > > > > 0.9), then it makes more sense to me to make the kafka >> >> > > release >> >> > > > > > > version >> >> > > > > > > > > > available in order to determine which version of the >> >> > request >> >> > > > API >> >> > > > > > > should >> >> > > > > > > > > be >> >> > > > > > > > > > used rather than adding a new request type which >> exposes >> >> > all >> >> > > of >> >> > > > > the >> >> > > > > > > > > > different supported versions for all of the request >> >> types. >> >> > > > > Request >> >> > > > > > > API >> >> > > > > > > > > > versions all change in lockstep with Kafka releases >> >> anyway. >> >> > > > > > > > > > >> >> > > > > > > > > > -Jason >> >> > > > > > > > > > >> >> > > > > > > > > > On Wed, Mar 2, 2016 at 5:15 PM, Becket Qin < >> >> > > > becket....@gmail.com >> >> > > > > > >> >> > > > > > > > wrote: >> >> > > > > > > > > > >> >> > > > > > > > > > > I think using Kafka release version makes sense. >> More >> >> > > > > > particularly, >> >> > > > > > > > we >> >> > > > > > > > > > can >> >> > > > > > > > > > > use the ApiVersion and this will cover all the >> >> interval >> >> > > > version >> >> > > > > > as >> >> > > > > > > > > well. >> >> > > > > > > > > > In >> >> > > > > > > > > > > KAFKA-3025, we added the ApiVersion to message >> format >> >> > > version >> >> > > > > > > > mapping, >> >> > > > > > > > > We >> >> > > > > > > > > > > can add the ApiKey to version mapping to ApiVersion >> as >> >> > > well. >> >> > > > We >> >> > > > > > can >> >> > > > > > > > > move >> >> > > > > > > > > > > ApiVersion class to o.a.k.c package and use it for >> >> both >> >> > > > server >> >> > > > > > and >> >> > > > > > > > > > clients. >> >> > > > > > > > > > > >> >> > > > > > > > > > > @Jason, if we cache the release info in metadata and >> >> not >> >> > > > > > > re-validate >> >> > > > > > > > > the >> >> > > > > > > > > > > release on reconnect, would it still work if we do a >> >> > > rolling >> >> > > > > > > > downgrade? >> >> > > > > > > > > > > >> >> > > > > > > > > > > On Wed, Mar 2, 2016 at 3:16 PM, Jason Gustafson < >> >> > > > > > > ja...@confluent.io> >> >> > > > > > > > > > > wrote: >> >> > > > > > > > > > > >> >> > > > > > > > > > > > I think Dana's suggestion to include the Kafka >> >> release >> >> > > > > version >> >> > > > > > > > makes >> >> > > > > > > > > a >> >> > > > > > > > > > > lot >> >> > > > > > > > > > > > of sense. I'm actually wondering why you would >> need >> >> the >> >> > > > > > > individual >> >> > > > > > > > > API >> >> > > > > > > > > > > > versions if you have that? It sounds like keeping >> >> track >> >> > > of >> >> > > > > all >> >> > > > > > > the >> >> > > > > > > > > api >> >> > > > > > > > > > > > version information would add a lot of complexity >> to >> >> > > > clients >> >> > > > > > > since >> >> > > > > > > > > > > they'll >> >> > > > > > > > > > > > have to try to handle different version >> permutations >> >> > > which >> >> > > > > are >> >> > > > > > > not >> >> > > > > > > > > > > actually >> >> > > > > > > > > > > > possible in practice. Wouldn't it be simpler to >> know >> >> > that >> >> > > > > > you're >> >> > > > > > > > > > talking >> >> > > > > > > > > > > to >> >> > > > > > > > > > > > an 0.9 broker than that you're talking to a broker >> >> > which >> >> > > > > > supports >> >> > > > > > > > > > > version 2 >> >> > > > > > > > > > > > of the group coordinator request, version 1 of >> fetch >> >> > > > request, >> >> > > > > > > etc? >> >> > > > > > > > > > Also, >> >> > > > > > > > > > > > the release version could be included in the >> broker >> >> > > > > information >> >> > > > > > > in >> >> > > > > > > > > the >> >> > > > > > > > > > > > topic metadata request which would save the need >> for >> >> > the >> >> > > > > > > additional >> >> > > > > > > > > > round >> >> > > > > > > > > > > > trip on every reconnect. >> >> > > > > > > > > > > > >> >> > > > > > > > > > > > -Jason >> >> > > > > > > > > > > > >> >> > > > > > > > > > > > On Tue, Mar 1, 2016 at 7:59 PM, Ashish Singh < >> >> > > > > > > asi...@cloudera.com> >> >> > > > > > > > > > > wrote: >> >> > > > > > > > > > > > >> >> > > > > > > > > > > > > On Tue, Mar 1, 2016 at 6:30 PM, Gwen Shapira < >> >> > > > > > > g...@confluent.io> >> >> > > > > > > > > > > wrote: >> >> > > > > > > > > > > > > >> >> > > > > > > > > > > > > > One more thing, the KIP actually had 3 parts: >> >> > > > > > > > > > > > > > 1. The version protocol >> >> > > > > > > > > > > > > > 2. New response on messages of wrong API key >> or >> >> > wrong >> >> > > > > > version >> >> > > > > > > > > > > > > > 3. Protocol documentation >> >> > > > > > > > > > > > > > >> >> > > > > > > > > > > > > There is a WIP patch for adding protocol docs, >> >> > > > > > > > > > > > > https://github.com/apache/kafka/pull/970 . By >> >> > protocol >> >> > > > > > > > > > documentation, >> >> > > > > > > > > > > > you >> >> > > > > > > > > > > > > mean updating this, right? >> >> > > > > > > > > > > > > >> >> > > > > > > > > > > > > > >> >> > > > > > > > > > > > > > I understand that you are offering to only >> >> > implement >> >> > > > part >> >> > > > > > 1? >> >> > > > > > > > > > > > > > But the KIP discussion and vote should still >> >> cover >> >> > > all >> >> > > > > > three >> >> > > > > > > > > parts, >> >> > > > > > > > > > > > > > they will just be implemented in separate >> JIRA? >> >> > > > > > > > > > > > > > >> >> > > > > > > > > > > > > The patch for KAFKA-3307, >> >> > > > > > > > https://github.com/apache/kafka/pull/986 >> >> > > > > > > > > , >> >> > > > > > > > > > > > covers >> >> > > > > > > > > > > > > 1 and 2. KAFKA-3309 tracks documentation part. >> >> Yes, >> >> > we >> >> > > > > should >> >> > > > > > > > > include >> >> > > > > > > > > > > all >> >> > > > > > > > > > > > > the three points you mentioned while discussing >> or >> >> > > voting >> >> > > > > for >> >> > > > > > > > > KIP-35. >> >> > > > > > > > > > > > > >> >> > > > > > > > > > > > > > >> >> > > > > > > > > > > > > > On Tue, Mar 1, 2016 at 5:25 PM, Ashish Singh < >> >> > > > > > > > > asi...@cloudera.com> >> >> > > > > > > > > > > > > wrote: >> >> > > > > > > > > > > > > > > On Tue, Mar 1, 2016 at 5:11 PM, Gwen >> Shapira < >> >> > > > > > > > > g...@confluent.io> >> >> > > > > > > > > > > > > wrote: >> >> > > > > > > > > > > > > > > >> >> > > > > > > > > > > > > > >> I don't see a use for the name - clients >> >> should >> >> > be >> >> > > > > able >> >> > > > > > to >> >> > > > > > > > > > > translate >> >> > > > > > > > > > > > > > >> ApiKey to name for any API they support, >> and >> >> I'm >> >> > > not >> >> > > > > > sure >> >> > > > > > > > why >> >> > > > > > > > > > > would >> >> > > > > > > > > > > > a >> >> > > > > > > > > > > > > > >> client need to log anything about APIs it >> >> does >> >> > not >> >> > > > > > > support. >> >> > > > > > > > > Am I >> >> > > > > > > > > > > > > > >> missing something? >> >> > > > > > > > > > > > > > >> >> >> > > > > > > > > > > > > > > Yea, it is a fair assumption that client >> would >> >> > know >> >> > > > > about >> >> > > > > > > > APIs >> >> > > > > > > > > it >> >> > > > > > > > > > > > > > supports. >> >> > > > > > > > > > > > > > > It could have been helpful for client users >> to >> >> > see >> >> > > > new >> >> > > > > > APIs >> >> > > > > > > > > > though, >> >> > > > > > > > > > > > > > however >> >> > > > > > > > > > > > > > > users can always refer to protocol doc of >> new >> >> > > version >> >> > > > > to >> >> > > > > > > find >> >> > > > > > > > > > > > > > corresponding >> >> > > > > > > > > > > > > > > names of the new APIs. >> >> > > > > > > > > > > > > > > >> >> > > > > > > > > > > > > > >> >> >> > > > > > > > > > > > > > >> On a related note, Magnus is currently on >> >> > > vacation, >> >> > > > > but >> >> > > > > > he >> >> > > > > > > > > > should >> >> > > > > > > > > > > be >> >> > > > > > > > > > > > > > >> back at the end of next week. I'd like to >> >> hold >> >> > off >> >> > > > on >> >> > > > > > the >> >> > > > > > > > vote >> >> > > > > > > > > > > until >> >> > > > > > > > > > > > > > >> he gets back since his experience in >> >> > implementing >> >> > > > > > clients >> >> > > > > > > > and >> >> > > > > > > > > > his >> >> > > > > > > > > > > > > > >> opinions will be very valuable for this >> >> > > discussion. >> >> > > > > > > > > > > > > > >> >> >> > > > > > > > > > > > > > > That is great. It will be valuable to have >> his >> >> > > > > feedback. >> >> > > > > > I >> >> > > > > > > > will >> >> > > > > > > > > > > hold >> >> > > > > > > > > > > > > off >> >> > > > > > > > > > > > > > on >> >> > > > > > > > > > > > > > > removing "api_name" and >> >> "api_deprecated_versions" >> >> > > or >> >> > > > > > adding >> >> > > > > > > > > > release >> >> > > > > > > > > > > > > > version. >> >> > > > > > > > > > > > > > > >> >> > > > > > > > > > > > > > >> >> >> > > > > > > > > > > > > > >> Gwen >> >> > > > > > > > > > > > > > >> >> >> > > > > > > > > > > > > > >> On Tue, Mar 1, 2016 at 4:24 PM, Ashish >> Singh >> >> < >> >> > > > > > > > > > asi...@cloudera.com >> >> > > > > > > > > > > > >> >> > > > > > > > > > > > > > wrote: >> >> > > > > > > > > > > > > > >> > Works with me. I will update PR to remove >> >> > this. >> >> > > > > > > > > > > > > > >> > >> >> > > > > > > > > > > > > > >> > Also, "api_name" have been pointed out >> as a >> >> > > > concern. >> >> > > > > > > > > However, >> >> > > > > > > > > > it >> >> > > > > > > > > > > > can >> >> > > > > > > > > > > > > > be >> >> > > > > > > > > > > > > > >> > handy for logging and similar purposes. >> Any >> >> > take >> >> > > > on >> >> > > > > > > that? >> >> > > > > > > > > > > > > > >> > >> >> > > > > > > > > > > > > > >> > On Tue, Mar 1, 2016 at 3:46 PM, Gwen >> >> Shapira < >> >> > > > > > > > > > g...@confluent.io >> >> > > > > > > > > > > > >> >> > > > > > > > > > > > > > wrote: >> >> > > > > > > > > > > > > > >> > >> >> > > > > > > > > > > > > > >> >> Jay also mentioned: >> >> > > > > > > > > > > > > > >> >> "Or, alternately, since deprecation has >> no >> >> > > > > functional >> >> > > > > > > > > impact >> >> > > > > > > > > > > and >> >> > > > > > > > > > > > is >> >> > > > > > > > > > > > > > >> >> just a message >> >> > > > > > > > > > > > > > >> >> to developers, we could just leave it >> out >> >> of >> >> > > the >> >> > > > > > > protocol >> >> > > > > > > > > and >> >> > > > > > > > > > > > just >> >> > > > > > > > > > > > > > have >> >> > > > > > > > > > > > > > >> it >> >> > > > > > > > > > > > > > >> >> in release notes etc." >> >> > > > > > > > > > > > > > >> >> >> >> > > > > > > > > > > > > > >> >> I'm in favor of leaving it out of the >> >> > > protocol. I >> >> > > > > > can't >> >> > > > > > > > > > really >> >> > > > > > > > > > > > see >> >> > > > > > > > > > > > > a >> >> > > > > > > > > > > > > > >> >> use-case. >> >> > > > > > > > > > > > > > >> >> >> >> > > > > > > > > > > > > > >> >> Gwen >> >> > > > > > > > > > > > > > >> >> >> >> > > > > > > > > > > > > > >> >> On Mon, Feb 29, 2016 at 3:55 PM, Ashish >> >> > Singh < >> >> > > > > > > > > > > > asi...@cloudera.com >> >> > > > > > > > > > > > > > >> >> > > > > > > > > > > > > > >> wrote: >> >> > > > > > > > > > > > > > >> >> >> >> > > > > > > > > > > > > > >> >> > I hope it is OK for me to make some >> >> > progress >> >> > > > > here. >> >> > > > > > I >> >> > > > > > > > have >> >> > > > > > > > > > > made >> >> > > > > > > > > > > > > the >> >> > > > > > > > > > > > > > >> >> > following changes. >> >> > > > > > > > > > > > > > >> >> > >> >> > > > > > > > > > > > > > >> >> > 1. Updated KIP-35, to adopt Jay's >> >> > suggestion >> >> > > on >> >> > > > > > > > > maintaining >> >> > > > > > > > > > > > > > separate >> >> > > > > > > > > > > > > > >> list >> >> > > > > > > > > > > > > > >> >> > of deprecated versions, instead of >> >> using a >> >> > > > > version >> >> > > > > > of >> >> > > > > > > > -1. >> >> > > > > > > > > > > > > > >> >> > 2. Added information on required >> >> > permissions, >> >> > > > > > > Describe >> >> > > > > > > > > > action >> >> > > > > > > > > > > > on >> >> > > > > > > > > > > > > > >> Cluster >> >> > > > > > > > > > > > > > >> >> > resource, to be able to retrieve >> >> protocol >> >> > > > > versions >> >> > > > > > > > from a >> >> > > > > > > > > > > auth >> >> > > > > > > > > > > > > > enabled >> >> > > > > > > > > > > > > > >> >> > Kafka cluster. >> >> > > > > > > > > > > > > > >> >> > >> >> > > > > > > > > > > > > > >> >> > Created >> >> > > > > > > > https://issues.apache.org/jira/browse/KAFKA-3304 >> >> > > > > > > > > . >> >> > > > > > > > > > > > > Primary >> >> > > > > > > > > > > > > > >> patch >> >> > > > > > > > > > > > > > >> >> is >> >> > > > > > > > > > > > > > >> >> > available to review, >> >> > > > > > > > > > > https://github.com/apache/kafka/pull/986 >> >> > > > > > > > > > > > > > >> >> > >> >> > > > > > > > > > > > > > >> >> > On Thu, Feb 25, 2016 at 1:27 PM, >> Ashish >> >> > > Singh < >> >> > > > > > > > > > > > > asi...@cloudera.com >> >> > > > > > > > > > > > > > > >> >> > > > > > > > > > > > > > >> >> wrote: >> >> > > > > > > > > > > > > > >> >> > >> >> > > > > > > > > > > > > > >> >> > > Kafka clients in Hadoop ecosystem, >> >> Flume, >> >> > > > > Spark, >> >> > > > > > > etc, >> >> > > > > > > > > > have >> >> > > > > > > > > > > > > found >> >> > > > > > > > > > > > > > it >> >> > > > > > > > > > > > > > >> >> > really >> >> > > > > > > > > > > > > > >> >> > > difficult to cope up with Kafka >> >> releases >> >> > as >> >> > > > > they >> >> > > > > > > want >> >> > > > > > > > > to >> >> > > > > > > > > > > > > support >> >> > > > > > > > > > > > > > >> users >> >> > > > > > > > > > > > > > >> >> on >> >> > > > > > > > > > > > > > >> >> > > different Kafka versions. Capability >> >> to >> >> > > > > retrieve >> >> > > > > > > > > protocol >> >> > > > > > > > > > > > > version >> >> > > > > > > > > > > > > > >> will >> >> > > > > > > > > > > > > > >> >> > go a >> >> > > > > > > > > > > > > > >> >> > > long way to ease out those pain >> >> points. I >> >> > > > will >> >> > > > > be >> >> > > > > > > > happy >> >> > > > > > > > > > to >> >> > > > > > > > > > > > help >> >> > > > > > > > > > > > > > out >> >> > > > > > > > > > > > > > >> >> with >> >> > > > > > > > > > > > > > >> >> > > the work on this KIP. @Magnus, >> thanks >> >> for >> >> > > > > driving >> >> > > > > > > > this, >> >> > > > > > > > > > is >> >> > > > > > > > > > > it >> >> > > > > > > > > > > > > OK >> >> > > > > > > > > > > > > > if >> >> > > > > > > > > > > > > > >> I >> >> > > > > > > > > > > > > > >> >> > carry >> >> > > > > > > > > > > > > > >> >> > > forward the work from here. It will >> be >> >> > > ideal >> >> > > > to >> >> > > > > > > have >> >> > > > > > > > > this >> >> > > > > > > > > > > in >> >> > > > > > > > > > > > > > >> 0.10.0.0. >> >> > > > > > > > > > > > > > >> >> > > >> >> > > > > > > > > > > > > > >> >> > > On Mon, Oct 12, 2015 at 9:29 PM, Jay >> >> > Kreps >> >> > > < >> >> > > > > > > > > > > j...@confluent.io >> >> > > > > > > > > > > > > >> >> > > > > > > > > > > > > > >> wrote: >> >> > > > > > > > > > > > > > >> >> > > >> >> > > > > > > > > > > > > > >> >> > >> I wonder if we need to solve the >> >> error >> >> > > > > problem? >> >> > > > > > I >> >> > > > > > > > > think >> >> > > > > > > > > > > this >> >> > > > > > > > > > > > > KIP >> >> > > > > > > > > > > > > > >> >> gives a >> >> > > > > > > > > > > > > > >> >> > >> descent work around. >> >> > > > > > > > > > > > > > >> >> > >> >> >> > > > > > > > > > > > > > >> >> > >> Probably we should have included an >> >> > error >> >> > > in >> >> > > > > the >> >> > > > > > > > > > response >> >> > > > > > > > > > > > > > header, >> >> > > > > > > > > > > > > > >> but >> >> > > > > > > > > > > > > > >> >> we >> >> > > > > > > > > > > > > > >> >> > >> debated it at the time decided not >> to >> >> > and >> >> > > > now >> >> > > > > it >> >> > > > > > > is >> >> > > > > > > > > > pretty >> >> > > > > > > > > > > > > hard >> >> > > > > > > > > > > > > > to >> >> > > > > > > > > > > > > > >> add >> >> > > > > > > > > > > > > > >> >> > >> because the headers aren't >> versioned >> >> > > (d'oh). >> >> > > > > > > > > > > > > > >> >> > >> >> >> > > > > > > > > > > > > > >> >> > >> It seems like any other solution is >> >> > going >> >> > > to >> >> > > > > be >> >> > > > > > > kind >> >> > > > > > > > > of >> >> > > > > > > > > > a >> >> > > > > > > > > > > > > hack, >> >> > > > > > > > > > > > > > >> right? >> >> > > > > > > > > > > > > > >> >> > >> Sending malformed responses back >> >> seems >> >> > > like >> >> > > > > not >> >> > > > > > a >> >> > > > > > > > > clean >> >> > > > > > > > > > > > > > solution... >> >> > > > > > > > > > > > > > >> >> > >> >> >> > > > > > > > > > > > > > >> >> > >> (Not sure if I was pro- having a >> >> > top-level >> >> > > > > error >> >> > > > > > > or >> >> > > > > > > > > not, >> >> > > > > > > > > > > but >> >> > > > > > > > > > > > > in >> >> > > > > > > > > > > > > > any >> >> > > > > > > > > > > > > > >> >> case >> >> > > > > > > > > > > > > > >> >> > >> the rationale for the decision was >> >> that >> >> > so >> >> > > > > many >> >> > > > > > of >> >> > > > > > > > the >> >> > > > > > > > > > > > > requests >> >> > > > > > > > > > > > > > >> were >> >> > > > > > > > > > > > > > >> >> > >> per-partition or per-topic or >> >> whatever >> >> > and >> >> > > > > hence >> >> > > > > > > > fail >> >> > > > > > > > > or >> >> > > > > > > > > > > > > > succeed at >> >> > > > > > > > > > > > > > >> >> that >> >> > > > > > > > > > > > > > >> >> > >> level and this makes it hard to >> know >> >> > what >> >> > > > the >> >> > > > > > > right >> >> > > > > > > > > > > > top-level >> >> > > > > > > > > > > > > > error >> >> > > > > > > > > > > > > > >> >> code >> >> > > > > > > > > > > > > > >> >> > >> is >> >> > > > > > > > > > > > > > >> >> > >> and hard for the client to figure >> out >> >> > what >> >> > > > to >> >> > > > > do >> >> > > > > > > > with >> >> > > > > > > > > > the >> >> > > > > > > > > > > > top >> >> > > > > > > > > > > > > > level >> >> > > > > > > > > > > > > > >> >> > error >> >> > > > > > > > > > > > > > >> >> > >> if some of the partitions succeed >> but >> >> > > there >> >> > > > > is a >> >> > > > > > > > > > top-level >> >> > > > > > > > > > > > > > error). >> >> > > > > > > > > > > > > > >> >> > >> >> >> > > > > > > > > > > > > > >> >> > >> I think actually this new API >> >> actually >> >> > > > gives a >> >> > > > > > way >> >> > > > > > > > to >> >> > > > > > > > > > > handle >> >> > > > > > > > > > > > > > this >> >> > > > > > > > > > > > > > >> >> > >> gracefully on the client side by >> just >> >> > > having >> >> > > > > > > clients >> >> > > > > > > > > > that >> >> > > > > > > > > > > > want >> >> > > > > > > > > > > > > > to >> >> > > > > > > > > > > > > > >> be >> >> > > > > > > > > > > > > > >> >> > >> graceful check for support for >> their >> >> > > > version. >> >> > > > > > > > Clients >> >> > > > > > > > > > that >> >> > > > > > > > > > > > do >> >> > > > > > > > > > > > > > that >> >> > > > > > > > > > > > > > >> >> will >> >> > > > > > > > > > > > > > >> >> > >> have a graceful message. >> >> > > > > > > > > > > > > > >> >> > >> >> >> > > > > > > > > > > > > > >> >> > >> At some point if we're ever >> reworking >> >> > the >> >> > > > > > headers >> >> > > > > > > we >> >> > > > > > > > > > > should >> >> > > > > > > > > > > > > > really >> >> > > > > > > > > > > > > > >> >> > >> consider >> >> > > > > > > > > > > > > > >> >> > >> (a) versioning them and (b) adding >> a >> >> > > > top-level >> >> > > > > > > error >> >> > > > > > > > > > code >> >> > > > > > > > > > > in >> >> > > > > > > > > > > > > the >> >> > > > > > > > > > > > > > >> >> > response. >> >> > > > > > > > > > > > > > >> >> > >> But given this would be a big >> >> breaking >> >> > > > change >> >> > > > > > and >> >> > > > > > > > this >> >> > > > > > > > > > is >> >> > > > > > > > > > > > > really >> >> > > > > > > > > > > > > > >> just >> >> > > > > > > > > > > > > > >> >> to >> >> > > > > > > > > > > > > > >> >> > >> give a nicer error message seems >> >> like it >> >> > > > > > probably >> >> > > > > > > > > isn't >> >> > > > > > > > > > > > worth >> >> > > > > > > > > > > > > > it to >> >> > > > > > > > > > > > > > >> >> try >> >> > > > > > > > > > > > > > >> >> > to >> >> > > > > > > > > > > > > > >> >> > >> do something now. >> >> > > > > > > > > > > > > > >> >> > >> >> >> > > > > > > > > > > > > > >> >> > >> -Jay >> >> > > > > > > > > > > > > > >> >> > >> >> >> > > > > > > > > > > > > > >> >> > >> >> >> > > > > > > > > > > > > > >> >> > >> >> >> > > > > > > > > > > > > > >> >> > >> On Mon, Oct 12, 2015 at 8:11 PM, >> >> > Jiangjie >> >> > > > Qin >> >> > > > > > > > > > > > > > >> >> <j...@linkedin.com.invalid >> >> > > > > > > > > > > > > > >> >> > > >> >> > > > > > > > > > > > > > >> >> > >> wrote: >> >> > > > > > > > > > > > > > >> >> > >> >> >> > > > > > > > > > > > > > >> >> > >> > I am thinking instead of >> returning >> >> an >> >> > > > empty >> >> > > > > > > > > response, >> >> > > > > > > > > > it >> >> > > > > > > > > > > > > > would be >> >> > > > > > > > > > > > > > >> >> > >> better to >> >> > > > > > > > > > > > > > >> >> > >> > return an explicit >> >> > > > > UnsupportedVersionException >> >> > > > > > > > code. >> >> > > > > > > > > > > > > > >> >> > >> > >> >> > > > > > > > > > > > > > >> >> > >> > Today KafkaApis handles the error >> >> in >> >> > the >> >> > > > > > > following >> >> > > > > > > > > > way: >> >> > > > > > > > > > > > > > >> >> > >> > 1. For requests/responses using >> old >> >> > > Scala >> >> > > > > > > classes, >> >> > > > > > > > > > > > KafkaApis >> >> > > > > > > > > > > > > > uses >> >> > > > > > > > > > > > > > >> >> > >> > RequestOrResponse.handleError() >> to >> >> > > return >> >> > > > an >> >> > > > > > > error >> >> > > > > > > > > > > > response. >> >> > > > > > > > > > > > > > >> >> > >> > 2. For requests/response using >> Java >> >> > > > classes >> >> > > > > > > (only >> >> > > > > > > > > > > > > > >> JoinGroupRequest >> >> > > > > > > > > > > > > > >> >> and >> >> > > > > > > > > > > > > > >> >> > >> > Heartbeat now), KafkaApis calls >> >> > > > > > > > > > > > > > >> AbstractRequest.getErrorResponse() >> >> > > > > > > > > > > > > > >> >> to >> >> > > > > > > > > > > > > > >> >> > >> > return an error response. >> >> > > > > > > > > > > > > > >> >> > >> > >> >> > > > > > > > > > > > > > >> >> > >> > In KAFKA-2512, I am returning an >> >> > > > > > > > > > > > UnsupportedVersionException >> >> > > > > > > > > > > > > > for >> >> > > > > > > > > > > > > > >> >> case >> >> > > > > > > > > > > > > > >> >> > >> [1] >> >> > > > > > > > > > > > > > >> >> > >> > when see an unsupported version. >> >> This >> >> > > will >> >> > > > > put >> >> > > > > > > the >> >> > > > > > > > > > error >> >> > > > > > > > > > > > > code >> >> > > > > > > > > > > > > > per >> >> > > > > > > > > > > > > > >> >> > topic >> >> > > > > > > > > > > > > > >> >> > >> or >> >> > > > > > > > > > > > > > >> >> > >> > partition for most of the >> requests, >> >> > but >> >> > > > > might >> >> > > > > > > not >> >> > > > > > > > > work >> >> > > > > > > > > > > all >> >> > > > > > > > > > > > > the >> >> > > > > > > > > > > > > > >> time. >> >> > > > > > > > > > > > > > >> >> > >> e.g. >> >> > > > > > > > > > > > > > >> >> > >> > TopicMetadataRequest with an >> empty >> >> > topic >> >> > > > > set. >> >> > > > > > > > > > > > > > >> >> > >> > >> >> > > > > > > > > > > > > > >> >> > >> > Case [2] does not quite work for >> >> > > > unsupported >> >> > > > > > > > > version, >> >> > > > > > > > > > > > > because >> >> > > > > > > > > > > > > > we >> >> > > > > > > > > > > > > > >> >> will >> >> > > > > > > > > > > > > > >> >> > >> > thrown an uncaught exception when >> >> > > version >> >> > > > is >> >> > > > > > not >> >> > > > > > > > > > > > recognized >> >> > > > > > > > > > > > > > (BTW >> >> > > > > > > > > > > > > > >> >> this >> >> > > > > > > > > > > > > > >> >> > >> is a >> >> > > > > > > > > > > > > > >> >> > >> > bug). Part of the reason is that >> >> for >> >> > > some >> >> > > > > > > response >> >> > > > > > > > > > > types, >> >> > > > > > > > > > > > > > error >> >> > > > > > > > > > > > > > >> code >> >> > > > > > > > > > > > > > >> >> > is >> >> > > > > > > > > > > > > > >> >> > >> not >> >> > > > > > > > > > > > > > >> >> > >> > part of the response level field. >> >> > > > > > > > > > > > > > >> >> > >> > >> >> > > > > > > > > > > > > > >> >> > >> > Maybe it worth checking how each >> >> > > response >> >> > > > is >> >> > > > > > > > dealing >> >> > > > > > > > > > > with >> >> > > > > > > > > > > > > > error >> >> > > > > > > > > > > > > > >> code >> >> > > > > > > > > > > > > > >> >> > >> today. >> >> > > > > > > > > > > > > > >> >> > >> > A scan of the response formats >> >> gives >> >> > the >> >> > > > > > > following >> >> > > > > > > > > > > result: >> >> > > > > > > > > > > > > > >> >> > >> > 1. TopicMetadataResponse - per >> >> topic >> >> > > error >> >> > > > > > code, >> >> > > > > > > > > does >> >> > > > > > > > > > > not >> >> > > > > > > > > > > > > work >> >> > > > > > > > > > > > > > >> when >> >> > > > > > > > > > > > > > >> >> > the >> >> > > > > > > > > > > > > > >> >> > >> > topic set is empty in the >> request. >> >> > > > > > > > > > > > > > >> >> > >> > 2. ProduceResonse - per partition >> >> > error >> >> > > > > code. >> >> > > > > > > > > > > > > > >> >> > >> > 3. OffsetCommitResponse - per >> >> > partition. >> >> > > > > > > > > > > > > > >> >> > >> > 4. OffsetFetchResponse - per >> >> > partition. >> >> > > > > > > > > > > > > > >> >> > >> > 5. OffsetResponse - per >> partition. >> >> > > > > > > > > > > > > > >> >> > >> > 6. FetchResponse - per partition >> >> > > > > > > > > > > > > > >> >> > >> > 7. ConsumerMetadataResponse - >> >> response >> >> > > > level >> >> > > > > > > > > > > > > > >> >> > >> > 8. ControlledShutdownResponse - >> >> > response >> >> > > > > level >> >> > > > > > > > > > > > > > >> >> > >> > 9. JoinGroupResponse - response >> >> level >> >> > > > > > > > > > > > > > >> >> > >> > 10. HearbeatResponse - response >> >> level >> >> > > > > > > > > > > > > > >> >> > >> > 11. LeaderAndIsrResponse - >> response >> >> > > level >> >> > > > > > > > > > > > > > >> >> > >> > 12. StopReplicaResponse - >> response >> >> > level >> >> > > > > > > > > > > > > > >> >> > >> > 13. UpdateMetadataResponse - >> >> response >> >> > > > level >> >> > > > > > > > > > > > > > >> >> > >> > >> >> > > > > > > > > > > > > > >> >> > >> > So from the list above it looks >> for >> >> > each >> >> > > > > > > response >> >> > > > > > > > we >> >> > > > > > > > > > are >> >> > > > > > > > > > > > > > actually >> >> > > > > > > > > > > > > > >> >> able >> >> > > > > > > > > > > > > > >> >> > >> to >> >> > > > > > > > > > > > > > >> >> > >> > return an error code, as long as >> we >> >> > make >> >> > > > > sure >> >> > > > > > > the >> >> > > > > > > > > > topic >> >> > > > > > > > > > > or >> >> > > > > > > > > > > > > > >> partition >> >> > > > > > > > > > > > > > >> >> > >> won't >> >> > > > > > > > > > > > > > >> >> > >> > be empty when the error code is >> at >> >> > topic >> >> > > > or >> >> > > > > > > > > partition >> >> > > > > > > > > > > > level. >> >> > > > > > > > > > > > > > >> Luckily >> >> > > > > > > > > > > > > > >> >> > in >> >> > > > > > > > > > > > > > >> >> > >> the >> >> > > > > > > > > > > > > > >> >> > >> > above list we only need to worry >> >> about >> >> > > > > > > > > > > > > TopicMetadataResponse. >> >> > > > > > > > > > > > > > >> >> > >> > >> >> > > > > > > > > > > > > > >> >> > >> > Maybe error handling is out of >> the >> >> > scope >> >> > > > of >> >> > > > > > this >> >> > > > > > > > > KIP, >> >> > > > > > > > > > > but >> >> > > > > > > > > > > > I >> >> > > > > > > > > > > > > > >> prefer >> >> > > > > > > > > > > > > > >> >> we >> >> > > > > > > > > > > > > > >> >> > >> think >> >> > > > > > > > > > > > > > >> >> > >> > through how to deal with error >> code >> >> > for >> >> > > > the >> >> > > > > > > > > requests, >> >> > > > > > > > > > > > > because >> >> > > > > > > > > > > > > > >> there >> >> > > > > > > > > > > > > > >> >> > are >> >> > > > > > > > > > > > > > >> >> > >> > more request types to be added in >> >> > > > KAFKA-2464 >> >> > > > > > and >> >> > > > > > > > > > future >> >> > > > > > > > > > > > > > patches. >> >> > > > > > > > > > > > > > >> >> > >> > >> >> > > > > > > > > > > > > > >> >> > >> > Thanks, >> >> > > > > > > > > > > > > > >> >> > >> > >> >> > > > > > > > > > > > > > >> >> > >> > Jiangjie (Becket) Qin >> >> > > > > > > > > > > > > > >> >> > >> > >> >> > > > > > > > > > > > > > >> >> > >> > On Mon, Oct 12, 2015 at 6:04 PM, >> >> Jay >> >> > > > Kreps < >> >> > > > > > > > > > > > > j...@confluent.io> >> >> > > > > > > > > > > > > > >> >> wrote: >> >> > > > > > > > > > > > > > >> >> > >> > >> >> > > > > > > > > > > > > > >> >> > >> > > Two quick pieces of feedback: >> >> > > > > > > > > > > > > > >> >> > >> > > 1. The use of a version of -1 >> as >> >> > > magical >> >> > > > > > entry >> >> > > > > > > > > > > dividing >> >> > > > > > > > > > > > > > between >> >> > > > > > > > > > > > > > >> >> > >> > deprecated >> >> > > > > > > > > > > > > > >> >> > >> > > versions is a bit hacky. What >> >> about >> >> > > > > instead >> >> > > > > > > > having >> >> > > > > > > > > > an >> >> > > > > > > > > > > > > array >> >> > > > > > > > > > > > > > of >> >> > > > > > > > > > > > > > >> >> > >> supported >> >> > > > > > > > > > > > > > >> >> > >> > > versions and a separate array >> of >> >> > > > > deprecated >> >> > > > > > > > > > versions. >> >> > > > > > > > > > > > The >> >> > > > > > > > > > > > > > >> >> deprecated >> >> > > > > > > > > > > > > > >> >> > >> > > versions would always be a >> >> subset of >> >> > > the >> >> > > > > > > > supported >> >> > > > > > > > > > > > > versions. >> >> > > > > > > > > > > > > > >> Or, >> >> > > > > > > > > > > > > > >> >> > >> > > alternately, since deprecation >> >> has >> >> > no >> >> > > > > > > functional >> >> > > > > > > > > > > impact >> >> > > > > > > > > > > > > and >> >> > > > > > > > > > > > > > is >> >> > > > > > > > > > > > > > >> >> just >> >> > > > > > > > > > > > > > >> >> > a >> >> > > > > > > > > > > > > > >> >> > >> > > message to developers, we could >> >> just >> >> > > > leave >> >> > > > > > it >> >> > > > > > > > out >> >> > > > > > > > > of >> >> > > > > > > > > > > the >> >> > > > > > > > > > > > > > >> protocol >> >> > > > > > > > > > > > > > >> >> > and >> >> > > > > > > > > > > > > > >> >> > >> > just >> >> > > > > > > > > > > > > > >> >> > >> > > have it in release notes etc. >> >> > > > > > > > > > > > > > >> >> > >> > > 2. I think including the api >> name >> >> > may >> >> > > > > cause >> >> > > > > > > some >> >> > > > > > > > > > > > problems. >> >> > > > > > > > > > > > > > >> >> Currently >> >> > > > > > > > > > > > > > >> >> > >> the >> >> > > > > > > > > > > > > > >> >> > >> > > api key is the primary key that >> >> we >> >> > > keep >> >> > > > > > > > consistent >> >> > > > > > > > > > but >> >> > > > > > > > > > > > we >> >> > > > > > > > > > > > > > have >> >> > > > > > > > > > > > > > >> >> > >> actually >> >> > > > > > > > > > > > > > >> >> > >> > > evolved the english description >> >> of >> >> > the >> >> > > > > apis >> >> > > > > > as >> >> > > > > > > > > they >> >> > > > > > > > > > > have >> >> > > > > > > > > > > > > > >> changed. >> >> > > > > > > > > > > > > > >> >> > The >> >> > > > > > > > > > > > > > >> >> > >> > only >> >> > > > > > > > > > > > > > >> >> > >> > > use I can think of for the name >> >> > would >> >> > > be >> >> > > > > if >> >> > > > > > > > people >> >> > > > > > > > > > > used >> >> > > > > > > > > > > > > the >> >> > > > > > > > > > > > > > >> >> logical >> >> > > > > > > > > > > > > > >> >> > >> name >> >> > > > > > > > > > > > > > >> >> > >> > > and tried to resolve the api >> key, >> >> > but >> >> > > > that >> >> > > > > > > would >> >> > > > > > > > > be >> >> > > > > > > > > > > > wrong. >> >> > > > > > > > > > > > > > Not >> >> > > > > > > > > > > > > > >> >> sure >> >> > > > > > > > > > > > > > >> >> > >> if we >> >> > > > > > > > > > > > > > >> >> > >> > > actually need the english name, >> >> if >> >> > > there >> >> > > > > is >> >> > > > > > a >> >> > > > > > > > use >> >> > > > > > > > > > > case I >> >> > > > > > > > > > > > > > guess >> >> > > > > > > > > > > > > > >> >> we'll >> >> > > > > > > > > > > > > > >> >> > >> just >> >> > > > > > > > > > > > > > >> >> > >> > > have to be very clear that the >> >> name >> >> > is >> >> > > > > just >> >> > > > > > > > > > > > documentation >> >> > > > > > > > > > > > > > and >> >> > > > > > > > > > > > > > >> can >> >> > > > > > > > > > > > > > >> >> > >> change >> >> > > > > > > > > > > > > > >> >> > >> > > any time. >> >> > > > > > > > > > > > > > >> >> > >> > > >> >> > > > > > > > > > > > > > >> >> > >> > > -Jay >> >> > > > > > > > > > > > > > >> >> > >> > > >> >> > > > > > > > > > > > > > >> >> > >> > > On Fri, Sep 25, 2015 at 2:53 >> PM, >> >> > > Magnus >> >> > > > > > > > Edenhill < >> >> > > > > > > > > > > > > > >> >> > mag...@edenhill.se> >> >> > > > > > > > > > > > > > >> >> > >> > > wrote: >> >> > > > > > > > > > > > > > >> >> > >> > > >> >> > > > > > > > > > > > > > >> >> > >> > > > Good evening, >> >> > > > > > > > > > > > > > >> >> > >> > > > >> >> > > > > > > > > > > > > > >> >> > >> > > > KIP-35 was created to address >> >> > > current >> >> > > > > and >> >> > > > > > > > future >> >> > > > > > > > > > > > > > >> broker-client >> >> > > > > > > > > > > > > > >> >> > >> > > > compatibility. >> >> > > > > > > > > > > > > > >> >> > >> > > > >> >> > > > > > > > > > > > > > >> >> > >> > > > >> >> > > > > > > > > > > > > > >> >> > >> > > > >> >> > > > > > > > > > > > > > >> >> > >> > > >> >> > > > > > > > > > > > > > >> >> > >> > >> >> > > > > > > > > > > > > > >> >> > >> >> >> > > > > > > > > > > > > > >> >> > >> >> > > > > > > > > > > > > > >> >> >> >> > > > > > > > > > > > > > >> >> >> > > > > > > > > > > > > > >> >> > > > > > > > > > > > > >> >> > > > > > > > > > > > >> >> > > > > > > > > > > >> >> > > > > > > > > > >> >> > > > > > > > > >> >> > > > > > > > >> >> > > > > > > >> >> > > > > > >> >> > > > > >> >> > > > >> >> > > >> >> > >> >> >> https://cwiki.apache.org/confluence/display/KAFKA/KIP-35+-+Retrieving+protocol+version >> >> > > > > > > > > > > > > > >> >> > >> > > > >> >> > > > > > > > > > > > > > >> >> > >> > > > Summary: >> >> > > > > > > > > > > > > > >> >> > >> > > > * allow clients to retrieve >> >> the >> >> > > > > broker's >> >> > > > > > > > > protocol >> >> > > > > > > > > > > > > version >> >> > > > > > > > > > > > > > >> >> > >> > > > * make broker handle unknown >> >> > > protocol >> >> > > > > > > > requests >> >> > > > > > > > > > > > > gracefully >> >> > > > > > > > > > > > > > >> >> > >> > > > >> >> > > > > > > > > > > > > > >> >> > >> > > > Feedback and comments >> welcome! >> >> > > > > > > > > > > > > > >> >> > >> > > > >> >> > > > > > > > > > > > > > >> >> > >> > > > Regards, >> >> > > > > > > > > > > > > > >> >> > >> > > > Magnus >> >> > > > > > > > > > > > > > >> >> > >> > > > >> >> > > > > > > > > > > > > > >> >> > >> > > >> >> > > > > > > > > > > > > > >> >> > >> > >> >> > > > > > > > > > > > > > >> >> > >> >> >> > > > > > > > > > > > > > >> >> > > >> >> > > > > > > > > > > > > > >> >> > > >> >> > > > > > > > > > > > > > >> >> > > >> >> > > > > > > > > > > > > > >> >> > > -- >> >> > > > > > > > > > > > > > >> >> > > >> >> > > > > > > > > > > > > > >> >> > > Regards, >> >> > > > > > > > > > > > > > >> >> > > Ashish >> >> > > > > > > > > > > > > > >> >> > > >> >> > > > > > > > > > > > > > >> >> > >> >> > > > > > > > > > > > > > >> >> > >> >> > > > > > > > > > > > > > >> >> > >> >> > > > > > > > > > > > > > >> >> > -- >> >> > > > > > > > > > > > > > >> >> > >> >> > > > > > > > > > > > > > >> >> > Regards, >> >> > > > > > > > > > > > > > >> >> > Ashish >> >> > > > > > > > > > > > > > >> >> > >> >> > > > > > > > > > > > > > >> >> >> >> > > > > > > > > > > > > > >> > >> >> > > > > > > > > > > > > > >> > >> >> > > > > > > > > > > > > > >> > >> >> > > > > > > > > > > > > > >> > -- >> >> > > > > > > > > > > > > > >> > >> >> > > > > > > > > > > > > > >> > Regards, >> >> > > > > > > > > > > > > > >> > Ashish >> >> > > > > > > > > > > > > > >> >> >> > > > > > > > > > > > > > > >> >> > > > > > > > > > > > > > > >> >> > > > > > > > > > > > > > > >> >> > > > > > > > > > > > > > > -- >> >> > > > > > > > > > > > > > > >> >> > > > > > > > > > > > > > > Regards, >> >> > > > > > > > > > > > > > > Ashish >> >> > > > > > > > > > > > > > >> >> > > > > > > > > > > > > >> >> > > > > > > > > > > > > >> >> > > > > > > > > > > > > >> >> > > > > > > > > > > > > -- >> >> > > > > > > > > > > > > >> >> > > > > > > > > > > > > Regards, >> >> > > > > > > > > > > > > Ashish >> >> > > > > > > > > > > > > >> >> > > > > > > > > > > > >> >> > > > > > > > > > > >> >> > > > > > > > > > >> >> > > > > > > > > >> >> > > > > > > > >> >> > > > > > > >> >> > > > > > >> >> > > > > > >> >> > > > > > >> >> > > > > > -- >> >> > > > > > >> >> > > > > > Regards, >> >> > > > > > Ashish >> >> > > > > > >> >> > > > > >> >> > > > >> >> > > >> >> > >> >> > >> >> > >> >> > -- >> >> > >> >> > Regards, >> >> > Ashish >> >> > >> >> >> > >> > >> > >> > -- >> > >> > Regards, >> > Ashish >> > >> >> >> >> -- >> >> Regards, >> Ashish >>