Yeah I agree with that characterization of the tradeoff. I think what that would imply would be that evolution of the metadata request (or the protocol version request) would remain server-first, whereas other apis would be independent. Not sure if I've fully thought it through, though.
-Jay 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 >
