SteNicholas commented on code in PR #3800:
URL: https://github.com/apache/celeborn/pull/3800#discussion_r3804640560
##########
worker/src/main/java/org/apache/celeborn/service/deploy/worker/storage/LocalPartitionDataReader.java:
##########
@@ -71,12 +71,16 @@ public void readHeaderBuffer(int headerSize) throws
IOException {
public void readBufferIntoReadBuffer(ByteBuf buf, long fileSize, int length,
String filePath)
throws IOException {
Utils.checkFileIntegrity(fileSize - dataFileChanel.position(), length,
filePath);
- ByteBuffer tmpBuffer = ByteBuffer.allocate(length);
- while (tmpBuffer.hasRemaining()) {
- dataFileChanel.read(tmpBuffer);
+ long position = dataFileChanel.position();
+ int totalRead = 0;
+ while (totalRead < length) {
+ int read = buf.writeBytes(dataFileChanel, position + totalRead, length -
totalRead);
+ if (read < 0) {
Review Comment:
Fail on premature EOF:`dataFileSize` is cached when the reader is
constructed, so if the file is subsequently truncated, `writeBytes` can return
`-1` here even though the initial integrity check passed. Breaking out returns
a partially filled `ByteBuf`, while `PartitionDataReader.readBuffer` still
reports and accounts for the full requested length. Please throw
`EOFException`/`FileCorruptedException` unless exactly `length` bytes are read;
the loop should also avoid spinning indefinitely if a read returns `0`.
--
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]
