WRT the version I don't think it will really help to catch bugs. It is definitely possible for us to have protocol bugs, but I don't think that that version would catch the problem since the person breaking the protocol would probably not change the version number. I also think it is confusing for the person implementing the client. Having a version seems to indicate that we can send back different response versions, which we can't. Should they handle them? What should they do if they get an unknown version? In reality we should just not do this and guarantee that we send back the version they expect.
I think the two things we need to do to ensure this really works is. The first is to fix the way we are handling serialization, it is currently very fiddly and error prone. It was okay when there were two requests with a couple fields each, but isn't working any more. The second is that starting with the next release we need to reinstate the practice we had with the previous protocol of saving protocol request and response binaries for each request from each release and testing that we are properly compatible with old versions. I think that this is a very effective kind of test and would catch bugs of the form you describe. -Jay On Fri, Dec 7, 2012 at 5:40 AM, ben fleis <b...@monkey.org> wrote: > First off, let me amend my previous votes: +1 for both ApiKey and > ApiVersion in all responses, even @ 4 bytes. The number of bytes is > insignificant, useful in the debugging and multiplex scenarios, even if the > underlying service doesn't yet support it. (The protocol still can.) > CorrelationId can clearly be used to the same end, but it's still my > preference. > > Secondly, things changed since yesterday, so now i'm using the doc to > update my protocol. > > Biggest comment -- the visual layout of the wire protocol doesn't reflect > the way I want to read a protocol. (IMHO, BNF makes sense for token/string > based protocols, but not so much for binary.) Alignment, byte counts when > available, etc. would all be fantastic. This comes from my own notes, and > gives an idea of what I wanted to know while implementing: > > StandardRequestHeader: > > > field name | type | bytes | notes > > > ---------------------------------------------------------------------------- > size | int32 | 4 | remaining > packet size > api_key | int16 | 2 | currently > [0,5] > api_version | int16 | 2 | > correlation_id | int32 | 4 | > client_id | int16_string | 2+|s| | > > TopicMetadata Request: > > field name | type | bytes | notes > > > ---------------------------------------------------------------------------- > <StandardRequestHeader> | 14+|s| | > len([topic name]) | int32 | 4 | > [topic name] | [int16_string] | [2+|s|] | > > I will have more commentary as I read more, but this is the major thing for > me. > > b >
