Re: [PR] KAFKA-14945: Add Serializer#serializeToByteBuffer() to reduce memory copying [kafka]
LinShunKang commented on PR #12685:
URL: https://github.com/apache/kafka/pull/12685#issuecomment-1929526582
> @LinShunKang , sorry for being late. I had a quick look at #14617, it
looks like the `ByteBufferSerializer#serialize` is a public API and cannot be
changed without KIP. You know more than I do, so, in your opinion, what should
we do from now? Could we not touch the `ByteBufferSerializer#serialize` and
only implement `ByteBufferSerializer#serializeToByteBuffer`? Do you think we
should include people in this [PR](https://github.com/apache/kafka/pull/14617)
to this PR for discussion? Or do you have any other thoughts?
We could not only implement `ByteBufferSerializer#serializeToByteBuffer`
because if `Serializer` implements this method, then the `Serializer#serialize`
will never be called. And `ByteBufferSerializer#serialize` has obvious logical
problems.
I believe we should address the logical issues in
`ByteBufferSerializer#serialize`, but this will introduce breaking changes for
existing users. And the `Serializer` should not have both `serialize` and
`serializeToByteBuffer` methods at the same time. Therefore, I suggest we
tackle these issues in Kafka 4.0, where we can modify the signature of the
`Serializer`:
from
```
//before 4.0
public interface Serializer {
byte[] serialize(String topic, T data);
default byte[] serialize(String topic, Headers headers, T data) {
return serialize(topic, data);
}
}
```
to
```
//since 4.0
public interface Serializer {
ByteBuffer serialize(String topic, T data);
ByteBuffer serialize(String topic, Headers headers, T data);
}
```
Then we could announce that we are modified the signature of the
`Serializer` for existing users.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] KAFKA-14945: Add Serializer#serializeToByteBuffer() to reduce memory copying [kafka]
showuon commented on PR #12685: URL: https://github.com/apache/kafka/pull/12685#issuecomment-1926319143 @LinShunKang , sorry for being late. I had a quick look at https://github.com/apache/kafka/pull/14617, it looks like the `ByteBufferSerializer#serialize` is a public API and cannot be changed without KIP. You know more than I do, so, in your opinion, what should we do from now? Could we keep the `ByteBufferSerializer#serialize` and implement `ByteBufferSerializer#serializeToByteBuffer`? Do you think we should include people in this [PR](https://github.com/apache/kafka/pull/14617) to this PR for discussion? Or do you have any other thoughts? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] KAFKA-14945: Add Serializer#serializeToByteBuffer() to reduce memory copying [kafka]
LinShunKang commented on PR #12685: URL: https://github.com/apache/kafka/pull/12685#issuecomment-1925307450 > @LinShunKang It looks like you need to rebase this PR to resolve conflicts. Ping @divijvaidya @showuon @dengziming, can you help get this merged? Thanks @mimaison I would like to resolve conflicts, but due to the presence of [#14617](https://github.com/apache/kafka/pull/14617), I don't know how to implement `ByteBufferSerializer#serializeToByteBuffer(String, ByteBuffer)`. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] KAFKA-14945: Add Serializer#serializeToByteBuffer() to reduce memory copying [kafka]
mimaison commented on PR #12685: URL: https://github.com/apache/kafka/pull/12685#issuecomment-1923787527 @LinShunKang It looks like you need to rebase this PR to resolve conflicts. Ping @divijvaidya @showuon @dengziming, can you help get this merged? Thanks -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] KAFKA-14945: Add Serializer#serializeToByteBuffer() to reduce memory copying [kafka]
mimaison commented on PR #12685: URL: https://github.com/apache/kafka/pull/12685#issuecomment-1862895717 @divijvaidya @showuon @dengziming You voted on the KIP ([KIP-872](https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=228495828)), can you help this PR progress? It has been opened for over a year! Thanks -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
