Hi Jason,
On point 1) in the message protocol the headers are simply a byte array, as
like the key or value, this is to clearly demarcate the header in the core
message. Then the header byte array in the core message is an array of key,
value pairs. This is what it is denoting.
Then this would be I guess in the given notation:
Headers => [KeyLength, Key, ValueLength, Value]
KeyLength => int32 <-----------------NEW size of the byte[] of the
serialised key value
Key => bytes <---------------------- NEW serialised string (UTF8) bytes of
the header key
ValueLength => int32 <-------------- NEW size of the byte[] of the
serialised header value
Value => bytes <-------------------- NEW serialised form of the header value
The key length and value length is matching the way the protocol is defined in
the core message currently.
On point 2)
Var sized ints, this was discussed much earlier on, in fact I had suggested it
myself (with Hadoop references), the complexity of this compared to having a
simpler protocol was argued and agreed it wasn’t worth the complexity as all
other clients in other languages would need to ensure theyre using the right
var size algorithm, as there is a few.
On point 3)
We did the attributes, optional approach as originally there was marked concern
that headers would cause a message size overhead for others, who don’t want
them. As such this is the clean solution to achieve that. If that no longer
holds, and we don’t care that we add 4bytes overhead, then im happy to remove.
I’m personally in favour of keeping the message as small as possible so people
don’t get shocks in perf and throughputs dues to message size, unless they
actively use the feature, as such I do prefer the attribute bit wise feature
flag approach myself.
On 16/02/2017, 05:40, "Jason Gustafson" <ja...@confluent.io> wrote:
We have proposed a few significant changes to the message format in KIP-98
which now seems likely to pass (perhaps with some iterations on
implementation details). It would be good to try and coordinate the changes
in both of the proposals to make sure they are consistent and compatible.
I think using the attributes to indicate null headers is a reasonable
approach. We have proposed to do the same thing for the message key and
value. That said, I sympathize with Jay's argument. Having multiple ways to
specify a null value increases the overall complexity of the protocol. You
can see this just from the fact that you need the extra verbiage in the
protocol specification in this KIP and in KIP-98 to describe the dependence
between the fields and the attributes. It seems like a slippery slope if
you start allowing different request types to implement the protocol
specification differently.
You can also argue that the messages already are and are likely to remain a
special case. For example, there is currently no generality in how
compressed message sets are represented that would be applicable for other
request types. Some might see this divergence as an unfortunate protocol
deficiency which should be fixed; others might see it as sort of the
inevitability of needing to optimize where it counts most. I'm probably
somewhere in between, but I think we probably all share the intuition that
the protocol should be kept as consistent as possible. With that in mind,
here are a few comments:
1. One thing I found a little odd when reading the current proposal is that
the headers are both represented as an array of bytes and as an array of
key/value pairs. I'd probably suggest something like this:
Headers => [HeaderKey HeaderValue]
HeaderKey => String
HeaderValue => Bytes
An array in the Kafka protocol is represented as a 4-byte integer
indicating the number of elements in the array followed by the
serialization of the elements. Unless I'm misunderstanding, what you have
instead is the total size of the headers in bytes followed by the elements.
I'm not sure I see any reason for this inconsistency.
2. In KIP-98, we've introduced variable-length integer fields. Effectively,
we've enriched (or "complicated" as Jay might say ;) the protocol
specification to include the following types: VarInt, VarLong,
UnsignedVarInt and UnsignedVarLong.
Along with these primitives, we could introduce the following types:
VarSizeArray => NumberOfItems Item1 Item2 .. ItemN
NumberOfItems => UnsignedVarInt
VarSizeNullableArray => NumberOfItemsOrNull Item1 Item2 .. ItemN
NumberOfItemsOrNull => VarInt (-1 means null)
And similarly for the `String` and `Bytes` types. These types can save a
considerable amount of space in this proposal because they can be used for
both the number of headers included in the message and the lengths of the
header keys and values. We could do this instead:
Headers => VarSizeArray[HeaderKey HeaderValue]
HeaderKey => VarSizeString
HeaderValue => VarSizeBytes
Combining the savings from the use of variable length fields, the benefit
of using the attributes to represent null seems pretty small.
3. Whichever way we go (whether we use the attributes or not), we should at
least be consistent between this KIP and KIP-98. It would be very strange
to have two ways to represent null values in the same schema. Either way is
OK with me. I think some message-level optimizations are justifiable, but
the savings here seem minimal (a few bytes per message), so maybe it's not
worth the cost of letting the message diverge even further from the rest of
the protocol.
-Jason
On Wed, Feb 15, 2017 at 8:52 AM, radai <radai.rosenbl...@gmail.com> wrote:
> I've trimmed the inline contents as this mail is getting too big for the
> apache mailing list software to deliver :-(
>
> 1. the important thing for interoperability is for different "interested
> parties" (plugins, infra layers/wrappers, user-code) to be able to stick
> pieces of metadata onto msgs without getting in each other's way. a common
> key scheme (Strings, as of the time of this writing?) is all thats
required
> for that. it is assumed that the other end interested in any such piece of
> metadata knows the encoding, and byte[] provides for the most flexibility.
> i believe this is the same logic behind core kafka being byte[]/byte[] -
> Strings are more "usable" but bytes are flexible and so were chosen.
> Also - core kafka doesnt even do that good of a job on usability of the
> payload (example - i have to specify the nop byte[] "decoders" explicitly
> in conf), and again sacrificies usability for the sake of performance (no
> convenient single-record processing as poll is a batch, lots of obscure
> little config details exposing internals of the batching mechanism, etc)
>
> this is also why i really dislike the idea of a "type system" for header
> values, it further degrades the usability, adds complexity and will
> eventually get in people's way, also, it would be the 2nd/3rd home-group
> serialization mechanism in core kafka (counting 2 iterations of the "type
> definition DSL")
>
> 2. this is an implementation detail, and not even a very "user facing"
one?
> to the best of my understanding the vote process is on proposed
> API/behaviour. also - since we're willing to go with strings just
serialize
> a 0-sized header blob and IIUC you dont need any optionals anymore.
>
> 3. yes, we can :-)
>
> On Tue, Feb 14, 2017 at 11:56 PM, Michael Pearce <michael.pea...@ig.com>
> wrote:
>
> > Hi Jay,
> >
> > 1) There was some initial debate on the value part, as youll note
String,
> > String headers were discounted early on. The reason for this is
> flexibility
> > and keeping in line with the flexibility of key, value of the message
> > object itself. I don’t think it takes away from an ecosystem as each
> plugin
> > will care for their own key, this way ints, booleans , exotic custom
> binary
> > can all be catered for=.
> > a. If you really wanted to push for a typed value interface, I wouldn’t
> > want just String values supported, but the the primatives plus string
and
> > also still keeping the ability to have a binary for custom binaries that
> > some organisations may have.
> > i. I have written this slight alternative here,
> https://cwiki.apache.org/
> > confluence/display/KAFKA/KIP-82+-+Add+Record+Headers+-+Typed
> > ii. Essentially the value bytes, has a leading byte overhead.
> > 1. This tells you what type the value is, before reading the rest of
the
> > bytes, allowing serialisation/deserialization to and from the
primitives,
> > string and byte[]. This is akin to some other messaging systems.
> > 2) We are making it optional, so that for those not wanting headers have
> 0
> > bytes overhead (think of it as a feature flag), I don’t think this is
> > complex, especially if comparing to changes proposed in other kips like
> > kip-98.
> > a. If you really really don’t like this, we can drop it, but it would
> mean
> > buying into 4 bytes extra overhead for users who do not want to use
> headers.
> > 3) In the summary yes, it is at a higher level, but I think this is well
> > documented in the proposed changes section.
> > a. Added getHeaders method to Producer/Consumer record (that is it)
> > b. We’ve also detailed the new Headers class that this method returns
> that
> > encapsulates the headers protocol and logic.
> >
> > Best,
> > Mike
> >
> > ==Original questions from the vote thread from Jay.==
> >
> > Couple of things I think we still need to work out:
> >
> > 1. I think we agree about the key, but I think we haven't talked
about
> > the value yet. I think if our goal is an open ecosystem of these
> header
> > spread across many plugins from many systems we should consider
making
> > this
> > a string as well so it can be printed, set via a UI, set in config,
> etc.
> > Basically encouraging pluggable serialization formats here will lead
> to
> > a
> > bit of a tower of babel.
> > 2. This proposal still includes a pretty big change to our
> serialization
> > and protocol definition layer. Essentially it is introducing an
> optional
> > type, where the format is data dependent. I think this is actually a
> big
> > change though it doesn't seem like it. It means you can no longer
> > specify
> > this type with our type definition DSL, and likewise it requires
> custom
> > handling in client libs. This isn't a huge thing, since the Record
> > definition is custom anyway, but I think this kind of protocol
> > inconsistency is very non-desirable and ties you to hand-coding
> things.
> > I
> > think the type should instead by [Key Value] in our BNF, where key
and
> > value are both short strings as used elsewhere. This brings it in
line
> > with
> > the rest of the protocol.
> > 3. Could we get more specific about the exact Java API change to
> > ProducerRecord, ConsumerRecord, Record, etc?
> >
> > -Jay
> >
>
The information contained in this email is strictly confidential and for the
use of the addressee only, unless otherwise indicated. If you are not the
intended recipient, please do not read, copy, use or disclose to others this
message or any attachment. Please also notify the sender by replying to this
email or by telephone (+44(020 7896 0011) and then delete the email and any
copies of it. Opinions, conclusion (etc) that do not relate to the official
business of this company shall be understood as neither given nor endorsed by
it. IG is a trading name of IG Markets Limited (a company registered in England
and Wales, company number 04008957) and IG Index Limited (a company registered
in England and Wales, company number 01190902). Registered address at Cannon
Bridge House, 25 Dowgate Hill, London EC4R 2YA. Both IG Markets Limited
(register number 195355) and IG Index Limited (register number 114059) are
authorised and regulated by the Financial Conduct Authority.
