Thank you for your comment, Divij.
4. I have added JavaDoc for
`ByteBufferSerializer#serializeToByteBuffer(String, ByteBuffer)` in the
'Public Interfaces' section, PTAL.
Best,
ShunKang
Divij Vaidya 于2023年4月28日周五 21:06写道：
> Thank you. I have a small nit otherwise the proposal looks good to me.
Thank you. I have a small nit otherwise the proposal looks good to me.
4. nit -> please add javadoc in the KIP as well since we are voting on KIP
in this discussion thread and not on the PR.
--
Divij Vaidya
On Fri, Apr 28, 2023 at 1:28 PM ShunKang Lin
wrote:
> Thank you for your comment, Div
Thank you for your comment, Divij.
4. I have added JavaDoc for
`ByteBufferSerializer#serializeToByteBuffer(String, ByteBuffer)`, commit
was here:
https://github.com/apache/kafka/pull/12685/commits/a57e0a31c10a5ba49bc2e735b389188e6e071f62,
PTAL.
6. I have added this description in the third parag
4. I did not observe any changes made to the KIP about this. Could we
please add a JavaDoc to serializeToByteBuffer() where we clearly call out
that the indices of input ByteBuffer will be modified.
5. Thank you. My comment on this is resolved now.
6. We should probably mention the strategy to
re
Thank you for your comment, Divij.
4. Do you still have any questions about #4?
5. I add test case for ByteBufferSerializer backward compatibility:
https://github.com/apache/kafka/pull/12685/commits/393af38c27ec8d810a2326ac4b89a53b177e3ee1
Best,
ShunKang
Divij Vaidya 于2023年4月19日周三 00:45写道：
>
3. Ok. Seems like there is no way around to enforce better semantics and
maintain backward compatibility as well! Let's go ahead with what you
proposed and create a JIRA to fix the semantics in version 4.x. My comment
is resolved here.
--
Divij Vaidya
On Mon, Apr 10, 2023 at 6:47 AM ShunKang Li
Thanks for your comment.
This KIP does not modify ByteBufferSerializer#serialize(), so do we need to
clarify this aspect on motivation?
Best,
ShunKang
Ismael Juma 于2023年4月10日 周一12:37写道：
> Hi,
>
> One interesting aspect is that the current `ByteBufferSerializer` avoids
> copies in the following
Hi,
One interesting aspect is that the current `ByteBufferSerializer` avoids
copies in the following case:
if (data.hasArray()) {
final byte[] arr = data.array();
if (data.arrayOffset() == 0 && arr.length == data.remaining()) {
return arr;
}
}
It would be good to clarify this aspect in the motiv
Sorry for the late reply due to recent work overload.
3. If the ByteBufferSerializer#serializeToByteBuffer(String, ByteBuffer)
method does not maintain the same behavior as the
ByteBufferSerializer#serialize(String, ByteBuffer) method, it will break
the code logic for those users who originally us
Thanks John!
I will consider your opinion, I’ve been busy at work lately.
Best,
ShunKang
John Roesler 于2022年11月6日 周日23:22写道：
> Thanks for the reply, ShunKang!
>
> You’re absolutely right, we should not change the behavior of the existing
> method.
>
> Regarding the new method, I was thinking th
Apologies for the late reply. I will be more proactive on this thead moving
ahead.
3. Understood. Perhaps, `ByteBuffer#asReadOnlyBuffer()` is not the right
solution and I acknowledge that the current API modifies the offsets of the
user provided input when it calls flip(). I still believe that we
Thanks for the reply, ShunKang!
You’re absolutely right, we should not change the behavior of the existing
method.
Regarding the new method, I was thinking that this is a good opportunity to
correct what seems to be strange semantics in the original one. If we keep the
same semantics and want
Hi John,
Thanks for your comments!
For your first question, I see some unit test cases that give us a
ByteBuffer not set to read before calling
`ByteBufferSerializer#serialize(String, ByteBuffer)`, e.g.
`ArticleSerializer`, `AugmentedArticleSerializer`,
`AugmentedCommentSerializer` and `CommentSe
Hi ShunKang,
Thanks for the KIP!
I’ve been wanting to transition toward byte buffers for a while, so this is a
nice start.
I thought it was a bit weird to flip the buffer inside the serializer, but I
see the existing one already does that. I would have thought it would make more
sense for th
Bump this thread again : )
ShunKang Lin 于2022年9月25日 周日23:59写道：
> Hi all, I'd like to start a new discussion thread on KIP-872 (Kafka
> Client) which proposes that add Serializer#serializeToByteBuffer() to
> reduce memory copying.
>
> KIP:
> https://cwiki.apache.org/confluence/pages/viewpage.actio
Bump this thread to see if there are any comments/thoughts.
Best,
ShunKang
ShunKang Lin 于2022年9月30日周五 23:58写道：
> Hi Divij Vaidya,
>
> 3. Sounds good, but `ByteBuffer#asReadOnlyBuffer()` returns a read-only
> `ByteBuffer` which `ByteBuffer#hasArray()` returns false, then it will make
> `Utils#w
Hi Divij Vaidya,
3. Sounds good, but `ByteBuffer#asReadOnlyBuffer()` returns a read-only
`ByteBuffer` which `ByteBuffer#hasArray()` returns false, then it will make
`Utils#writeTo(DataOutput, ByteBuffer, int)` perform efficiently Lower
(called in `DefaultRecord#writeTo(DataOutputStream, int, long,
1. You are right. We append the message to the `DefaultRecord` and append
is a copy operation. Hence, the ByteBuffer would be released at the end of
the KafkaProducer#doSend() method. This comment is resolved.
2. I don't foresee any compatibility issues since #1 is not a problem
anymore. This comme
Hi Divij Vaidya,
Thanks for your comments.
1. I checked the code of KafkaProducer#doSend()
and RecordAccumulator#append(), if KafkaProducer#doSend() returns it means
serializedKey and serializedValue have been appended to
ProducerBatch#recordsBuilder and we don't keep reference of serializedKey
a
Hello
I believe that the current behaviour of creating a copy of the user
provided input is the correct approach because of the following reasons:
1. In the existing implementation (considering cases when T is ByteBuffer
in Serializer#serialize(String,Headers,T)) we copy the data (T) into a new
b
Hi all, I'd like to start a new discussion thread on KIP-872 (Kafka Client)
which proposes that add Serializer#serializeToByteBuffer() to reduce memory
copying.
KIP:
https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=228495828
Thanks, ShunKang
21 matches
Mail list logo
