LinShunKang commented on PR #12683: URL: https://github.com/apache/kafka/pull/12683#issuecomment-1262595051
> I re-run the test case in trunk as well as in your branch locally with IDE, and both passes, so I think it's due to jenkins machines compute resource limit itself. > > But I got a separate question for the fix itself: normally when a bytebuffer was allocated first, it was in `write` mode, and hence we would expect to first `flip` it before reading data in serialization. But what if a caller `flip` it first before calling the serde, in order to make it in `read` mode? > > No test failures give us some confidence that probably no one is yet calling it that way, but we have to make sure no one will use it by first `flip` themselves as well. So I think it's worth adding a few comments on top of the `ByteBufferSerializer` emphasizing what it exactly is trying to do (i.e. if it would try to reset limit / position / mark), and that callers does not need to make it `read` before calling serialize. > > Could you add such docs into the class before merging it? I made some changes to `ByteBufferSerializer#serialize(String, ByteBuffer)` to keep the `ByteBuffer` offsets unchanged. And, I also add docs for ByteBufferSerializer. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
