HDDS-10511. Support ByteBufferPositionedReadable in OzoneFSInputStream. #6372
Conversation
jojochuang
left a comment
jojochuang
left a comment
There was a problem hiding this comment.
Looks functionally correct. Need a closer look to make sure it doesn't send extra readChunk requests and so on which degrades performance.
We should also update stream capability: add
case StreamCapabilities.PREADBYTEBUFFER:
| * @see #read(long, ByteBuffer) | ||
| */ | ||
| void readFully(long position, ByteBuffer buf) throws IOException; | ||
| } |
There was a problem hiding this comment.
We will use the ByteBufferPositionedReadable interface defined in hadoop-common directly. This file can be removed. Is there dependency problem for this?
There was a problem hiding this comment.
The problem is that some of the acceptance tests run with Hadoop 3.2 runtime which does not have this API.
There was a problem hiding this comment.
This interface is added in Hadoop 3.3, so the older hadoop version client fails to load ozone fs jar. This file can be removed when we drop support for older hadoop version.
There was a problem hiding this comment.
I see. Oh, there are comments explaining this.
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/OzoneFSInputStream.java
Outdated
Show resolved
Hide resolved
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/OzoneFSInputStream.java
Outdated
Show resolved
Hide resolved
|
|
||
| // Buffer size more than actual data, still read should succeed | ||
| ByteBuffer buffer1 = ByteBuffer.allocate(30 * 1024 * 1024 * 2); | ||
| readBytes = inputStream.read(12, buffer1); |
There was a problem hiding this comment.
Can you add readBytes check, to make sure the bytes read value is as expected?
There was a problem hiding this comment.
Also can you add
- file position check, make sure file position is not changed after read or readFully.
- boundary check, such as illegal position value.
ChenSammi
left a comment
ChenSammi
left a comment
There was a problem hiding this comment.
The last patch looks good to me. Thanks @ashishkumar50 .
|
Thanks @ashishkumar50 @ChenSammi |
|
Confirmed it's being used by Apache HBase 2.5.0 and above. |
…m. (apache#6372) Co-authored-by: ashishk <[email protected]>
…m. (apache#6372) Co-authored-by: ashishk <[email protected]>
4 participants
What changes were proposed in this pull request?
Support ByteBufferPositionedReadable in OzoneFSInputStream.
This will benefit HBase read.
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-10511
How was this patch tested?
New integration test