You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-issues@hadoop.apache.org by "anujmodi2021 (via GitHub)" <gi...@apache.org> on 2025/07/29 13:05:01 UTC
[PR] Hadoop 19641 [hadoop]
anujmodi2021 opened a new pull request, #7835:
URL: https://github.com/apache/hadoop/pull/7835
### Description of PR
JIRA: https://issues.apache.org/jira/browse/HADOOP-19641
We have observed this across multiple workload runs that when we start reading data from input stream. The first read which came to input stream has to be read synchronously even if we trigger prefetch request for that particular offset. Most of the times we end up doing extra work of checking if the prefetch is trigerred, removing prefetch from the pending queue and go ahead to do a direct remote read in workload thread itself.
To avoid all this overhead, we will always bypass read ahead for the very first read of each input stream and trigger read aheads for second read onwards.
### How was this patch tested?
TBA
--
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: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] Hadoop 19641: [ABFS][ReadAheadV2] First Read should bypass ReadBufferManager [hadoop]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #7835: Hadoop 19641: [ABFS][ReadAheadV2] First Read should bypass ReadBufferManager
URL: https://github.com/apache/hadoop/pull/7835
--
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: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] Hadoop 19641: [ABFS][ReadAheadV2] First Read should bypass ReadBufferManager [hadoop]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #7835:
URL: https://github.com/apache/hadoop/pull/7835#discussion_r2253585143
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsInputStream.java:
##########
@@ -511,8 +512,12 @@ private int readInternal(final long position, final byte[] b, final int offset,
}
int receivedBytes;
- // queue read-aheads
- int numReadAheads = this.readAheadQueueDepth;
+ /*
+ * Number of prefetches to queue for each request is configurable.
+ * For the first read of this input stream, we don't read ahead but keep
+ * the current read data in cache as pattern might not be sequential.
+ */
+ int numReadAheads = firstRead? 1 : this.readAheadQueueDepth;
Review Comment:
[nitpick] The ternary operator lacks spaces around the `?` operator, which deviates from Java coding conventions. It should be `firstRead ? 1 : this.readAheadQueueDepth`.
```suggestion
int numReadAheads = firstRead ? 1 : this.readAheadQueueDepth;
```
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsInputStream.java:
##########
@@ -348,14 +348,15 @@ private int readOneBlock(final byte[] b, final int off, final int len) throws IO
if (alwaysReadBufferSize) {
bytesRead = readInternal(fCursor, buffer, 0, bufferSize, false);
} else {
- // Enable readAhead when reading sequentially
+ // Switch between enabling and disabling read ahead based on the workload read pattern.
if (-1 == fCursorAfterLastRead || fCursorAfterLastRead == fCursor || b.length >= bufferSize) {
- LOG.debug("Sequential read with read ahead size of {}", bufferSize);
+ // Sequential read pattern detected. Enable read ahead.
+ LOG.debug("Sequential read with read size of {} and read ahead enabled", bufferSize);
bytesRead = readInternal(fCursor, buffer, 0, bufferSize, false);
} else {
- // Enabling read ahead for random reads as well to reduce number of remote calls.
+ // Random read pattern detected. Disable read ahead.
int lengthWithReadAhead = Math.min(b.length + readAheadRange, bufferSize);
- LOG.debug("Random read with read ahead size of {}", lengthWithReadAhead);
+ LOG.debug("Random read with read size of {} and read ahead disabled", lengthWithReadAhead);
Review Comment:
The log message mentions 'read ahead disabled' but the variable logged is `lengthWithReadAhead`, which suggests read-ahead is still being used. This is confusing and inconsistent with the message.
```suggestion
LOG.debug("Random read with requested size {} (read ahead range: {}), actual read size: {}, read ahead disabled", b.length, readAheadRange, lengthWithReadAhead);
```
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsInputStream.java:
##########
@@ -348,14 +348,15 @@ private int readOneBlock(final byte[] b, final int off, final int len) throws IO
if (alwaysReadBufferSize) {
bytesRead = readInternal(fCursor, buffer, 0, bufferSize, false);
} else {
- // Enable readAhead when reading sequentially
+ // Switch between enabling and disabling read ahead based on the workload read pattern.
if (-1 == fCursorAfterLastRead || fCursorAfterLastRead == fCursor || b.length >= bufferSize) {
- LOG.debug("Sequential read with read ahead size of {}", bufferSize);
+ // Sequential read pattern detected. Enable read ahead.
+ LOG.debug("Sequential read with read size of {} and read ahead enabled", bufferSize);
Review Comment:
The log message mentions 'read size of {}' but logs `bufferSize` instead of the actual read size (`b.length`). This could be misleading when debugging read operations.
```suggestion
LOG.debug("Sequential read with read size of {} and read ahead enabled", b.length);
```
--
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: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] Hadoop 19641: [ABFS][ReadAheadV2] First Read should bypass ReadBufferManager [hadoop]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #7835:
URL: https://github.com/apache/hadoop/pull/7835#issuecomment-3537170768
We're closing this stale PR because it has been open for 100 days with no activity. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
If you feel like this was a mistake, or you would like to continue working on it, please feel free to re-open it and ask for a committer to remove the stale tag and review again.
Thanks all for your contribution.
--
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: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] Hadoop 19641: [ABFS][ReadAheadV2] First Read should bypass ReadBufferManager [hadoop]
Posted by "hadoop-yetus (via GitHub)" <gi...@apache.org>.
hadoop-yetus commented on PR #7835:
URL: https://github.com/apache/hadoop/pull/7835#issuecomment-3132986833
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 40s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 1 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 40m 36s | | trunk passed |
| +1 :green_heart: | compile | 0m 43s | | trunk passed with JDK Ubuntu-11.0.27+6-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | compile | 0m 39s | | trunk passed with JDK Private Build-1.8.0_452-8u452-ga~us1-0ubuntu1~20.04-b09 |
| +1 :green_heart: | checkstyle | 0m 33s | | trunk passed |
| +1 :green_heart: | mvnsite | 0m 43s | | trunk passed |
| +1 :green_heart: | javadoc | 0m 41s | | trunk passed with JDK Ubuntu-11.0.27+6-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javadoc | 0m 36s | | trunk passed with JDK Private Build-1.8.0_452-8u452-ga~us1-0ubuntu1~20.04-b09 |
| +1 :green_heart: | spotbugs | 1m 9s | | trunk passed |
| +1 :green_heart: | shadedclient | 35m 18s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 0m 32s | | the patch passed |
| +1 :green_heart: | compile | 0m 31s | | the patch passed with JDK Ubuntu-11.0.27+6-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javac | 0m 31s | | the patch passed |
| +1 :green_heart: | compile | 0m 30s | | the patch passed with JDK Private Build-1.8.0_452-8u452-ga~us1-0ubuntu1~20.04-b09 |
| +1 :green_heart: | javac | 0m 30s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 0m 21s | | the patch passed |
| +1 :green_heart: | mvnsite | 0m 33s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 29s | | the patch passed with JDK Ubuntu-11.0.27+6-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javadoc | 0m 27s | | the patch passed with JDK Private Build-1.8.0_452-8u452-ga~us1-0ubuntu1~20.04-b09 |
| +1 :green_heart: | spotbugs | 1m 10s | | the patch passed |
| +1 :green_heart: | shadedclient | 35m 8s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 3m 4s | | hadoop-azure in the patch passed. |
| +1 :green_heart: | asflicense | 0m 37s | | The patch does not generate ASF License warnings. |
| | | 126m 33s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.51 ServerAPI=1.51 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-7835/1/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/7835 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
| uname | Linux 6eb9eaef582a 5.15.0-144-generic #157-Ubuntu SMP Mon Jun 16 07:33:10 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / d2b15027a91c8a6f6dc4147499dd640d18765cbd |
| Default Java | Private Build-1.8.0_452-8u452-ga~us1-0ubuntu1~20.04-b09 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.27+6-post-Ubuntu-0ubuntu120.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_452-8u452-ga~us1-0ubuntu1~20.04-b09 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-7835/1/testReport/ |
| Max. process+thread count | 554 (vs. ulimit of 5500) |
| modules | C: hadoop-tools/hadoop-azure U: hadoop-tools/hadoop-azure |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-7835/1/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0 https://yetus.apache.org |
This message was automatically generated.
--
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: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] Hadoop 19641: [ABFS][ReadAheadV2] First Read should bypass ReadBufferManager [hadoop]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #7835:
URL: https://github.com/apache/hadoop/pull/7835#discussion_r2244433207
##########
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/TestAbfsInputStream.java:
##########
@@ -477,11 +494,14 @@ public void testSuccessfulReadAhead() throws Exception {
AbfsInputStream inputStream = getAbfsInputStream(client, "testSuccessfulReadAhead.txt");
int beforeReadCompletedListSize = getBufferManager().getCompletedReadListSize();
- // First read request that triggers readAheads.
+ // First read request always bypasses readahead. This will succeed
inputStream.read(new byte[ONE_KB]);
- // Only the 3 readAhead threads should have triggered client.read
- verifyReadCallCount(client, 3);
+ // This will start triggering readahead requests.
+ inputStream.read((new byte[1024]));
Review Comment:
Inconsistent array size usage. The code uses `new byte[1024]` here while using `new byte[ONE_KB]` elsewhere in the same file. Using the constant `ONE_KB` would be more consistent and maintainable.
```suggestion
inputStream.read((new byte[ONE_KB]));
```
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsInputStream.java:
##########
@@ -348,14 +348,20 @@ private int readOneBlock(final byte[] b, final int off, final int len) throws IO
if (alwaysReadBufferSize) {
bytesRead = readInternal(fCursor, buffer, 0, bufferSize, false);
} else {
- // Enable readAhead when reading sequentially
- if (-1 == fCursorAfterLastRead || fCursorAfterLastRead == fCursor || b.length >= bufferSize) {
- LOG.debug("Sequential read with read ahead size of {}", bufferSize);
+ // Switch between enabling and disabling read ahead based on the workload read pattern.
+ // First Read on input stream should always bypass read ahead.
+ if (firstRead) {
Review Comment:
The `firstRead` variable is referenced but not defined or updated in this diff. The code checks this variable but there's no logic shown to set it to false after the first read, which could cause all reads to bypass read-ahead indefinitely.
##########
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/TestAbfsInputStream.java:
##########
@@ -119,14 +119,14 @@ private AbfsInputStream getAbfsInputStream(AbfsClient mockAbfsClient,
String fileName) throws IOException {
AbfsInputStreamContext inputStreamContext = new AbfsInputStreamContext(-1);
// Create AbfsInputStream with the client instance
- AbfsInputStream inputStream = new AbfsInputStream(
+ AbfsInputStream inputStream = Mockito.spy(new AbfsInputStream(
mockAbfsClient,
null,
FORWARD_SLASH + fileName,
- THREE_KB,
+ THREE_KB + ONE_KB, // First read will always bypass readahead
Review Comment:
The comment suggests this change is related to first read bypassing readahead, but increasing the file size by ONE_KB doesn't clearly relate to the first read behavior. This change appears to be adjusting test data size rather than directly testing the first read bypass feature.
```suggestion
ONE_KB, // Set file size to match the read buffer size to ensure the first read bypasses readahead
```
--
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: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] Hadoop 19641: [ABFS][ReadAheadV2] First Read should bypass ReadBufferManager [hadoop]
Posted by "hadoop-yetus (via GitHub)" <gi...@apache.org>.
hadoop-yetus commented on PR #7835:
URL: https://github.com/apache/hadoop/pull/7835#issuecomment-3139479470
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 33s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 1s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 1s | | detect-secrets was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 1 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 38m 36s | | trunk passed |
| +1 :green_heart: | compile | 0m 42s | | trunk passed with JDK Ubuntu-11.0.27+6-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | compile | 0m 38s | | trunk passed with JDK Private Build-1.8.0_452-8u452-ga~us1-0ubuntu1~20.04-b09 |
| +1 :green_heart: | checkstyle | 0m 35s | | trunk passed |
| +1 :green_heart: | mvnsite | 0m 44s | | trunk passed |
| +1 :green_heart: | javadoc | 0m 43s | | trunk passed with JDK Ubuntu-11.0.27+6-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javadoc | 0m 36s | | trunk passed with JDK Private Build-1.8.0_452-8u452-ga~us1-0ubuntu1~20.04-b09 |
| +1 :green_heart: | spotbugs | 1m 9s | | trunk passed |
| +1 :green_heart: | shadedclient | 35m 16s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 0m 31s | | the patch passed |
| +1 :green_heart: | compile | 0m 32s | | the patch passed with JDK Ubuntu-11.0.27+6-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javac | 0m 32s | | the patch passed |
| +1 :green_heart: | compile | 0m 30s | | the patch passed with JDK Private Build-1.8.0_452-8u452-ga~us1-0ubuntu1~20.04-b09 |
| +1 :green_heart: | javac | 0m 30s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 0m 22s | | the patch passed |
| +1 :green_heart: | mvnsite | 0m 33s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 29s | | the patch passed with JDK Ubuntu-11.0.27+6-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javadoc | 0m 27s | | the patch passed with JDK Private Build-1.8.0_452-8u452-ga~us1-0ubuntu1~20.04-b09 |
| +1 :green_heart: | spotbugs | 1m 8s | | the patch passed |
| +1 :green_heart: | shadedclient | 35m 5s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 3m 5s | | hadoop-azure in the patch passed. |
| +1 :green_heart: | asflicense | 0m 37s | | The patch does not generate ASF License warnings. |
| | | 124m 8s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.51 ServerAPI=1.51 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-7835/2/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/7835 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
| uname | Linux 1157b8de8554 5.15.0-144-generic #157-Ubuntu SMP Mon Jun 16 07:33:10 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / 645405660c58528fcc339cb92227d18defd185aa |
| Default Java | Private Build-1.8.0_452-8u452-ga~us1-0ubuntu1~20.04-b09 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.27+6-post-Ubuntu-0ubuntu120.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_452-8u452-ga~us1-0ubuntu1~20.04-b09 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-7835/2/testReport/ |
| Max. process+thread count | 703 (vs. ulimit of 5500) |
| modules | C: hadoop-tools/hadoop-azure U: hadoop-tools/hadoop-azure |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-7835/2/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0 https://yetus.apache.org |
This message was automatically generated.
--
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: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org