Skip to content

HDFS-17914. Prevent DFSInputStream from issuing 0-byte reads#8463

Open
lirui-apache wants to merge 1 commit intoapache:trunkfrom
lirui-apache:hdfs-17914
Open

HDFS-17914. Prevent DFSInputStream from issuing 0-byte reads#8463
lirui-apache wants to merge 1 commit intoapache:trunkfrom
lirui-apache:hdfs-17914

Conversation

@lirui-apache
Copy link
Copy Markdown

Description of PR

Avoid empty buffer read in DFSInputStream#actualGetFromOneDataNode by terminating the loop once the buffer is filled.

How was this patch tested?

Added new test

For code changes:

  • Does the title or this PR starts with the corresponding JIRA issue id (e.g. 'HADOOP-17799. Your PR title ...')?
  • Object storage: have the integration tests been executed and the endpoint declared according to the connector-specific documentation?
  • If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?
  • If applicable, have you updated the LICENSE, LICENSE-binary, NOTICE-binary files?

AI Tooling

If an AI tool was used:

while (true) {
// Stop once the slice is filled; an extra read with remaining()==0
// can trigger wasted slow-lane I/O in SCR.
while (tmp.hasRemaining()) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution first. Just one nit concern, is it only one difference that return at L1235(after PR) or return at L1236(before PR)? It will perform a zero-byte transfer in-memory and return immediately if tmp.remaining()=0, So it is OK for me before PR. welcome discussion if I miss something. Thanks again.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review. I hit the issue when I was testing short circuit read. If we call BlockReaderLocal.read with tmp.remaining() == 0, it will hit the slow lane via BlockReaderLocal::readWithBounceBuffer::fillDataBuf, which will actually allocate and read into the bounce buffer. But since user buffer is filled, we cannot drain into it and the read data is thrown away and wasted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants