Thanks Ismael for spotting from the discuss thread. Extra details documented on wiki.
Cheers Mike ________________________________________ From: isma...@gmail.com <isma...@gmail.com> on behalf of Ismael Juma <ism...@juma.me.uk> Sent: Friday, March 24, 2017 9:36:34 AM To: dev@kafka.apache.org Subject: Re: [VOTE] KIP-82 Add Record Headers Hi Michael, Can you please update the wiki page to cover the following 2 points that were discussed in the mailing list thread, but should be there too: 1. How to create an instance of a header to pass to the `add` method. You suggested doing it via the implementation class. If we do it that way, then the implementation class is also a public interface and needs to be included in the KIP. If we go with my suggestion of using a create method in the Header interface, we avoid that issue. 2. Given the mutability of the headers and the fact that we close them at some point, it means that users should not resend the same record if they use headers and interceptors. They should either configure the producer so that automatic retries are attempted (preferred although there are some known limitations regarding records that are timed out before a send is actually attempted) or they need to create a new record while being careful about what headers they include (if they rely fully on interceptors for headers, they could just not include any headers in the new headers). This information should be included in the compatibility section. Thanks! Ismael On Fri, Mar 24, 2017 at 8:29 AM, Michael Pearce <michael.pea...@ig.com> wrote: > Hi Jun, > > Thanks for your vote, I’ve updated the wiki to document this detail. > > Cheers > Mike > > On 24/03/2017, 05:00, "Jun Rao" <j...@confluent.io> wrote: > > Hi, Ismael, > > Ok, that make sense. > > > Hi, Michael, > > Could we document this in the wiki? > > +1 from me on the KIP. > > Thanks, > > Jun > > On Thu, Mar 23, 2017 at 7:58 PM, Ismael Juma <ism...@juma.me.uk> > wrote: > > > Hi Jun, > > > > The close method will only exist in the implementation class as it's > not > > meant to be called from user code. > > > > Ismael > > > > On 24 Mar 2017 1:51 am, "Jun Rao" <j...@confluent.io> wrote: > > > > > Hi, Michael, > > > > > > The KIP looks good to me overall. Just one comment. The wiki says > "This > > > will be done by calling "close()" method". However, there is no > close() > > in > > > Headers. > > > > > > Thanks, > > > > > > Jun > > > > > > On Thu, Mar 23, 2017 at 9:34 AM, Michael Pearce < > michael.pea...@ig.com> > > > wrote: > > > > > > > Thanks all for the +1 votes so far, just one more committer vote > > needed ( > > > > > > > > Please note: > > > > > > > > I have made one small adjustment to the kip based on Ismael’s > comment > > in > > > > discussion thread, and further requested by Jason in the vote > thread. > > > > > > > > Please note the below method is changed based on this feedback. > > > > > > > > Headers.header(key)` to be `Headers.lastHeader(key) > > > > > > > > Thanks > > > > Mike > > > > > > > > On 22/03/2017, 16:39, "Joel Koshy" <jjkosh...@gmail.com> wrote: > > > > > > > > +1 > > > > > > > > On Tue, Mar 21, 2017 at 5:01 PM, Jason Gustafson < > > ja...@confluent.io > > > > > > > > wrote: > > > > > > > > > Thanks for the KIP! +1 (binding) from me. Just one nit: > can we > > > change > > > > > `Headers.header(key)` to be `Headers.lastHeader(key)`? > It's not a > > > > > deal-breaker, but I think it's better to let the name > reflect the > > > > actual > > > > > behavior as clearly as possible. > > > > > > > > > > -Jason > > > > > > > > > > On Wed, Feb 15, 2017 at 6:10 AM, Jeroen van Disseldorp < > > > > jer...@axual.io> > > > > > wrote: > > > > > > > > > > > +1 on introducing the concept of headers, neutral on > specific > > > > > > implementation. > > > > > > > > > > > > > > > > > > > > > > > > On 14/02/2017 22:34, Jay Kreps wrote: > > > > > > > > > > > >> 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 > > > > > >> > > > > > >> On Tue, Feb 14, 2017 at 9:42 AM, Michael Pearce < > > > > michael.pea...@ig.com> > > > > > >> wrote: > > > > > >> > > > > > >> Hi all, > > > > > >>> > > > > > >>> We would like to start the voting process for KIP-82 – > Add > > > record > > > > > >>> headers. > > > > > >>> The KIP can be found > > > > > >>> at > > > > > >>> > > > > > >>> https://cwiki.apache.org/confluence/display/KAFKA/KIP- > > > > > >>> 82+-+Add+Record+Headers > > > > > >>> > > > > > >>> Discussion thread(s) can be found here: > > > > > >>> > > > > > >>> http://search-hadoop.com/m/ > Kafka/uyzND1nSTOHTvj81?subj= > > > > > >>> Re+DISCUSS+KIP+82+Add+Record+Headers > > > > > >>> http://search-hadoop.com/m/ > Kafka/uyzND1Arxt22Tvj81?subj= > > > > > >>> Re+DISCUSS+KIP+82+Add+Record+Headers > > > > > >>> http://search-hadoop.com/?project=Kafka&q=KIP-82 > > > > > >>> > > > > > >>> > > > > > >>> > > > > > >>> 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. > > > > > > > > > > > > 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.
