gaborgsomogyi commented on PR #28112:
URL: https://github.com/apache/flink/pull/28112#issuecomment-4388127239
Hi @Samrat002, great work reducing IOPS via lazy seek and range requests!
While reviewing the implementation I found one remaining efficiency issue worth
addressing.
What's working well
A seek that lands within the read-ahead buffer is served entirely from the
buffer - no extra `GetObject` call, no bytes pulled from S3. The attached patch
includes `seekWithinBuffer_afterSmallRead_doesNotTouchUnderlyingStream` which
demonstrates this and passes.
Remaining issue: bulk reads bypass the `BufferedInputStream` buffer
`BufferedInputStream.read(byte[], off, len)` has an internal fast-path: when
`len >= bufferSize` it reads directly from the underlying stream, skipping the
local buffer entirely. After such a read the buffer is empty.
This means a forward seek that follows a bulk read cannot be satisfied from
the buffer and falls through to `bufferedStream.skip()`, which consumes bytes
from the live HTTP connection. No new `GetObject` is issued (IOPS are fine),
but unnecessary bytes are downloaded and discarded.
The attached patch includes
`seekWithinBuffer_afterLargeRead_touchesUnderlyingStream` which demonstrates
the bug - it currently fails because `bytesSkippedFromUnderlying` is `10`
instead of `0`.
Suggested fix
Replace `BufferedInputStream` with a plain `byte[]` buffer managed directly,
tracking `bufferStart` (file offset of the first byte in the buffer),
`bufferOffset` (current read position within the array), and `bufferLength`
(valid bytes). Then `lazySeek()` can check `[bufferStart, bufferStart +
bufferLength)` to decide whether the seek is satisfiable in-buffer,
independently of whether the preceding read was a byte-at-a-time or a bulk read.
The patch file adds the `AtomicLong` counters to `TrackingInputStream` and
both test methods.
[buffer-efficiency-tests.patch](https://github.com/user-attachments/files/27439474/buffer-efficiency-tests.patch)
--
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]
