HDDS-10821. Ensure ozone write all the buffer content to FileChannel. #6652
Conversation
| * Write all the contents of the buffer from the current position to the limit | ||
| * to {@code channel}. | ||
| */ | ||
| void writeFully(GatheringByteChannel channel) throws IOException; |
There was a problem hiding this comment.
I would like to suggest fixing writeTo instead of adding this new method to the interface.
-
In what case may we want to use the existing method? With this fix,
writeTobecomes unused in production. -
TestChunkBuffer.assertWritetestswriteToand expects it to write all data. However, it only works because the channels it uses, unlikeFileChannel, write fully. This can be verified by randomly adjustinglimitfor the buffer inMockGatheringChannel.write(ByteBuffer), i.e. simulating incomplete writes. On the other hand,writeFullyis not covered by unit tests. -
We can avoid the changes in
ChunkUtils, most of which is due towriteFullynot returning the number of bytes written.
There was a problem hiding this comment.
@duongkame you can pick adoroszlai@daee756, which implements the suggested change
| throws IOException | ||
| { |
There was a problem hiding this comment.
hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/common/utils/BufferUtils.java
149: '{' at column 3 should be on the previous line.
| if (n <= 0) | ||
| throw new IllegalStateException("no bytes written"); |
There was a problem hiding this comment.
hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/common/utils/BufferUtils.java
152: 'if' construct must use '{}'s.
What changes were proposed in this pull request?
Ozone should expect
FileChannel.writedoesn't fully write the entire buffer and not crash the pipeline one that happens.See HDDS-10821.
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-10821
How was this patch tested?
CI.