The current producer interceptor API is this: ProducerRecord<K, V> onSend(ProducerRecord<K, V> record);
So adding a header means creating a new ProducerRecord with a new header added to the current headers and returning it. Would that not work? -Jason On Wed, Feb 22, 2017 at 1:45 PM, Michael Pearce <michael.pea...@ig.com> wrote: > So how would you have this work if not mutable where interceptors would > add headers? > > Sent using OWA for iPhone > ________________________________________ > From: Jason Gustafson <ja...@confluent.io> > Sent: Wednesday, February 22, 2017 8:42:27 PM > To: dev@kafka.apache.org > Subject: Re: [DISCUSS] KIP-82 - Add Record Headers > > I think the point on the mutability of Headers is worth discussing a little > more. As far as I can tell, once the ProducerRecord (or ConsumerRecord) is > constructed, there should be no need to further change the headers. Is that > correct? If so, then why not enforce that that is the case through the API? > One problem with mutability it that it constrains the implementation of > Headers. For example, if we were backing with a byte slice, would we recopy > the bytes if a header is added or would we maintain a satellite collection > of added records. Seems not great either way. If we really think mutability > is needed, perhaps we could add a method to headers to convert it to a > mutable type (e.g. a Headers.toMap)? > > I'm also with Ismael about exposing Headers.get(). I thought it might make > sense to have a method like this instead: > > Iterable<Header> findMatching(Pattern pattern); > > This makes the (potential) need to scan the headers clear in the API. I'd > also be fine exposing no getter at all. In general, Ï think it's good to > start with a minimalistic API and work from there since it's always easier > to add methods than remove them. > > -Jason > > On Wed, Feb 22, 2017 at 9:16 AM, Michael Pearce <michael.pea...@ig.com> > wrote: > > > > > Hi Ismael > > > > On point 1, > > > > Sure makes sense will update shortly. > > > > On point 2, > > > > Setter/getter typical to properties/headers api’s traditionally are map > > styled interfaces and what I believe is most expected styled thus the > Key, > > Value setter. > > Also it would mean rather than an interface, we would be making our > > internal header impl object we have for the array, exposed. E.g. if we > had > > a Map really this would be Map.Entry interface, this is the same reasons > on > > the map interface I cannot actually make the underlying Node object > that’s > > the implementation for HashMap, so that internals can be changed. > > > > > > On point 3, > > > > I think it people do expect it to be performant, thus why originally > > concern I raised with using an array for to me is an early memory > > optimisation. I think the user experience of properties/headers is on a > > get/set model. This is why its important we have encapsulated logic that > > then allows us to change this to a map, if this becomes and issue, and > the > > memory overhead of hashmap is less so. > > > > > > > > > > On 22/02/2017, 15:56, "isma...@gmail.com on behalf of Ismael Juma" < > > isma...@gmail.com on behalf of ism...@juma.me.uk> wrote: > > > > Hi all, > > > > Great to see the progress that has been achieved on this one. :) A > few > > comments regarding the APIs (I'm still reviewing the message format > > changes): > > > > 1. Nit: `getHeaders` in `ProducerRecord` and `ConsumerRecord` should > be > > named `headers` (we avoid the `get` prefix in Kafka) > > > > 2. The `Headers` class is mutable (there's an `add` method). Does it > > need > > to be? If so, it would be good to explain why. Related to that, we > > should > > also explain the thinking around thread-safety. If we keep the `add` > > method, it may make sense for it to take a `Header` (that way we can > > add > > things to `Header` without changing the interface). > > > > 3. Do we need the `Headers.get()` method? People usually assume that > > `get` > > would be efficient, but depending on the implementation (the current > > proposal states that an array would be used), it may not be. If we > > expect > > the number of headers to be small, it doesn't matter though. > > > > Ismael > > > > On Tue, Feb 21, 2017 at 6:38 PM, Michael Pearce < > michael.pea...@ig.com > > > > > wrote: > > > > > Hi Jason, > > > > > > Have converted the interface/api bullets into interface code > > snippets. > > > > > > Agreed implementation won’t take too long. We have early versions > > already. > > > Maybe a week before you think about merging I would assume it would > > be more > > > stabilised? I was thinking then we could fork from your confluent > > branch, > > > making and then holding KIP-82 changes in a patch file, that we can > > then > > > re-fork from apache once KIP98 final is merged, and apply patch > with > > last > > > minute changes. > > > > > > Cheers > > > Mike > > > > > > > > > On 22/02/2017, 00:56, "Jason Gustafson" <ja...@confluent.io> > wrote: > > > > > > Hey Michael, > > > > > > Awesome. I have a minor request. The APIs are currently > > documented as a > > > wiki list. Would you mind adding a code snippet instead? It's a > > bit > > > easier > > > to process. > > > > > > How will be best to manage this, as we will obviously build off > > your > > > KIP’s > > > > protocol changes, to avoid a merge hell, should we branch > from > > your > > > branch > > > > in the confluent repo or is it worth having a KIP-98 special > > branch > > > in the > > > > apache git, that we can branch/fork from? > > > > > > > > > I was thinking about this also. Ideally we'd like to get the > > changes > > > in as > > > close together as possible since we only want one magic bump > and > > some > > > users > > > deploy trunk. The level of effort to change the format for > > headers > > > seems > > > not too high. Do you agree? My guess is that the KIP-98 message > > format > > > patch will take 2-3 weeks to review before we merge to trunk, > so > > you > > > could > > > hold off implementing until that patch has somewhat stabilized. > > That > > > would > > > save some potential rebase pain. > > > > > > -Jason > > > > > > > > > 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. >
