Hi Colin, Thanks for the KIP! This is a significant improvement. One of my personal interests in this proposal is solving the compatibility problems we have with the internal schemas used to define consumer offsets and transaction metadata. Currently we have to guard schema bumps with the inter-broker protocol format. Once the format is bumped, there is no way to downgrade. By fixing this, we can potentially begin using the new schemas before the IBP is bumped while still allowing downgrade.
There are a surprising number of other situations we have encountered this sort of problem. We have hacked around it in special cases by allowing nullable fields to the end of the schema, but this is not really an extensible approach. I'm looking forward to having a better option. With that said, I have a couple questions on the proposal: 1. For each request API, we need one version bump to begin support for "flexible versions." Until then, we won't have the option of using tagged fields even if the broker knows how to handle them. Does it make sense to go ahead and do a universal bump of each request API now so that we'll have this option going forward? 2. The alternating length/tag header encoding lets us save a byte in the common case. The downside is that it's a bit more complex to specify. It also has some extra cost if the length exceeds the tag substantially. Basically we'd have to pad the tag, right? I think I'm wondering if we should just bite the bullet and use two varints instead. Thanks, Jason On Fri, Aug 9, 2019 at 4:31 PM Colin McCabe <cmcc...@apache.org> wrote: > Hi all, > > I've made some updates to this KIP. Specifically, I wanted to avoid > including escape bytes in the serialization format, since it was too > complex. Also, I think this is a good opportunity to slim down our > variable length fields. > > best, > Colin > > > On Thu, Jul 11, 2019, at 20:52, Colin McCabe wrote: > > On Tue, Jul 9, 2019, at 15:29, Jose Armando Garcia Sancio wrote: > > > Thanks Colin for the KIP. For my own edification why are we doing this > > > "Optional fields can have any type, except for an array of > structures."? > > > Why can't we have an array of structures? > > > > Optional fields are serialized starting with their total length. This > > is straightforward to calculate for primitive fields like INT32, (or > > even an array of INT32), but more difficult to calculate for an array > > of structures. Basically, we'd have to do a two-pass serialization > > where we first calculate the lengths of everything, and then write it > > out. > > > > The nice thing about this KIP is that there's nothing in the protocol > > stopping us from adding support for this feature in the future. We > > wouldn't have to really change the protocol at all to add support. But > > we'd have to change a lot of serialization code. Given almost all of > > our use-cases for optional fields are adding an extra field here or > > there, it seems reasonable not to support it for right now. > > > > best, > > Colin > > > > > > > > -- > > > -Jose > > > > > >
