Yeah I think that is the point--we have a proposal for a new protocol
versioning scheme and a vote on it but it doesn't actually describe
how versioning will work yet! I gave my vague impression based on this
thread, but I want to make sure that is correct and get it written
down before we adopt it.
-Jay
On Mon, Mar 14, 2016 at 5:58 PM, Gwen Shapira <g...@confluent.io> wrote:
> On Mon, Mar 14, 2016 at 5:31 PM, Jay Kreps <j...@confluent.io> wrote:
>
>> Couple of missing things:
>>
>> This KIP doesn't have a proposal on versioning it just gives different
>> options, it'd be good to get a concrete proposal in the KIP. Here is my
>> understanding of what we are proposing (can someone sanity check and if
>> correct, update the kip):
>>
>> 1. We will augment the existing api_version field in the header with a
>> protocol_version that will begin at some initial value and increment by
>> 1
>> every time we make a changes to any of the api_versions (question:
>> including internal apis?).
>>
>
> Jay, this part was not in the KIP and was never discussed.
> Are you proposing adding this? Or is it just an assumption you made?
>
>
>
>> 2. The protocol_version will be added to the metadata request
>> 3. We will also add a string that this proposal is calling VersionString
>> which will describe the build of kafka in some way. The clients should
>> not
>> under any circumstances do anything with this string other than print it
>> out to the user.
>>
>> One thing I'm not sure about: I think currently metadata sits in the client
>> for 10 mins by default. Say a client bootstraps and then a server is
>> downgraded to an earlier version, won't the client's metadata version
>> indicate that that client handles a version it doesn't actually handle any
>> more? We need to document how clients will handle this.
>>
>> Here are some comments on other details:
>>
>> 1. As a minor thing I think we should avoid naming the fields VersionId
>> and VersionString which sort of implies they are both used for
>> versioning.
>> I think we should call them something like ProtocolVersion and
>> BuildDescription, with BuildDescription being totally unspecified other
>> than that it is some kind of human readable string describing a
>> particular
>> Kafka build. We really don't want a client attempting to use this
>> string in
>> any way as that would always be the wrong thing to do in the versioning
>> scheme we are proposing, you should always use the protocol version.
>> 2. Does making the topics field in the metadata request nullable
>> actually make sense? We have a history of wanting to add magical values
>> rather than fields. Currently topics=[a] means give me information about
>> topic a, topics=[] means give me information about all topics, and we
>> are
>> proposing topics=null would mean don't give me topics. I don't have a
>> strong opinion.
>> 3. I prefer Jason's proposal on using a conservative metadata version
>> versus the empty response hack. However I think that may actually
>> exacerbate the downgrade scenario I described.
>> 4. I agree with Jason that we should really look at the details of the
>> implementation so we know it works--implementing server support without
>> actually trying it is kind of risky.
>>
>> As a meta comment: I'd really like to encourage us to think of the protocol
>> as a document that includes the following things:
>>
>> - The binary format, error codes, etc
>> - The request/response interaction
>> - The semantics of each request in different cases
>> - Instructions on how to use this to implement a client
>>
>> This document is versioned with the protocol number and is the source of
>> truth for the protocol.
>>
>> Part of any protocol change needs to be an update to the instructions on
>> how to use that part of the protocol. We should be opinionated. If there
>> are two options there should be a reason, and then we need to document both
>> and say exactly when to use each.
>>
>> I think we also need to get a "how to" document on protocol changes just so
>> people know what they need to do to add a new protocol feature.
>>
>> -Jay
>>
>> On Mon, Mar 14, 2016 at 4:51 PM, Jason Gustafson <ja...@confluent.io>
>> wrote:
>>
>> > >
>> > > Are you suggesting this as a solution for the problem where a KIP-35
>> > aware
>> > > client sends a higher version of metadata req, say v2, to a KIP-35
>> aware
>> > > broker that only supports up to v1.
>> >
>> >
>> > Yes, that's right. In that case, the client first sends v1, finds out
>> that
>> > the broker supports v2, and then sends v2 (if it has any reason to do
>> so).
>> >
>> > We had a bit of discussion on such scenarios, and it seemed to be a
>> chicken
>> > > and egg problem that is hard to avoid. Your suggestion definitely makes
>> > > sense, however it falls under the purview of clients.
>> >
>> >
>> > That basically means clients should figure it out for themselves? Might
>> be
>> > nice to have a better answer.
>> >
>> > KIP-35 only aims on adding support for getting the version info from a
>> > > broker. This definitely can be utilized by our clients. However, that
>> can
>> > > follow KIP-35 changes. Does this sound reasonable to you?
>> >
>> >
>> > It may be OK, but I'm a little concerned about offering a feature that we
>> > don't support ourselves. Sometimes it's not until implementation that we
>> > find out whether it really works as expected. And if we're eventually
>> > planning to support it, I feel we should think through some of the cases
>> a
>> > bit more. For example, the upgrade and downgrade cases that Becket
>> > mentioned earlier. It doesn't feel too great to support this feature
>> unless
>> > we can offer guidance on how to use it.
>> >
>> > -Jason
>> >
>> >
>> >
>> > On Mon, Mar 14, 2016 at 4:20 PM, Ashish Singh <asi...@cloudera.com>
>> wrote:
>> >
>> > > Hi Jason,
>> > >
>> > > On Mon, Mar 14, 2016 at 4:04 PM, Jason Gustafson <ja...@confluent.io>
>> > > wrote:
>> > >
>> > > > Perhaps clients should always send the oldest version of the metadata
>> > > > request which supports KIP-35 when initially connecting to the
>> cluster.
>> > > > Depending on the versions in the response, it can upgrade to a more
>> > > recent
>> > > > version. Then maybe we don't need the empty response hack?
>> > > >
>> > > Are you suggesting this as a solution for the problem where a KIP-35
>> > aware
>> > > client sends a higher version of metadata req, say v2, to a KIP-35
>> aware
>> > > broker that only supports up to v1.
>> > >
>> > > We had a bit of discussion on such scenarios, and it seemed to be a
>> > chicken
>> > > and egg problem that is hard to avoid. Your suggestion definitely makes
>> > > sense, however it falls under the purview of clients.
>> > >
>> > > >
>> > > > One thing that's not clear to me is whether the ultimate goal of this
>> > KIP
>> > > > is to have our clients support multiple broker versions. It would be
>> a
>> > > > little weird to have this feature if our own clients don't use it.
>> > > >
>> > > KIP-35 only aims on adding support for getting the version info from a
>> > > broker. This definitely can be utilized by our clients. However, that
>> can
>> > > follow KIP-35 changes. Does this sound reasonable to you?
>> > >
>> > > >
>> > > > -Jason
>> > > >
>> > > > On Mon, Mar 14, 2016 at 3:34 PM, Ashish Singh <asi...@cloudera.com>
>> > > wrote:
>> > > >
>> > > > > On Mon, Mar 14, 2016 at 2:12 PM, Ismael Juma <ism...@juma.me.uk>
>> > > wrote:
>> > > > >
>> > > > > > On Mon, Mar 14, 2016 at 8:45 PM, Gwen Shapira <g...@confluent.io
>> >
>> > > > wrote:
>> > > > > > >
>> > > > > > > > I don't see how it helps. If the client is communicating
>> with a
>> > > > > broker
>> > > > > > > that
>> > > > > > > > does not support KIP-35, that broker will simply close the
>> > > > > connection.
>> > > > > > If
>> > > > > > > > the broker supports KIP-35, then it will provide the broker
>> > > > version.
>> > > > > I
>> > > > > > > > don't envisage a scenario where a broker does not support
>> > KIP-35,
>> > > > but
>> > > > > > > > implements the new behaviour of sending an empty response. Do
>> > > you?
>> > > > > > > >
>> > > > > > > > Are you sure about that? Per KIP-35, the broker supplies the
>> > > > version
>> > > > > in
>> > > > > > > response to Metadata request, not in response to anything else.
>> > > > > > > If the client sends producer request version 42 (accidentally
>> or
>> > > due
>> > > > to
>> > > > > > > premature upgrade) to KIP-35-compactible broker - we want to
>> see
>> > an
>> > > > > empty
>> > > > > > > packet and not a connection close.
>> > > > > > > Sending a broker version was deemed impractical IIRC.
>> > > > > > >
>> > > > > >
>> > > > > > OK, so this is a different case than the one Ashish described
>> > > ("client
>> > > > > that
>> > > > > > wants to support broker versions that do not provide broker
>> version
>> > > in
>> > > > > > metadata and broker versions that provides version info in
>> > > metadata").
>> > > > > So,
>> > > > > > you are suggesting that if a client is communicating with a
>> broker
>> > > that
>> > > > > > implements KIP-35 and it receives an empty response, it will
>> assume
>> > > > that
>> > > > > > the broker doesn't support the request version and it won't try
>> to
>> > > > parse
>> > > > > > the response? I think it would be good to explain this kind of
>> > thing
>> > > in
>> > > > > > detail in the KIP.
>> > > > > >
>> > > > > Actually even in this case and the case I mentioned, closing
>> > connection
>> > > > > should be fine. Lets think about possible reasons that could lead
>> to
>> > > this
>> > > > > issue.
>> > > > >
>> > > > > 1. Client has incorrect mapping of supported protocols for a broker
>> > > > > version.
>> > > > > 2. Client misread broker version from metadata response.
>> > > > > 3. Client constructed unsupported protocol version by mistake.
>> > > > >
>> > > > > In all the above cases irrespective of what broker does, client
>> will
>> > > keep
>> > > > > sending wrong request version.
>> > > > >
>> > > > > At this point, I think sending an empty packet instead of closing
>> > > > > connection is a nice to have and not mandatory requirement. Like in
>> > the
>> > > > > above case, a client can catch parsing error and be sure that there
>> > is
>> > > > > something wrong in the protocol version it is sending. However, a
>> > > generic
>> > > > > connection close does not really provide any information on
>> probable
>> > > > cause.
>> > > > >
>> > > > > What do you guys suggest?
>> > > > >
>> > > > > >
>> > > > > > Ismael
>> > > > > >
>> > > > >
>> > > > >
>> > > > >
>> > > > > --
>> > > > >
>> > > > > Regards,
>> > > > > Ashish
>> > > > >
>> > > >
>> > >
>> > >
>> > >
>> > > --
>> > >
>> > > Regards,
>> > > Ashish
>> > >
>> >
>>
