[GitHub] [kafka] junrao commented on a change in pull request #9970: Kafka-12233: `FileRecords.writeTo` set length incorrectly

[GitBox <gi...@apache.org>]

junrao commented on a change in pull request #9970:
URL: https://github.com/apache/kafka/pull/9970#discussion_r564747239



##########
File path: clients/src/main/java/org/apache/kafka/common/record/FileRecords.java
##########
@@ -278,7 +278,7 @@ public long writeTo(TransferableChannel destChannel, long offset, int length) th
                     file.getAbsolutePath(), oldSize, newSize));
 
         long position = start + offset;
-        int count = Math.min(length, oldSize);
+        long count = Math.min(length, oldSize - offset);

Review comment:
       This code is called from SocketServer through RecordsSend. The expectation is that the underlying file channel has enough bytes to return for the requested length. Normally this should be the case. If for some reason this is not true, by bounding the transfer amount by the remaining bytes in the file channel will cause RecordsSend.writeTo() to be called in a loop until the request times out. This may hide a real problem.
   
   Perhaps what we could do is to throw an IllegalStateException if the file channel doesn't have enough bytes left. This will cause the socket channel to be closed and the Selector to log a warning.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org