cheeeee commented on code in PR #23928:
URL: https://github.com/apache/kafka/pull/23928#discussion_r4396954665
##########
clients/src/main/java/org/apache/kafka/common/record/internal/FileRecords.java:
##########
@@ -325,30 +330,53 @@ public int writeTo(TransferableChannel destChannel, int
offset, int length) thro
long position = start + offset;
int count = Math.min(length, oldSize - offset);
try {
- long newSize = Math.min(channel.size(), end) - start;
+ long newSize = Math.min(fileSize(position), end) - start;
if (newSize < oldSize)
throw new KafkaException(String.format(
"Size of FileRecords %s has been truncated during
write: old size %d, new size %d",
file.getAbsolutePath(), oldSize, newSize));
// safe to cast to int since `count` is an int
return (int) destChannel.transferFrom(channel, position, count);
} catch (IOException e) {
- // The transfer reads the file and writes the socket in one call,
so a storage failure and a socket
- // failure both arrive here as a bare IOException. Let the handler
find out which one it was. A closed
- // channel means the file was not read, so there is nothing to
find out.
- if (transferFailureHandler != null && !(e instanceof
ClosedChannelException))
+ if (transferFailureHandler != null &&
mayBeFileReadFailure(destChannel, e))
transferFailureHandler.onTransferFailure(this, position,
count, e);
throw e;
}
}
/**
- * Read the given region of the backing file, after {@link #writeTo}
failed on it, to tell a storage failure from
- * a socket failure. The whole region is read rather than its first byte
because, depending on the platform, the
- * transfer may fail anywhere within it without reporting progress. Only a
failed read of an open channel counts
- * as a storage failure: the end of the file may be reached because the
log was truncated concurrently (e.g. when
- * the replica becomes a follower), and the channel may have been closed
concurrently, by a deletion or a
- * shutdown; neither says anything about the medium.
+ * The size of the backing file. Failing to get it counts as failing to
read the file at the given position, where
+ * the transfer was to start.
+ */
+ private long fileSize(long position) throws IOException {
+ try {
+ return channel.size();
+ } catch (IOException e) {
+ throw FileReadException.ofReadFailure(position, e);
+ }
+ }
+
+ /**
+ * Whether the given failure of {@link #writeTo} may come from a failed
read of the backing file. A
+ * {@link FileReadException} always does. A {@link ClosedChannelException}
never does: the file or the network
+ * channel was closed concurrently, by a deletion, a shutdown or a closing
connection, which says nothing about the
+ * medium. Any other exception does only if the network channel cannot
tell a failed read of the file from a failed
+ * write, as when a single system call does both.
Review Comment:
Done (13b81d084f). I changed the same wording in the PR description.
--
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]
