Hi Becket,

Thanks for sharing your thoughts. More inline.

On Wed, Mar 1, 2017 at 2:54 AM, Becket Qin <becket....@gmail.com> wrote:

> 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.
>

The difference is that the user chooses the value type. They are free to
choose a mutable or immutable type. A generic interceptor cannot mutate the
value because it doesn't know the type (and it could be immutable). One
could write an interceptor that checked the type of the value at runtime
and did things based on that. But again, the user who creates the record is
in control.

>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.


Yes, with headers, we are providing a model for the user (the user doesn't
get to choose it like with keys and values) and for the interceptors. So, I
think it's not the same.


> 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.


If users want to take that risk, it's fine. But we give them the option to
protect themselves. With mutable headers, there is no option.


> 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.
>

I think the assumption that there would be tons of objects created is
incorrect (I suggested a solution that would only involve one additional
reference in the `Header` instance). The usability of the immutable API
seemed to be more of an issue.

In any case, if we do add the `close()` method, then we need to add a note
to the compatibility section stating that once a producer record is sent,
it cannot be sent again as this would cause interceptors that add headers
to fail.

Ismael

Reply via email to