Ashish, thanks, didnt know that. For ApiVersionRequest requesting no Apis to be returned doesnt make sense so the distinction isn't necessary, but I'm fine with adding Null to be more in line with future protocol requests, as long as it doesn't delay this KIP any longer! :)
2016-04-05 19:38 GMT+02:00 Ashish Singh <asi...@cloudera.com>: > Magnus, it is proposed to be changed in version 1, > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-4+-+Command+line+and+centralized+administrative+operations#KIP-4-Commandlineandcentralizedadministrativeoperations-MetadataSchema > . > > On Tue, Apr 5, 2016 at 10:23 AM, Magnus Edenhill <mag...@edenhill.se> > wrote: > > > Empty arrays are already used in MetadataRequest to retrieve all topics > in > > the cluster, > > ApiVersion request will have the same standard semantics. > > > > > > 2016-04-05 19:01 GMT+02:00 Ewen Cheslack-Postava <e...@confluent.io>: > > > > > Also, just a thought but is empty list the sentinel we really want to > > > indicate we want all API versions? We've got nullable string and > nullable > > > bytes in the protocol. Should we have nullable array support as well > and > > > use that to indicate we want everything? I can't think of a use case > for > > > sending an empty list, but null seems like a better sentinel than an > > empty > > > list. > > > > > > -Ewen > > > > > > On Tue, Apr 5, 2016 at 9:23 AM, Ewen Cheslack-Postava < > e...@confluent.io > > > > > > wrote: > > > > > > > > > > > > > > > On Mon, Apr 4, 2016 at 11:24 AM, Gwen Shapira <g...@confluent.io> > > wrote: > > > > > > > >> > > > > >> > > > > >> > In case of connection closures, the KIP recommends that clients > > > >> should > > > >> > use some other method of determining the apiRequest version to > > use, > > > >> > like, > > > >> > probing. For instance, client will send V0 version of > apiVersion > > > >> request > > > >> > and will try higher versions incrementally. In case b, client > > will > > > >> > eventually get apiVersion response and know what api versions > it > > > >> should > > > >> > use. For case a and c, client will eventually give up and > > propagate > > > >> an > > > >> > error to application. > > > >> > > > > >> > > > > >> I strongly disagree that we should recommend this probing method. > > > >> Probing is essentially what clients do now (since we lack any way to > > > >> communicate versions), and is what we are trying to solve with this > > KIP. > > > >> Considering that different brokers could have different versions, > and > > > that > > > >> brokers can change version at any point, this sounds slow, difficult > > to > > > >> implement and fragile. > > > >> > > > >> Also note that even with this method, without VersionRequest v0, we > > will > > > >> break clients in the one way Kafka currently promises to never > break: > > > Old > > > >> clients won't be able to work with new brokers. > > > >> > > > >> If this is part of KIP-35, I am against it. > > > >> > > > >> Since all Request/Responses in our protocol have versions, > publishing > > > >> versions for each request/response should be something we can easily > > > >> support into the future. It sounds far easier than asking every > single > > > >> client to implement the method you specified above. > > > >> > > > > > > > > Gwen, > > > > > > > > Agreed, and I think it would be fine to make permanent support > (barring > > > > massive changes to the protocol) part of the KIP text. There's really > > no > > > > reason not to and it basically just turns this into the basis for a > > > pretty > > > > simple handshake protocol. > > > > > > > > (I'm tempted to not even bring this up given that we're converging, > but > > > > one reason I could see this being changed in the future is that > > protocol > > > > support is only conveyed in one direction. This could also be turned > > > into a > > > > slightly more general handshake approach where the client also > > advertises > > > > what it supports. However, given the way request/response versioning > > > works, > > > > I can't think of a reason we'd need this atm.) > > > > > > > > -Ewen > > > > > > > > > > > >> > > > >> Gwen > > > >> > > > > > > > > > > > > > > > > -- > > > > Thanks, > > > > Ewen > > > > > > > > > > > > > > > > -- > > > Thanks, > > > Ewen > > > > > > > > > -- > > Regards, > Ashish >
