Hi Renu, that is not completely true, the LZ4 compression codec was added without a MagicByte bump. (LZ4 might be a bad example though since this feature was added without updating the protocol docs..)
Unless the broker needs the MagicByte internally (for translating old logs on disk or whatever) I dont see a need for bumping the MagicByte when just adding a bit *definition* to an existing field. With just one MagicByte bump so far it is hard to say what is right or wrong, but this might be a good time for us to decide on some rules. I dont have a strong opinion, either way is fine with me. Regards, Magnus 2016-10-26 20:01 GMT+02:00 Renu Tewari <tewa...@gmail.com>: > +1 @Magnus. > It is also in line with traditional use of the magic field to indicate a > change in the format of the message. Thus a change in the magic field > indicates a different "schema" which in this case would reflect adding a > new field, removing a field or changing the type of fields etc. > > The version number bump does not change the message format but just the > interpretation of the values. > > Going with what @Michael suggested on keeping it simple we don't need to a > add a new field to indicate a tombstone and therefore require a change in > the magic byte field. The attribute bit is sufficient for indicating the > new interpretation of attribute.bit5 to indicate a tombstone. > > Bumping the version number and changing the attribute bit keeps it simple. > > > Thanks > Renu > > > > On Wed, Oct 26, 2016 at 10:09 AM, Mayuresh Gharat < > gharatmayures...@gmail.com> wrote: > > > I see the reasoning Magnus described. > > If you check the docs https://kafka.apache.org/ > documentation#messageformat > > , > > it says : > > > > 1 byte "magic" identifier to allow format changes, value is 0 or 1 > > > > Moreover as per comments in the code : > > /** > > * The "magic" value > > * When magic value is 0, the message uses absolute offset and does not > > have a timestamp field. > > * When magic value is 1, the message uses relative offset and has a > > timestamp field. > > */ > > > > Since timeStamp was added as an actual field we bumped the the magic byte > > to 1 for this change. > > But since we are not adding an actual field, we can do away with bumping > up > > the magic byte. > > > > If we really want to go the standard route of bumping up the magic byte > for > > any change to message format we should actually add a new field for > > handling log compaction instead of just using the attribute field, which > > might sound like an overkill. > > > > Thanks, > > > > Mayuresh > > > > > > > > > > > > > > On Wed, Oct 26, 2016 at 1:32 AM, Magnus Edenhill <mag...@edenhill.se> > > wrote: > > > > > 2016-10-25 21:36 GMT+02:00 Nacho Solis <nso...@linkedin.com.invalid>: > > > > > > > I think you probably require a MagicByte bump if you expect correct > > > > behavior of the system as a whole. > > > > > > > > From a client perspective you want to make sure that when you > deliver a > > > > message that the broker supports the feature you're expecting > > > > (compaction). So, depending on the behavior of the broker on > > > encountering > > > > a previously undefined bit flag I would suggest making some change to > > > make > > > > certain that flag-based compaction is supported. I'm going to guess > > that > > > > the MagicByte would do this. > > > > > > > > > > I dont believe this is needed since it is already attributed through > the > > > request's API version. > > > > > > Producer: > > > * if a client sends ProduceRequest V4 then attributes.bit5 indicates a > > > tombstone > > > * if a clients sends ProduceRequest <V4 then attributes.bit5 is > ignored > > > and value==null indicates a tombstone > > > * in both cases the on-disk messages are stored with attributes.bit5 > (I > > > assume?) > > > > > > Consumer: > > > * if a clients sends FetchRequest V4 messages are sendfile():ed > directly > > > from disk (with attributes.bit5) > > > * if a client sends FetchRequest <V4 messages are slowpathed and > > > translated from attributes.bit5 to value=null as required. > > > > > > > > > That's my understanding anyway, please correct me if I'm wrong. > > > > > > /Magnus > > > > > > > > > > > > > On Tue, Oct 25, 2016 at 10:17 AM, Magnus Edenhill < > mag...@edenhill.se> > > > > wrote: > > > > > > > > > It is safe to assume that a previously undefined attributes bit > will > > be > > > > > unset in protocol requests from existing clients, if not, such a > > client > > > > is > > > > > already violating the protocol and needs to be fixed. > > > > > > > > > > So I dont see a need for a MagicByte bump, both broker and client > has > > > the > > > > > information it needs to construct or parse the message according to > > > > request > > > > > version. > > > > > > > > > > > > > > > 2016-10-25 18:48 GMT+02:00 Michael Pearce <michael.pea...@ig.com>: > > > > > > > > > > > Hi Magnus, > > > > > > > > > > > > I was wondering if I even needed to change those also, as > > technically > > > > > > we’re just making use of a non used attribute bit, but im not > 100% > > > that > > > > > it > > > > > > be always false currently. > > > > > > > > > > > > If someone can say 100% it will already be set false with current > > and > > > > > > historic bit wise masking techniques used over the time, we could > > do > > > > away > > > > > > with both, and simply just start to use it. Unfortunately I don’t > > > have > > > > > that > > > > > > historic knowledge so was hoping it would be flagged up in this > > > > > discussion > > > > > > thread ☺ > > > > > > > > > > > > Cheers > > > > > > Mike > > > > > > > > > > > > On 10/25/16, 5:36 PM, "Magnus Edenhill" <mag...@edenhill.se> > > wrote: > > > > > > > > > > > > Hi Michael, > > > > > > > > > > > > With the version bumps for Produce and Fetch requests, do you > > > > really > > > > > > need > > > > > > to bump MagicByte too? > > > > > > > > > > > > Regards, > > > > > > Magnus > > > > > > > > > > > > > > > > > > 2016-10-25 18:09 GMT+02:00 Michael Pearce < > > michael.pea...@ig.com > > > >: > > > > > > > > > > > > > Hi All, > > > > > > > > > > > > > > I would like to discuss the following KIP proposal: > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP- > > > > > > > 87+-+Add+Compaction+Tombstone+Flag > > > > > > > > > > > > > > This is off the back of the discussion on KIP-82 / KIP > > meeting > > > > > > where it > > > > > > > was agreed to separate this issue and feature. See: > > > > > > > http://mail-archives.apache.org/mod_mbox/kafka-dev/201610. > > > > > > > mbox/%3cCAJS3ho8OcR==EcxsJ8OP99pD2hz=iiGecWsv- > > > > > > > EZsBsNyDcKr=g...@mail.gmail.com%3e > > > > > > > > > > > > > > Thanks > > > > > > > Mike > > > > > > > > > > > > > > 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. > > > > > > > > > > > > > > > > > > > > > > > > > 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. > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > Nacho (Ignacio) Solis > > > > Kafka > > > > nso...@linkedin.com > > > > > > > > > > > > > > > -- > > -Regards, > > Mayuresh R. Gharat > > (862) 250-7125 > > >
