You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Chia-Ping Tsai <ch...@apache.org> on 2020/08/18 10:39:53 UTC

KIP-658 Deprecate all setters of Headers

Hi everyone,

I would like to start a discussion on KIP-658: https://cwiki.apache.org/confluence/x/-DJ4CQ

KIP-658 plans to deprecate all setters of Headers and provide default implementation. The benefit of this KIP are shown below.

1. Users don't need to implement those setters which are not used by kafka.
2. open a room to refactor Headers in next major (for example, a pojo Headers)

feedback is welcome!!!

----
chia-ping

Re: KIP-658 Deprecate all setters of Headers

Posted by "Matthias J. Sax" <mj...@apache.org>.
Well, I am not totally sure if immutability is actually desired for
`Headers`?

Consider the case when there is an application that just wants to add
for example a tracing header field. It would just call `add()` on the
consumer record's headers and pass the modified `Headers` object to the
`ProducerRecord` constructor without the need to deep-copy (to be fair,
there is no real deep copy as we would only copy some references).

Or an application which multiple components and one component creates a
`ProducerRecord`, some other component may or may not add some
additional headers, and a third component does the actually send?

I am wondering if this KIP might be a case of premature optimization? Is
there an actual issue you have observed with mutable headers? -- It
might also be worth to go back to the original headers KIP to see if
there was a discussion about immutability and what the reasoning for the
current design was?


-Matthias

On 8/18/20 7:42 PM, Chia-Ping Tsai wrote:
>> If you create a new `ProducerRecord` you would not pass in a `Headers`
>> object but a `Iterable<Header>`?
>>
>> And for `ConsumerRecord` you just get a `Headers` object back and don't
>> need to worry about the concrete implementation?
> 
> You are right. I'd give a bad case :(
> 
> For another reason, could we avoid using a mutable object to be the response type? ConsumerRecord and ProducerRecord have a good chance to be immutable if we can remove setters from Headers.  
> 
> On 2020/08/18 20:42:59, "Matthias J. Sax" <mj...@apache.org> wrote: 
>> Thanks for the KIP. However, I am not sure for what use-case you would
>> need to implement the `Headers` interface?
>>
>> If you create a new `ProducerRecord` you would not pass in a `Headers`
>> object but a `Iterable<Header>`?
>>
>> And for `ConsumerRecord` you just get a `Headers` object back and don't
>> need to worry about the concrete implementation?
>>
>>
>> -Matthias
>>
>>
>> On 8/18/20 3:39 AM, Chia-Ping Tsai wrote:
>>> Hi everyone,
>>>
>>> I would like to start a discussion on KIP-658: https://cwiki.apache.org/confluence/x/-DJ4CQ
>>>
>>> KIP-658 plans to deprecate all setters of Headers and provide default implementation. The benefit of this KIP are shown below.
>>>
>>> 1. Users don't need to implement those setters which are not used by kafka.
>>> 2. open a room to refactor Headers in next major (for example, a pojo Headers)
>>>
>>> feedback is welcome!!!
>>>
>>> ----
>>> chia-ping
>>>
>>
>>


Re: KIP-658 Deprecate all setters of Headers

Posted by Chia-Ping Tsai <ch...@apache.org>.
> If you create a new `ProducerRecord` you would not pass in a `Headers`
> object but a `Iterable<Header>`?
> 
> And for `ConsumerRecord` you just get a `Headers` object back and don't
> need to worry about the concrete implementation?

You are right. I'd give a bad case :(

For another reason, could we avoid using a mutable object to be the response type? ConsumerRecord and ProducerRecord have a good chance to be immutable if we can remove setters from Headers.  

On 2020/08/18 20:42:59, "Matthias J. Sax" <mj...@apache.org> wrote: 
> Thanks for the KIP. However, I am not sure for what use-case you would
> need to implement the `Headers` interface?
> 
> If you create a new `ProducerRecord` you would not pass in a `Headers`
> object but a `Iterable<Header>`?
> 
> And for `ConsumerRecord` you just get a `Headers` object back and don't
> need to worry about the concrete implementation?
> 
> 
> -Matthias
> 
> 
> On 8/18/20 3:39 AM, Chia-Ping Tsai wrote:
> > Hi everyone,
> > 
> > I would like to start a discussion on KIP-658: https://cwiki.apache.org/confluence/x/-DJ4CQ
> > 
> > KIP-658 plans to deprecate all setters of Headers and provide default implementation. The benefit of this KIP are shown below.
> > 
> > 1. Users don't need to implement those setters which are not used by kafka.
> > 2. open a room to refactor Headers in next major (for example, a pojo Headers)
> > 
> > feedback is welcome!!!
> > 
> > ----
> > chia-ping
> > 
> 
> 

Re: KIP-658 Deprecate all setters of Headers

Posted by "Matthias J. Sax" <mj...@apache.org>.
Thanks for the KIP. However, I am not sure for what use-case you would
need to implement the `Headers` interface?

If you create a new `ProducerRecord` you would not pass in a `Headers`
object but a `Iterable<Header>`?

And for `ConsumerRecord` you just get a `Headers` object back and don't
need to worry about the concrete implementation?


-Matthias


On 8/18/20 3:39 AM, Chia-Ping Tsai wrote:
> Hi everyone,
> 
> I would like to start a discussion on KIP-658: https://cwiki.apache.org/confluence/x/-DJ4CQ
> 
> KIP-658 plans to deprecate all setters of Headers and provide default implementation. The benefit of this KIP are shown below.
> 
> 1. Users don't need to implement those setters which are not used by kafka.
> 2. open a room to refactor Headers in next major (for example, a pojo Headers)
> 
> feedback is welcome!!!
> 
> ----
> chia-ping
>