Re: [DISCUSS] KIP-863: Reduce Fetcher#parseRecord() memory copy

Thanks ShunKang, that's a good point about ByteBuffer::hasArray. LGTM! On Wed, Oct 12, 2022 at 11:12 AM ShunKang Lin wrote: > Hi Chris, > > 1. Record keys/values are duplicated from `DefaultRecordBatch#buffer`, so > modifying key/value offsets will not modify the original ByteBuffer > offsets.

Re: [DISCUSS] KIP-863: Reduce Fetcher#parseRecord() memory copy

Hi Chris, 1. Record keys/values are duplicated from `DefaultRecordBatch#buffer`, so modifying key/value offsets will not modify the original ByteBuffer offsets. A read-only ByteBuffer calls `ByteBuffer#hasArray()` to return false, which means that a read-only ByteBuffer does not expose the

Re: [DISCUSS] KIP-863: Reduce Fetcher#parseRecord() memory copy

Hi Luke, Thanks for your comments. Best, ShunKang Luke Chen 于2022年10月11日周二 20:36写道： > Hi ShunKang, > > Had a quick look, I think It's a good idea. > I'll check it again tomorrow, and let you know if I have any questions. > > Luke > > On Sun, Sep 25, 2022 at 3:35 PM ShunKang Lin > wrote: > >

Re: [DISCUSS] KIP-863: Reduce Fetcher#parseRecord() memory copy

Hi ShunKang, Thanks for the KIP! I have a couple thoughts: 1. If we pass the ByteBuffer that we're using internally for the record key/value to the deserializer, it may be mutated by writing new bytes or altering the position. Should we permit this, or would it make sense to provide

Re: [DISCUSS] KIP-863: Reduce Fetcher#parseRecord() memory copy

Hi ShunKang, Had a quick look, I think It's a good idea. I'll check it again tomorrow, and let you know if I have any questions. Luke On Sun, Sep 25, 2022 at 3:35 PM ShunKang Lin wrote: > Hi Guozhang, > > When I try add method `default ByteBuffer serializeToByteBuffer(String > topic, Headers

Re: [DISCUSS] KIP-863: Reduce Fetcher#parseRecord() memory copy

Hi Guozhang, When I try add method `default ByteBuffer serializeToByteBuffer(String topic, Headers headers, T data)` for `ByteBufferSerializer`, I found `ByteBufferSerializer#serialize(String, ByteBuffer)` is not correct. Then I searched JIRA and found this:

Re: [DISCUSS] KIP-863: Reduce Fetcher#parseRecord() memory copy

Thanks Guozhang! Best, ShunKang Guozhang Wang 于2022年9月23日周五 00:27写道： > Could you start a separate VOTE email thread calling for votes? > > On Thu, Sep 22, 2022 at 9:19 AM ShunKang Lin > wrote: > > > Hi Guozhang, > > > > Thanks for your help! By the way, what should I do next? > > > > Best, >

Re: [DISCUSS] KIP-863: Reduce Fetcher#parseRecord() memory copy

Could you start a separate VOTE email thread calling for votes? On Thu, Sep 22, 2022 at 9:19 AM ShunKang Lin wrote: > Hi Guozhang, > > Thanks for your help! By the way, what should I do next? > > Best, > ShunKang > > Guozhang Wang 于2022年9月22日周四 23:21写道： > > > Thanks ShunKang, > > > > I made a

Re: [DISCUSS] KIP-863: Reduce Fetcher#parseRecord() memory copy

Hi Guozhang, Thanks for your help! By the way, what should I do next? Best, ShunKang Guozhang Wang 于2022年9月22日周四 23:21写道： > Thanks ShunKang, > > I made a few nit edits on the Motivation section as well. LGTM for me now. > > On Thu, Sep 22, 2022 at 7:33 AM ShunKang Lin > wrote: > > > Hi

Re: [DISCUSS] KIP-863: Reduce Fetcher#parseRecord() memory copy

Thanks ShunKang, I made a few nit edits on the Motivation section as well. LGTM for me now. On Thu, Sep 22, 2022 at 7:33 AM ShunKang Lin wrote: > Hi Guozhang, > > I've updated the "Motivation" section of the KIP, please take a look. > > Thanks. > ShunKang > > Guozhang Wang 于2022年9月21日周三

Re: [DISCUSS] KIP-863: Reduce Fetcher#parseRecord() memory copy

Hi Guozhang, I've updated the "Motivation" section of the KIP, please take a look. Thanks. ShunKang Guozhang Wang 于2022年9月21日周三 01:26写道： > In this case, could you update the KIP to clarify the allocation savings > more clearly in the "Motivation" section? Also you could mention that for >

Re: [DISCUSS] KIP-863: Reduce Fetcher#parseRecord() memory copy

In this case, could you update the KIP to clarify the allocation savings more clearly in the "Motivation" section? Also you could mention that for user customizable serdes, if they could provide overwrites on the overloaded function that's also possible for optimize memory allocations. Guozhang

Re: [DISCUSS] KIP-863: Reduce Fetcher#parseRecord() memory copy

1. Ack, thanks. 2. Sounds good, thanks for clarifying. On Tue, Sep 20, 2022 at 9:50 AM ShunKang Lin wrote: > Hi Guozhang, > > Thanks for your comments! > > 1. We can reduce memory allocation if the key/value types happen to be > ByteBuffer or String. > 2. I would like to add `default ByteBuffer

Re: [DISCUSS] KIP-863: Reduce Fetcher#parseRecord() memory copy

Hi Guozhang, Thanks for your comments! 1. We can reduce memory allocation if the key/value types happen to be ByteBuffer or String. 2. I would like to add `default ByteBuffer serializeToByteBuffer(String topic, Headers headers, T data)` in Serializer to reduce memory copy in

Re: [DISCUSS] KIP-863: Reduce Fetcher#parseRecord() memory copy

A separate question regarding the proposed API as well: what do you think to also augment the serializers with ByteBuffer return type in order to be symmetric with deserializers? On Mon, Sep 19, 2022 at 3:32 PM Guozhang Wang wrote: > Hello ShunKang, > > Thanks for filing the proposal, and

Re: [DISCUSS] KIP-863: Reduce Fetcher#parseRecord() memory copy

Hello ShunKang, Thanks for filing the proposal, and sorry for the late reply! I looked over your KIP proposal and the PR, in general I think I agree that adding an overloaded function with `ByteBuffer` param is beneficial, but I have a meta question regarding it's impact on Kafka consumer: my

[DISCUSS] KIP-863: Reduce Fetcher#parseRecord() memory copy

Hi all， I'd like to start a discussion on KIP-863 which is Reduce Fetcher#parseRecord() memory copy. This KIP can reduce Kafka Consumer memory allocation by nearly 50% during fetch records. Please check https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=225152035 and