Jackie-Jiang commented on code in PR #20069:
URL: https://github.com/apache/pinot/pull/20069#discussion_r4407276621
##########
pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/memory/PinotDataBuffer.java:
##########
@@ -649,6 +650,16 @@ public ByteBuffer toDirectByteBuffer(long offset, int
size) {
return toDirectByteBuffer(offset, size, order());
}
+ /// Reads a range synchronously through a borrowed direct buffer with this
receiver's byte order.
+ /// The callback receives a buffer positioned at zero with its limit set to
`size`. It may change the position
+ /// and limit, but must not modify the contents, release the buffer, or
retain or return the buffer or any alias.
+ /// The callback must not recursively access or release this receiver or its
views. The borrowed buffer is only
+ /// valid during the callback; the result must be independent of it.
+ public <T> T readWithByteBuffer(long offset, int size,
IOFunction<ByteBuffer, T> reader)
Review Comment:
On reconsideration, I'd prefer the original `readWithByteBuffer(...)`. It
describes both the read operation and what the callback receives. My earlier
`withByteBuffer(...)` suggestion loses the read-only intent and sounds like a
builder method; sorry for the churn.
`readWithBorrowedBuffer(...)` is another option if we want borrowing
explicit in the name, but the exact lifetime restrictions still need Javadoc,
so I don't think it clearly improves on the original. My recommendation is to
keep `readWithByteBuffer(...)`. This remains non-blocking.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
