Hi Ismael, Yes, there is a difference between Batch and Headers. I was just trying to see if that would work. Good point about sending the same ProducerRecord twice, but in fact in that case any reuse of objects would cause problem.
As you can imagine if the ProducerRecord has a value as a List and the Interceptor.onSend() can actually add an element to the List. If the producer.send() is called on the same ProducerRecord again, the value list would have one more element than the previously sent ProducerRecord even though the ProducerRecord itself is not mutable, right? Same thing can apply to any modifiable class type. >From this standpoint allowing headers to be mutable doesn't really weaken the mutability we already have. Admittedly a mutable header is kind of guiding user towards to change the headers in the existing object instead of creating a new one. But I think reusing an object while it is possible to be modified by user code is a risk that users themselves are willing to take. And we do not really protect against that. But there still seems value to allow the users to not pay the overhead of creating tons of objects if they do not reuse an object to send it twice, which is probably a more common case. Thanks, Jiangjie (Becket) Qin On Tue, Feb 28, 2017 at 12:43 PM, radai <radai.rosenbl...@gmail.com> wrote: > I will settle for any API really, but just wanted to point out that as it > stands right now the API targets the most "advanced" (hence obscure and > rare) use cases, at the expense of the simple and common ones. i'd suggest > (as the minimal set): > > Header header(String key) - returns JUST ONE (the very last) value given a > key > Iterable<Header> headers(String key) - returns ALL under a key > Iterable<Header> headers() - returns all, period. maybe allow null as key > to prev method instead? > void add(Header header) - appends a header (key inside). > void remove(String key) - removes ALL HEADERS under a key. > > this way naive get/set semantics map to header(key)/add(Header) cleanly and > simply while preserving the ability to handle more advanced use cases. > we can always add more convenience methods (like those dealing with lists - > addAll etc) but i think the 5 (potentially 4) above are sufficient for > basically everything. > > On Tue, Feb 28, 2017 at 4:08 AM, Ismael Juma <ism...@juma.me.uk> wrote: > > > Hi Becket, > > > > Comments inline. > > > > On Sat, Feb 25, 2017 at 10:33 PM, Becket Qin <becket....@gmail.com> > wrote: > > > > > > 1. Regarding the mutability. > > > > > > I think it would be a big convenience to have headers mutable during > > > certain stage in the message life cycle for the use cases you > mentioned. > > I > > > agree there is a material benefit especially given that we may have to > > > modify the headers for each message. > > > > > > That said, I also think it is fair to say that in the producer, in > order > > to > > > guarantee the correctness of the entire logic, it is necessary that at > > some > > > point we need to make producer record immutable. For example we > probably > > > don't want to see that users accidentally updated the headers when the > > > producer is doing the serialization or compression. > > > > > > Given that, would it be possible to make Headers to be able to switch > > from > > > mutable to immutable? We have done this for the Batch in the producer. > > For > > > example, initially the headers are mutable, but after it has gone > through > > > all the interceptors, we can call Headers.close() to make it immutable > > > afterwards. > > > > > > > The difference is that the batch is an internal class that is not exposed > > to users. Can you please explain what happens if a user tries to send the > > same ProducerRecord twice? Would an interceptor fail when trying to > mutate > > the header that is now closed? Or did you have something else in mind? > > > > Thanks, > > Ismael > > >
