Thanks John! I will consider your opinion, I’ve been busy at work lately.
Best, ShunKang John Roesler <vvcep...@apache.org>于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 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 to correct it later, we’ll be forced to > introduce yet another method later. This especially makes sense if we’re > thinking of deprecating the original method. But if you think it’s better > to keep it the way it is, I’m fine with it. > > I have no other comments. > > Thanks again for the KIP, > John > > On Sat, Nov 5, 2022, at 11:59, ShunKang Lin wrote: > > 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 `CommentSerializer`. If we don't flip > the > > ByteBuffer inside the `ByteBufferSerializer#serialize(String, > ByteBuffer)` > > it will break user code using `ByteBufferSerializer#serialize(String, > > ByteBuffer)`, and if we don't flip the ByteBuffer inside > > the `ByteBufferSerializer#serializeToByteBuffer(String, ByteBuffer)`, it > > will be even more strange to the user, because > > `ByteBufferSerializer#serialize(String, ByteBuffer)` and > > `ByteBufferSerializer#serializeToByteBuffer(String, ByteBuffer)` require > > users use the ByteBufferSerializer in two different ways. So if we think > of > > `ByteBufferSerialize#serializeToByteBuffer(String, ByteBuffer)` as > setting > > up a ByteBuffer to read later, is it more acceptable? > > > > For your second question, I plan to ultimately replace byte[] with > > ByteBuffer, I will document the intent in your KIP and JavaDocs later. > > > > I will clarify that if a Serializer implements the new method, then the > old > > one will never be called. > > > > Best, > > ShunKang > > > > John Roesler <vvcep...@apache.org> 于2022年11月4日周五 22:42写道： > > > >> 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 the caller to give us a buffer already set up for > >> reading. Do you think it makes sense to adopt this pattern for the new > >> method? > >> > >> Do you plan to keep the new methods as optional indefinitely, or do you > >> plan to ultimately replace byte[] with ByteBuffer? If it’s the latter, > then > >> it would be good to document the intent in your KIP and JavaDocs. > >> > >> It would be good to clarify that if a Serializer implements the new > >> method, then the old one will never be called. That way, implementations > >> can just throw an exception on that method instead of implementing both. > >> > >> Thanks again! > >> -John > >> > >> On Wed, Nov 2, 2022, at 20:14, ShunKang Lin wrote: > >> > Bump this thread again : ) > >> > > >> > ShunKang Lin <linshunkang....@gmail.com>于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.action?pageId=228495828 > >> >> Thanks, ShunKang > >> >> > >> >
