Copilot commented on code in PR #4841:
URL: https://github.com/apache/bookkeeper/pull/4841#discussion_r3608627379
##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/directentrylogger/DirectReader.java:
##########
@@ -226,8 +226,21 @@ void readBlock(long offset) throws IOException {
if ((bytesOutstanding - bytesRead) <= 0) {
break;
}
- bytesOutstanding -= bytesRead & Buffer.ALIGNMENT;
- bufferOffset += bytesRead & Buffer.ALIGNMENT;
+ long alignedBytesRead = bytesRead & ~(Buffer.ALIGNMENT - 1L);
+ if (alignedBytesRead <= 0) {
+ readBlockStats.registerFailedEvent(System.nanoTime() -
startNs, TimeUnit.NANOSECONDS);
+ throw new EOFException(exMsg("Short read did not make
aligned progress")
+ .kv("requestedBytes", blockSize)
+ .kv("offset", blockStart)
+ .kv("expectedBytes",
Math.min(blockSize, bytesAvailable))
Review Comment:
The EOFException metadata logs the block start and full block size, but the
failing read happens at `blockStart + bufferOffset` and requests `readSize`
bytes. If this triggers on a later iteration, the current
`offset`/`requestedBytes` values will be misleading when debugging short-read
failures.
##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/directentrylogger/DirectReader.java:
##########
@@ -157,7 +157,7 @@ private int readBytesIntoBuf(ByteBuf buf, long offset, int
size) throws IOExcept
.kv("offset", offset)
.kv("size", size).toString());
}
- return nativeBuffer.readByteBuf(buf, offsetInBuffer, size);
+ return nativeBuffer.readByteBuf(buf, offsetInBuffer, sizeInBuffer);
Review Comment:
The new `sizeInBuffer` limit prevents returning bytes beyond
`currentBlockEnd` when the last cached block is shorter than the reader's
native buffer (e.g., file smaller than `bufferSize`). There isn't currently a
regression test that exercises "fileSize < readerBufferSize" and attempts a
read that extends past EOF (previously could return garbage instead of throwing
EOFException). Consider adding a JUnit case for that scenario to protect this
behavior.
--
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]
