[jira] [Commented] (HADOOP-18521) ABFS ReadBufferManager buffer sharing across concurrent HTTP requests
[
https://issues.apache.org/jira/browse/HADOOP-18521?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17645451#comment-17645451
]
ASF GitHub Bot commented on HADOOP-18521:
-
hadoop-yetus commented on PR #5117:
URL: https://github.com/apache/hadoop/pull/5117#issuecomment-1344656088
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|::|--:|:|::|:---:|
| +0 :ok: | reexec | 0m 37s | | 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 6 new or modified test files. |
_ trunk Compile Tests _ |
| +0 :ok: | mvndep | 17m 19s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 31m 23s | | trunk passed |
| +1 :green_heart: | compile | 24m 34s | | trunk passed with JDK
Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04 |
| +1 :green_heart: | compile | 21m 1s | | trunk passed with JDK
Private Build-1.8.0_352-8u352-ga-1~20.04-b08 |
| +1 :green_heart: | checkstyle | 3m 40s | | trunk passed |
| +1 :green_heart: | mvnsite | 2m 35s | | trunk passed |
| -1 :x: | javadoc | 1m 12s |
[/branch-javadoc-hadoop-common-project_hadoop-common-jdkUbuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5117/9/artifact/out/branch-javadoc-hadoop-common-project_hadoop-common-jdkUbuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04.txt)
| hadoop-common in trunk failed with JDK
Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04. |
| -1 :x: | javadoc | 0m 44s |
[/branch-javadoc-hadoop-tools_hadoop-azure-jdkUbuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5117/9/artifact/out/branch-javadoc-hadoop-tools_hadoop-azure-jdkUbuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04.txt)
| hadoop-azure in trunk failed with JDK
Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04. |
| +1 :green_heart: | javadoc | 1m 27s | | trunk passed with JDK
Private Build-1.8.0_352-8u352-ga-1~20.04-b08 |
| +1 :green_heart: | spotbugs | 4m 2s | | trunk passed |
| +1 :green_heart: | shadedclient | 20m 48s | | branch has no errors
when building and testing our client artifacts. |
_ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 28s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 1m 30s | | the patch passed |
| +1 :green_heart: | compile | 25m 54s | | the patch passed with JDK
Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04 |
| +1 :green_heart: | javac | 25m 54s | | the patch passed |
| +1 :green_heart: | compile | 21m 47s | | the patch passed with JDK
Private Build-1.8.0_352-8u352-ga-1~20.04-b08 |
| +1 :green_heart: | javac | 21m 47s | | the patch passed |
| -1 :x: | blanks | 0m 0s |
[/blanks-eol.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5117/9/artifact/out/blanks-eol.txt)
| The patch has 1 line(s) that end in blanks. Use git apply --whitespace=fix
<>. Refer https://git-scm.com/docs/git-apply |
| -0 :warning: | checkstyle | 3m 57s |
[/results-checkstyle-root.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5117/9/artifact/out/results-checkstyle-root.txt)
| root: The patch generated 5 new + 1 unchanged - 0 fixed = 6 total (was 1) |
| +1 :green_heart: | mvnsite | 2m 32s | | the patch passed |
| -1 :x: | javadoc | 1m 4s |
[/patch-javadoc-hadoop-common-project_hadoop-common-jdkUbuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5117/9/artifact/out/patch-javadoc-hadoop-common-project_hadoop-common-jdkUbuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04.txt)
| hadoop-common in the patch failed with JDK
Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04. |
| -1 :x: | javadoc | 0m 41s |
[/patch-javadoc-hadoop-tools_hadoop-azure-jdkUbuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5117/9/artifact/out/patch-javadoc-hadoop-tools_hadoop-azure-jdkUbuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04.txt)
| hadoop-azure in the patch failed with JDK
Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04. |
| +1 :green_heart: | javadoc | 1m 22s | | the patch passed with JDK
Private Build-1.8.0_352-8u352-ga-1~20.04-b08 |
| +1 :green_heart: | spotbugs | 4m 8s | | the patch passed |
| +1 :green
[jira] [Commented] (HADOOP-18521) ABFS ReadBufferManager buffer sharing across concurrent HTTP requests
[
https://issues.apache.org/jira/browse/HADOOP-18521?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17645309#comment-17645309
]
Steve Loughran commented on HADOOP-18521:
-
We have a nice and minimal fix which can be easily backported anywhere that is
needed.
That larger patch of mine was intended to
# avoid adding the prefetches of closed streams to the completed list (impact:
buffers will be retained until timeout)
# add validation about accidental buffer reuse
# handle failures in the async read caused by the close()
# iostatistics for production code and testing. used in asserts and will allow
us to assess value of prefetching in production code
# pulling out of the methods invoked on abfs input stream into their own
interface, again for testing.
we don't need this as much any more; it's something where I would like most of
the features (1, 3, 4, 5) in, but we could look at that in terms of a broader
review of the readbuffer feature
I'm going to
rebase my pr
create a new jira for "extend readbuffer for testing/statistics" and move the
pr to that
> ABFS ReadBufferManager buffer sharing across concurrent HTTP requests
> -
>
> Key: HADOOP-18521
> URL: https://issues.apache.org/jira/browse/HADOOP-18521
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs/azure
>Affects Versions: 3.3.2, 3.3.3, 3.3.4
>Reporter: Steve Loughran
>Assignee: Steve Loughran
>Priority: Critical
> Labels: pull-request-available
>
> AbfsInputStream.close() can trigger the return of buffers used for active
> prefetch GET requests into the ReadBufferManager free buffer pool.
> A subsequent prefetch by a different stream in the same process may acquire
> this same buffer. This can lead to risk of corruption of its own prefetched
> data, data which may then be returned to that other thread.
> On releases without the fix for this (3.3.2+), the bug can be avoided by
> disabling all prefetching
> {code}
> fs.azure.readaheadqueue.depth = 0
> {code}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[jira] [Commented] (HADOOP-18521) ABFS ReadBufferManager buffer sharing across concurrent HTTP requests
[
https://issues.apache.org/jira/browse/HADOOP-18521?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17640956#comment-17640956
]
ASF GitHub Bot commented on HADOOP-18521:
-
hadoop-yetus commented on PR #5117:
URL: https://github.com/apache/hadoop/pull/5117#issuecomment-1331371867
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|::|--:|:|::|:---:|
| +0 :ok: | reexec | 0m 51s | | 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 6 new or modified test files. |
_ trunk Compile Tests _ |
| +0 :ok: | mvndep | 16m 5s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 26m 16s | | trunk passed |
| +1 :green_heart: | compile | 23m 32s | | trunk passed with JDK
Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | compile | 20m 53s | | trunk passed with JDK
Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | checkstyle | 4m 12s | | trunk passed |
| +1 :green_heart: | mvnsite | 3m 13s | | trunk passed |
| +1 :green_heart: | javadoc | 2m 44s | | trunk passed with JDK
Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javadoc | 2m 24s | | trunk passed with JDK
Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 4m 42s | | trunk passed |
| +1 :green_heart: | shadedclient | 21m 46s | | branch has no errors
when building and testing our client artifacts. |
_ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 31s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 1m 41s | | the patch passed |
| +1 :green_heart: | compile | 22m 41s | | the patch passed with JDK
Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javac | 22m 41s | | the patch passed |
| +1 :green_heart: | compile | 20m 46s | | the patch passed with JDK
Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | javac | 20m 46s | | the patch passed |
| -1 :x: | blanks | 0m 0s |
[/blanks-eol.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5117/8/artifact/out/blanks-eol.txt)
| The patch has 1 line(s) that end in blanks. Use git apply --whitespace=fix
<>. Refer https://git-scm.com/docs/git-apply |
| -0 :warning: | checkstyle | 4m 11s |
[/results-checkstyle-root.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5117/8/artifact/out/results-checkstyle-root.txt)
| root: The patch generated 5 new + 1 unchanged - 0 fixed = 6 total (was 1) |
| +1 :green_heart: | mvnsite | 3m 5s | | the patch passed |
| +1 :green_heart: | javadoc | 2m 41s | | the patch passed with JDK
Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javadoc | 2m 8s | | the patch passed with JDK
Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 4m 59s | | the patch passed |
| +1 :green_heart: | shadedclient | 22m 29s | | patch has no errors
when building and testing our client artifacts. |
_ Other Tests _ |
| +1 :green_heart: | unit | 18m 40s | | hadoop-common in the patch
passed. |
| +1 :green_heart: | unit | 2m 41s | | hadoop-azure in the patch
passed. |
| +1 :green_heart: | asflicense | 1m 18s | | The patch does not
generate ASF License warnings. |
| | | 239m 51s | | |
| Subsystem | Report/Notes |
|--:|:-|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base:
https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5117/8/artifact/out/Dockerfile
|
| GITHUB PR | https://github.com/apache/hadoop/pull/5117 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall
mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
| uname | Linux 7ef67244d4ab 4.15.0-191-generic #202-Ubuntu SMP Thu Aug 4
01:49:29 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / a261c9ac1cab9b2e1f95ef9ff59e1b02c07c5c4d |
| Default Java | Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| Multi-JDK versions |
/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04
/usr/lib/jvm/jav
[jira] [Commented] (HADOOP-18521) ABFS ReadBufferManager buffer sharing across concurrent HTTP requests
[
https://issues.apache.org/jira/browse/HADOOP-18521?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17640195#comment-17640195
]
ASF GitHub Bot commented on HADOOP-18521:
-
hadoop-yetus commented on PR #5117:
URL: https://github.com/apache/hadoop/pull/5117#issuecomment-1329588076
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|::|--:|:|::|:---:|
| +0 :ok: | reexec | 0m 55s | | Docker mode activated. |
_ Prechecks _ |
| +1 :green_heart: | dupname | 0m 1s | | 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 6 new or modified test files. |
_ trunk Compile Tests _ |
| +0 :ok: | mvndep | 16m 10s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 29m 8s | | trunk passed |
| +1 :green_heart: | compile | 25m 32s | | trunk passed with JDK
Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | compile | 22m 17s | | trunk passed with JDK
Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | checkstyle | 4m 25s | | trunk passed |
| +1 :green_heart: | mvnsite | 2m 53s | | trunk passed |
| +1 :green_heart: | javadoc | 2m 17s | | trunk passed with JDK
Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javadoc | 1m 49s | | trunk passed with JDK
Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 4m 22s | | trunk passed |
| +1 :green_heart: | shadedclient | 25m 4s | | branch has no errors
when building and testing our client artifacts. |
_ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 29s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 1m 49s | | the patch passed |
| +1 :green_heart: | compile | 27m 24s | | the patch passed with JDK
Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javac | 27m 24s | | the patch passed |
| +1 :green_heart: | compile | 26m 57s | | the patch passed with JDK
Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | javac | 26m 57s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks
issues. |
| -0 :warning: | checkstyle | 5m 1s |
[/results-checkstyle-root.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5117/7/artifact/out/results-checkstyle-root.txt)
| root: The patch generated 40 new + 1 unchanged - 0 fixed = 41 total (was 1)
|
| +1 :green_heart: | mvnsite | 3m 33s | | the patch passed |
| +1 :green_heart: | javadoc | 2m 49s | | the patch passed with JDK
Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javadoc | 2m 4s | | the patch passed with JDK
Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 5m 7s | | the patch passed |
| +1 :green_heart: | shadedclient | 25m 44s | | patch has no errors
when building and testing our client artifacts. |
_ Other Tests _ |
| +1 :green_heart: | unit | 18m 44s | | hadoop-common in the patch
passed. |
| +1 :green_heart: | unit | 2m 40s | | hadoop-azure in the patch
passed. |
| +1 :green_heart: | asflicense | 1m 19s | | The patch does not
generate ASF License warnings. |
| | | 263m 49s | | |
| Subsystem | Report/Notes |
|--:|:-|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base:
https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5117/7/artifact/out/Dockerfile
|
| GITHUB PR | https://github.com/apache/hadoop/pull/5117 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall
mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
| uname | Linux 9fca8013fa2f 4.15.0-191-generic #202-Ubuntu SMP Thu Aug 4
01:49:29 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / b224f33813755c2e5b2d73d64486ad4b0482feb6 |
| Default Java | Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| Multi-JDK versions |
/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04
/usr/lib/jvm/java-8-openjdk-amd64:Private
Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| Test Results |
https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5117/7/testReport/ |
| Max. pr
[jira] [Commented] (HADOOP-18521) ABFS ReadBufferManager buffer sharing across concurrent HTTP requests
[
https://issues.apache.org/jira/browse/HADOOP-18521?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17640155#comment-17640155
]
ASF GitHub Bot commented on HADOOP-18521:
-
steveloughran commented on code in PR #5117:
URL: https://github.com/apache/hadoop/pull/5117#discussion_r1033835902
##
hadoop-tools/hadoop-azure/src/test/resources/log4j.properties:
##
@@ -26,6 +26,8 @@
log4j.logger.org.apache.hadoop.fs.azure.AzureFileSystemThreadPoolExecutor=DEBUG
log4j.logger.org.apache.hadoop.fs.azure.BlockBlobAppendStream=DEBUG
log4j.logger.org.apache.hadoop.fs.azurebfs.contracts.services.TracingService=TRACE
log4j.logger.org.apache.hadoop.fs.azurebfs.services.AbfsClient=DEBUG
+log4j.logger.org.apache.hadoop.fs.azurebfs.services.ReadBufferManager=TRACE
Review Comment:
it was...but we run the rest of the tests logging at debug, and i don't see
the prefetcher being any chattier than the rest of the stack.
> ABFS ReadBufferManager buffer sharing across concurrent HTTP requests
> -
>
> Key: HADOOP-18521
> URL: https://issues.apache.org/jira/browse/HADOOP-18521
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs/azure
>Affects Versions: 3.3.2, 3.3.3, 3.3.4
>Reporter: Steve Loughran
>Assignee: Steve Loughran
>Priority: Critical
> Labels: pull-request-available
>
> AbfsInputStream.close() can trigger the return of buffers used for active
> prefetch GET requests into the ReadBufferManager free buffer pool.
> A subsequent prefetch by a different stream in the same process may acquire
> this same buffer. This can lead to risk of corruption of its own prefetched
> data, data which may then be returned to that other thread.
> On releases without the fix for this (3.3.2+), the bug can be avoided by
> disabling all prefetching
> {code}
> fs.azure.readaheadqueue.depth = 0
> {code}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[jira] [Commented] (HADOOP-18521) ABFS ReadBufferManager buffer sharing across concurrent HTTP requests
[
https://issues.apache.org/jira/browse/HADOOP-18521?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17640153#comment-17640153
]
ASF GitHub Bot commented on HADOOP-18521:
-
steveloughran commented on code in PR #5117:
URL: https://github.com/apache/hadoop/pull/5117#discussion_r1033833635
##
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsInputStream.java:
##
@@ -555,7 +579,7 @@ int readRemote(long position, byte[] b, int offset, int
length, TracingContext t
throw new FileNotFoundException(ere.getMessage());
}
}
- throw new IOException(ex);
+ throw ex;
Review Comment:
i haven't
```
class AzureBlobFileSystemException extends IOException
```
just removed one layer of wrapping so hive is less likely to lose the stack
trace
> ABFS ReadBufferManager buffer sharing across concurrent HTTP requests
> -
>
> Key: HADOOP-18521
> URL: https://issues.apache.org/jira/browse/HADOOP-18521
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs/azure
>Affects Versions: 3.3.2, 3.3.3, 3.3.4
>Reporter: Steve Loughran
>Assignee: Steve Loughran
>Priority: Critical
> Labels: pull-request-available
>
> AbfsInputStream.close() can trigger the return of buffers used for active
> prefetch GET requests into the ReadBufferManager free buffer pool.
> A subsequent prefetch by a different stream in the same process may acquire
> this same buffer. This can lead to risk of corruption of its own prefetched
> data, data which may then be returned to that other thread.
> On releases without the fix for this (3.3.2+), the bug can be avoided by
> disabling all prefetching
> {code}
> fs.azure.readaheadqueue.depth = 0
> {code}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[jira] [Commented] (HADOOP-18521) ABFS ReadBufferManager buffer sharing across concurrent HTTP requests
[
https://issues.apache.org/jira/browse/HADOOP-18521?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17639973#comment-17639973
]
ASF GitHub Bot commented on HADOOP-18521:
-
anmolanmol1234 commented on code in PR #5117:
URL: https://github.com/apache/hadoop/pull/5117#discussion_r1033433849
##
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/ReadBufferManager.java:
##
@@ -302,33 +325,33 @@ private synchronized boolean tryEvict() {
}
private boolean evict(final ReadBuffer buf) {
-// As failed ReadBuffers (bufferIndx = -1) are saved in completedReadList,
-// avoid adding it to freeList.
-if (buf.getBufferindex() != -1) {
- freeList.push(buf.getBufferindex());
-}
-
-completedReadList.remove(buf);
buf.setTracingContext(null);
if (LOGGER.isTraceEnabled()) {
LOGGER.trace("Evicting buffer idx {}; was used for file {} offset {}
length {}",
buf.getBufferindex(), buf.getStream().getPath(), buf.getOffset(),
buf.getLength());
}
+completedReadList.remove(buf);
Review Comment:
Can you please highlight again why should we not remove the buffer from the
completed list after it has been added to the free list ?
> ABFS ReadBufferManager buffer sharing across concurrent HTTP requests
> -
>
> Key: HADOOP-18521
> URL: https://issues.apache.org/jira/browse/HADOOP-18521
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs/azure
>Affects Versions: 3.3.2, 3.3.3, 3.3.4
>Reporter: Steve Loughran
>Assignee: Steve Loughran
>Priority: Critical
> Labels: pull-request-available
>
> AbfsInputStream.close() can trigger the return of buffers used for active
> prefetch GET requests into the ReadBufferManager free buffer pool.
> A subsequent prefetch by a different stream in the same process may acquire
> this same buffer. This can lead to risk of corruption of its own prefetched
> data, data which may then be returned to that other thread.
> On releases without the fix for this (3.3.2+), the bug can be avoided by
> disabling all prefetching
> {code}
> fs.azure.readaheadqueue.depth = 0
> {code}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[jira] [Commented] (HADOOP-18521) ABFS ReadBufferManager buffer sharing across concurrent HTTP requests
[
https://issues.apache.org/jira/browse/HADOOP-18521?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17639972#comment-17639972
]
ASF GitHub Bot commented on HADOOP-18521:
-
anmolanmol1234 commented on code in PR #5117:
URL: https://github.com/apache/hadoop/pull/5117#discussion_r1033420242
##
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsInputStream.java:
##
@@ -555,7 +579,7 @@ int readRemote(long position, byte[] b, int offset, int
length, TracingContext t
throw new FileNotFoundException(ere.getMessage());
}
}
- throw new IOException(ex);
+ throw ex;
Review Comment:
Any specific reason for changing the exception type from IOException to
AzureBlobFileSystemException ?
##
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/TestAbfsInputStream.java:
##
@@ -851,4 +880,67 @@ private void resetReadBufferManager(int bufferSize, int
threshold) {
// by successive tests can lead to OOM based on the dev VM/machine
capacity.
System.gc();
}
+
+ /**
+ * The first readahead closes the stream.
+ */
+ @Test
+ public void testStreamCloseInFirstReadAhead() throws Exception {
+describe("close a stream during prefetch, verify outcome is good");
+
+AbfsClient client = getMockAbfsClient();
+AbfsRestOperation successOp = getMockRestOp();
+
+AbfsInputStream inputStream = getAbfsInputStream(client, getMethodName());
+ReadBufferManager bufferManager = ReadBufferManager.getBufferManager();
+
+final long initialInProgressBlocksDiscarded =
bufferManager.getInProgressBlocksDiscarded();
+
+// on first read, the op succeeds but the stream is closed, which
+// means that the request should be considered a failure
+doAnswer(invocation -> {
+ LOG.info("in read call with {}", inputStream);
+ inputStream.close();
+ return successOp;
+}).doReturn(successOp)
+.when(client)
+.read(any(String.class), any(Long.class), any(byte[].class),
+any(Integer.class), any(Integer.class), any(String.class),
+any(String.class), any(TracingContext.class));
+
+// kick these off before the read() to avoid various race conditions.
+queueReadAheads(inputStream);
+
+// AbfsInputStream Read would have waited for the read-ahead for the
requested offset
+// as we are testing from ReadAheadManager directly, sleep for a sec to
+// get the read ahead threads to complete
+waitForPrefetchCompletion();
+
+// this triggers prefetching, which closes the stream while the read
+// is queued. which causes the prefetch to not return.
+// which triggers a blocking read, which will then fail.
+intercept(PathIOException.class, FSExceptionMessages.STREAM_IS_CLOSED, ()
-> {
+ // should fail
+ int bytesRead = inputStream.read(new byte[ONE_KB]);
+ // diagnostics info if failure wasn't raised
+ return "read " + bytesRead + " bytes from " + inputStream;
+});
+
+Assertions.assertThat(bufferManager.getCompletedReadListCopy())
+.filteredOn(rb -> rb.getStream() == inputStream)
+.describedAs("list of completed reads")
+.isEmpty();
+IOStatisticsStore ios = inputStream.getIOStatistics();
+assertThatStatisticCounter(ios, STREAM_READ_PREFETCH_BLOCKS_DISCARDED)
+.describedAs("blocks discarded by %s", inputStream)
+.isGreaterThan(0);
+
+// at least one of the blocks was discarded in progress.
+// this is guaranteed because the mockito callback must have been invoked
+// by the prefetcher
+Assertions.assertThat(bufferManager.getInProgressBlocksDiscarded())
+.describedAs("in progress blocks discarded")
+.isGreaterThan(initialInProgressBlocksDiscarded);
+ }
+
}
Review Comment:
nit: Add line at the end of the file.
##
hadoop-tools/hadoop-azure/src/test/resources/log4j.properties:
##
@@ -26,6 +26,8 @@
log4j.logger.org.apache.hadoop.fs.azure.AzureFileSystemThreadPoolExecutor=DEBUG
log4j.logger.org.apache.hadoop.fs.azure.BlockBlobAppendStream=DEBUG
log4j.logger.org.apache.hadoop.fs.azurebfs.contracts.services.TracingService=TRACE
log4j.logger.org.apache.hadoop.fs.azurebfs.services.AbfsClient=DEBUG
+log4j.logger.org.apache.hadoop.fs.azurebfs.services.ReadBufferManager=TRACE
Review Comment:
Was this added for testing as this might add a lot of logging ?
##
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/ReadBuffer.java:
##
@@ -162,4 +182,120 @@ public void setAnyByteConsumed(boolean isAnyByteConsumed)
{
this.isAnyByteConsumed = isAnyByteConsumed;
}
+ @Override
+ public String toString() {
+return super.toString() +
+"{ status=" + status +
+", offset=" + offset +
+", length=" + length +
+
[jira] [Commented] (HADOOP-18521) ABFS ReadBufferManager buffer sharing across concurrent HTTP requests
[
https://issues.apache.org/jira/browse/HADOOP-18521?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17639959#comment-17639959
]
ASF GitHub Bot commented on HADOOP-18521:
-
anmolanmol1234 commented on code in PR #5117:
URL: https://github.com/apache/hadoop/pull/5117#discussion_r1033415579
##
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/ReadBufferStreamOperations.java:
##
@@ -0,0 +1,70 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.fs.azurebfs.services;
+
+import java.io.IOException;
+
+import org.apache.hadoop.fs.azurebfs.utils.TracingContext;
+import org.apache.hadoop.fs.statistics.impl.IOStatisticsStore;
+
+/**
+ * Interface which is required for read buffer stream
+ * calls.
+ * Extracted from {@code AbfsInputStream} to make testing
+ * easier and to isolate what operations the read buffer
+ * makes of the streams using it.
+ */
+interface ReadBufferStreamOperations {
+
+ /**
+ * Read a block from the store.
+ * @param position position in file
+ * @param b destination buffer.
+ * @param offset offset in buffer
+ * @param length length of read
+ * @param tracingContext trace context
+ * @return count of bytes read.
+ * @throws IOException failure.
+ */
+ int readRemote(long position,
+ byte[] b,
+ int offset,
+ int length,
+ TracingContext tracingContext) throws IOException;
+
+ /**
+ * Is the stream closed?
+ * This must be thread safe as prefetch operations in
+ * different threads probe this before closure.
+ * @return true if the stream has been closed.
+ */
+ boolean isClosed();
+
+ String getStreamID();
+
+ IOStatisticsStore getIOStatistics();
+
+ /**
+ * Get the stream path as a string.
+ * @return path string.
+ */
+ String getPath();
+
Review Comment:
nit: Additional line, can be removed
> ABFS ReadBufferManager buffer sharing across concurrent HTTP requests
> -
>
> Key: HADOOP-18521
> URL: https://issues.apache.org/jira/browse/HADOOP-18521
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs/azure
>Affects Versions: 3.3.2, 3.3.3, 3.3.4
>Reporter: Steve Loughran
>Assignee: Steve Loughran
>Priority: Critical
> Labels: pull-request-available
>
> AbfsInputStream.close() can trigger the return of buffers used for active
> prefetch GET requests into the ReadBufferManager free buffer pool.
> A subsequent prefetch by a different stream in the same process may acquire
> this same buffer. This can lead to risk of corruption of its own prefetched
> data, data which may then be returned to that other thread.
> On releases without the fix for this (3.3.2+), the bug can be avoided by
> disabling all prefetching
> {code}
> fs.azure.readaheadqueue.depth = 0
> {code}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[jira] [Commented] (HADOOP-18521) ABFS ReadBufferManager buffer sharing across concurrent HTTP requests
[
https://issues.apache.org/jira/browse/HADOOP-18521?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17639958#comment-17639958
]
ASF GitHub Bot commented on HADOOP-18521:
-
anmolanmol1234 commented on code in PR #5117:
URL: https://github.com/apache/hadoop/pull/5117#discussion_r1033415579
##
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/ReadBufferStreamOperations.java:
##
@@ -0,0 +1,70 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.fs.azurebfs.services;
+
+import java.io.IOException;
+
+import org.apache.hadoop.fs.azurebfs.utils.TracingContext;
+import org.apache.hadoop.fs.statistics.impl.IOStatisticsStore;
+
+/**
+ * Interface which is required for read buffer stream
+ * calls.
+ * Extracted from {@code AbfsInputStream} to make testing
+ * easier and to isolate what operations the read buffer
+ * makes of the streams using it.
+ */
+interface ReadBufferStreamOperations {
+
+ /**
+ * Read a block from the store.
+ * @param position position in file
+ * @param b destination buffer.
+ * @param offset offset in buffer
+ * @param length length of read
+ * @param tracingContext trace context
+ * @return count of bytes read.
+ * @throws IOException failure.
+ */
+ int readRemote(long position,
+ byte[] b,
+ int offset,
+ int length,
+ TracingContext tracingContext) throws IOException;
+
+ /**
+ * Is the stream closed?
+ * This must be thread safe as prefetch operations in
+ * different threads probe this before closure.
+ * @return true if the stream has been closed.
+ */
+ boolean isClosed();
+
+ String getStreamID();
+
+ IOStatisticsStore getIOStatistics();
+
+ /**
+ * Get the stream path as a string.
+ * @return path string.
+ */
+ String getPath();
+
Review Comment:
Additional line, can be removed
> ABFS ReadBufferManager buffer sharing across concurrent HTTP requests
> -
>
> Key: HADOOP-18521
> URL: https://issues.apache.org/jira/browse/HADOOP-18521
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs/azure
>Affects Versions: 3.3.2, 3.3.3, 3.3.4
>Reporter: Steve Loughran
>Assignee: Steve Loughran
>Priority: Critical
> Labels: pull-request-available
>
> AbfsInputStream.close() can trigger the return of buffers used for active
> prefetch GET requests into the ReadBufferManager free buffer pool.
> A subsequent prefetch by a different stream in the same process may acquire
> this same buffer. This can lead to risk of corruption of its own prefetched
> data, data which may then be returned to that other thread.
> On releases without the fix for this (3.3.2+), the bug can be avoided by
> disabling all prefetching
> {code}
> fs.azure.readaheadqueue.depth = 0
> {code}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[jira] [Commented] (HADOOP-18521) ABFS ReadBufferManager buffer sharing across concurrent HTTP requests
[
https://issues.apache.org/jira/browse/HADOOP-18521?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17639950#comment-17639950
]
ASF GitHub Bot commented on HADOOP-18521:
-
pranavsaxena-microsoft commented on code in PR #5117:
URL: https://github.com/apache/hadoop/pull/5117#discussion_r1033198624
##
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/ReadBufferManager.java:
##
@@ -454,31 +597,86 @@ ReadBuffer getNextBlockToRead() throws
InterruptedException {
*/
void doneReading(final ReadBuffer buffer, final ReadBufferStatus result,
final int bytesActuallyRead) {
if (LOGGER.isTraceEnabled()) {
- LOGGER.trace("ReadBufferWorker completed read file {} for offset {}
outcome {} bytes {}",
- buffer.getStream().getPath(), buffer.getOffset(), result,
bytesActuallyRead);
-}
-synchronized (this) {
- // If this buffer has already been purged during
- // close of InputStream then we don't update the lists.
- if (inProgressList.contains(buffer)) {
-inProgressList.remove(buffer);
-if (result == ReadBufferStatus.AVAILABLE && bytesActuallyRead > 0) {
- buffer.setStatus(ReadBufferStatus.AVAILABLE);
- buffer.setLength(bytesActuallyRead);
-} else {
- freeList.push(buffer.getBufferindex());
- // buffer will be deleted as per the eviction policy.
+ LOGGER.trace("ReadBufferWorker completed file {} for offset {} bytes {};
{}",
+ buffer.getStream().getPath(), buffer.getOffset(),
bytesActuallyRead, buffer);
+}
+// decrement counter.
+buffer.prefetchFinished();
+
+try {
+ synchronized (this) {
+// remove from the list
+if (!inProgressList.remove(buffer)) {
+ // this is a sign of inconsistent state, so a major problem, such as
+ // double invocation of this method with the same buffer.
+ String message =
+ String.format("Read completed from an operation not declared as
in progress %s",
+ buffer);
+ LOGGER.warn(message);
+ if (buffer.hasIndexedBuffer()) {
+// release the buffer (which may raise an exception)
+placeBufferOnFreeList("read not in progress", buffer);
+ }
+ // report the failure
+ throw new IllegalStateException(message);
}
-// completed list also contains FAILED read buffers
-// for sending exception message to clients.
+
+// should the read buffer be added to the completed list?
+boolean addCompleted = true;
+// flag to indicate buffer should be freed
+boolean shouldFreeBuffer = false;
+// and the reason (for logging)
+String freeBufferReason = "";
+
buffer.setStatus(result);
buffer.setTimeStamp(currentTimeMillis());
-completedReadList.add(buffer);
+// did the read return any data?
+if (result == ReadBufferStatus.AVAILABLE
+&& bytesActuallyRead > 0) {
+ // successful read of data; update buffer state.
+ buffer.setLength(bytesActuallyRead);
+} else {
+ // read failed or there was no data; the buffer can be returned to
the free list.
+ shouldFreeBuffer = true;
+ freeBufferReason = "failed read";
+ // completed list also contains FAILED read buffers
+ // for sending exception message to clients.
+ // NOTE: checks for closed state may update this.
+ addCompleted = result == ReadBufferStatus.READ_FAILED;
Review Comment:
Should we not add the buffer in completedList. Reason being it will always
be removed from the list via oldFailedBuffers removal. Kindly suggest if there
is a reason for adding it. Thanks.
> ABFS ReadBufferManager buffer sharing across concurrent HTTP requests
> -
>
> Key: HADOOP-18521
> URL: https://issues.apache.org/jira/browse/HADOOP-18521
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs/azure
>Affects Versions: 3.3.2, 3.3.3, 3.3.4
>Reporter: Steve Loughran
>Assignee: Steve Loughran
>Priority: Critical
> Labels: pull-request-available
>
> AbfsInputStream.close() can trigger the return of buffers used for active
> prefetch GET requests into the ReadBufferManager free buffer pool.
> A subsequent prefetch by a different stream in the same process may acquire
> this same buffer. This can lead to risk of corruption of its own prefetched
> data, data which may then be returned to that other thread.
> On releases without the fix for this (3.3.2+), the bug can be avoided by
> disabling all prefetching
> {code}
> fs.azure.readaheadqueue.depth = 0
> {code}
--
This message
[jira] [Commented] (HADOOP-18521) ABFS ReadBufferManager buffer sharing across concurrent HTTP requests
[
https://issues.apache.org/jira/browse/HADOOP-18521?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17639905#comment-17639905
]
ASF GitHub Bot commented on HADOOP-18521:
-
pranavsaxena-microsoft commented on code in PR #5117:
URL: https://github.com/apache/hadoop/pull/5117#discussion_r1033198624
##
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/ReadBufferManager.java:
##
@@ -454,31 +597,86 @@ ReadBuffer getNextBlockToRead() throws
InterruptedException {
*/
void doneReading(final ReadBuffer buffer, final ReadBufferStatus result,
final int bytesActuallyRead) {
if (LOGGER.isTraceEnabled()) {
- LOGGER.trace("ReadBufferWorker completed read file {} for offset {}
outcome {} bytes {}",
- buffer.getStream().getPath(), buffer.getOffset(), result,
bytesActuallyRead);
-}
-synchronized (this) {
- // If this buffer has already been purged during
- // close of InputStream then we don't update the lists.
- if (inProgressList.contains(buffer)) {
-inProgressList.remove(buffer);
-if (result == ReadBufferStatus.AVAILABLE && bytesActuallyRead > 0) {
- buffer.setStatus(ReadBufferStatus.AVAILABLE);
- buffer.setLength(bytesActuallyRead);
-} else {
- freeList.push(buffer.getBufferindex());
- // buffer will be deleted as per the eviction policy.
+ LOGGER.trace("ReadBufferWorker completed file {} for offset {} bytes {};
{}",
+ buffer.getStream().getPath(), buffer.getOffset(),
bytesActuallyRead, buffer);
+}
+// decrement counter.
+buffer.prefetchFinished();
+
+try {
+ synchronized (this) {
+// remove from the list
+if (!inProgressList.remove(buffer)) {
+ // this is a sign of inconsistent state, so a major problem, such as
+ // double invocation of this method with the same buffer.
+ String message =
+ String.format("Read completed from an operation not declared as
in progress %s",
+ buffer);
+ LOGGER.warn(message);
+ if (buffer.hasIndexedBuffer()) {
+// release the buffer (which may raise an exception)
+placeBufferOnFreeList("read not in progress", buffer);
+ }
+ // report the failure
+ throw new IllegalStateException(message);
}
-// completed list also contains FAILED read buffers
-// for sending exception message to clients.
+
+// should the read buffer be added to the completed list?
+boolean addCompleted = true;
+// flag to indicate buffer should be freed
+boolean shouldFreeBuffer = false;
+// and the reason (for logging)
+String freeBufferReason = "";
+
buffer.setStatus(result);
buffer.setTimeStamp(currentTimeMillis());
-completedReadList.add(buffer);
+// did the read return any data?
+if (result == ReadBufferStatus.AVAILABLE
+&& bytesActuallyRead > 0) {
+ // successful read of data; update buffer state.
+ buffer.setLength(bytesActuallyRead);
+} else {
+ // read failed or there was no data; the buffer can be returned to
the free list.
+ shouldFreeBuffer = true;
+ freeBufferReason = "failed read";
+ // completed list also contains FAILED read buffers
+ // for sending exception message to clients.
+ // NOTE: checks for closed state may update this.
+ addCompleted = result == ReadBufferStatus.READ_FAILED;
Review Comment:
Should we not add the buffer in completedList. Reason being it will always
be removed from the list via oldFailedBuffers removal. Kindly suggest if there
is a reason for adding it. Thanks.
> ABFS ReadBufferManager buffer sharing across concurrent HTTP requests
> -
>
> Key: HADOOP-18521
> URL: https://issues.apache.org/jira/browse/HADOOP-18521
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs/azure
>Affects Versions: 3.3.2, 3.3.3, 3.3.4
>Reporter: Steve Loughran
>Assignee: Steve Loughran
>Priority: Critical
> Labels: pull-request-available
>
> AbfsInputStream.close() can trigger the return of buffers used for active
> prefetch GET requests into the ReadBufferManager free buffer pool.
> A subsequent prefetch by a different stream in the same process may acquire
> this same buffer. This can lead to risk of corruption of its own prefetched
> data, data which may then be returned to that other thread.
> On releases without the fix for this (3.3.2+), the bug can be avoided by
> disabling all prefetching
> {code}
> fs.azure.readaheadqueue.depth = 0
> {code}
--
This message
[jira] [Commented] (HADOOP-18521) ABFS ReadBufferManager buffer sharing across concurrent HTTP requests
[
https://issues.apache.org/jira/browse/HADOOP-18521?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17638780#comment-17638780
]
ASF GitHub Bot commented on HADOOP-18521:
-
hadoop-yetus commented on PR #5117:
URL: https://github.com/apache/hadoop/pull/5117#issuecomment-1327901575
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|::|--:|:|::|:---:|
| +0 :ok: | reexec | 0m 56s | | 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 6 new or modified test files. |
_ trunk Compile Tests _ |
| +0 :ok: | mvndep | 16m 56s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 26m 26s | | trunk passed |
| +1 :green_heart: | compile | 23m 44s | | trunk passed with JDK
Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | compile | 21m 1s | | trunk passed with JDK
Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | checkstyle | 4m 17s | | trunk passed |
| +1 :green_heart: | mvnsite | 3m 21s | | trunk passed |
| +1 :green_heart: | javadoc | 2m 37s | | trunk passed with JDK
Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javadoc | 2m 9s | | trunk passed with JDK
Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 4m 34s | | trunk passed |
| +1 :green_heart: | shadedclient | 21m 50s | | branch has no errors
when building and testing our client artifacts. |
_ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 29s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 1m 39s | | the patch passed |
| +1 :green_heart: | compile | 22m 36s | | the patch passed with JDK
Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javac | 22m 36s | | the patch passed |
| +1 :green_heart: | compile | 20m 55s | | the patch passed with JDK
Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | javac | 20m 55s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks
issues. |
| -0 :warning: | checkstyle | 4m 1s |
[/results-checkstyle-root.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5117/6/artifact/out/results-checkstyle-root.txt)
| root: The patch generated 38 new + 1 unchanged - 0 fixed = 39 total (was 1)
|
| +1 :green_heart: | mvnsite | 3m 7s | | the patch passed |
| +1 :green_heart: | javadoc | 2m 30s | | the patch passed with JDK
Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javadoc | 1m 59s | | the patch passed with JDK
Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 4m 33s | | the patch passed |
| +1 :green_heart: | shadedclient | 21m 47s | | patch has no errors
when building and testing our client artifacts. |
_ Other Tests _ |
| +1 :green_heart: | unit | 18m 42s | | hadoop-common in the patch
passed. |
| +1 :green_heart: | unit | 2m 43s | | hadoop-azure in the patch
passed. |
| +1 :green_heart: | asflicense | 1m 24s | | The patch does not
generate ASF License warnings. |
| | | 239m 28s | | |
| Subsystem | Report/Notes |
|--:|:-|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base:
https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5117/6/artifact/out/Dockerfile
|
| GITHUB PR | https://github.com/apache/hadoop/pull/5117 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall
mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
| uname | Linux 8beba5d39166 4.15.0-191-generic #202-Ubuntu SMP Thu Aug 4
01:49:29 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / f72ad51e28eb52321c5be354db1f2cc08771a25d |
| Default Java | Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| Multi-JDK versions |
/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04
/usr/lib/jvm/java-8-openjdk-amd64:Private
Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| Test Results |
https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5117/6/testReport/ |
| Max. pr
[jira] [Commented] (HADOOP-18521) ABFS ReadBufferManager buffer sharing across concurrent HTTP requests
[
https://issues.apache.org/jira/browse/HADOOP-18521?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17638677#comment-17638677
]
ASF GitHub Bot commented on HADOOP-18521:
-
hadoop-yetus commented on PR #5117:
URL: https://github.com/apache/hadoop/pull/5117#issuecomment-1327598664
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|::|--:|:|::|:---:|
| +0 :ok: | reexec | 0m 50s | | 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 5 new or modified test files. |
_ trunk Compile Tests _ |
| +0 :ok: | mvndep | 15m 56s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 26m 18s | | trunk passed |
| +1 :green_heart: | compile | 23m 35s | | trunk passed with JDK
Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | compile | 20m 56s | | trunk passed with JDK
Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | checkstyle | 4m 7s | | trunk passed |
| +1 :green_heart: | mvnsite | 3m 4s | | trunk passed |
| +1 :green_heart: | javadoc | 2m 43s | | trunk passed with JDK
Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javadoc | 1m 58s | | trunk passed with JDK
Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 4m 36s | | trunk passed |
| +1 :green_heart: | shadedclient | 21m 58s | | branch has no errors
when building and testing our client artifacts. |
_ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 31s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 1m 42s | | the patch passed |
| +1 :green_heart: | compile | 22m 46s | | the patch passed with JDK
Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javac | 22m 46s | | the patch passed |
| +1 :green_heart: | compile | 21m 1s | | the patch passed with JDK
Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | javac | 21m 1s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks
issues. |
| -0 :warning: | checkstyle | 3m 54s |
[/results-checkstyle-root.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5117/5/artifact/out/results-checkstyle-root.txt)
| root: The patch generated 37 new + 1 unchanged - 0 fixed = 38 total (was 1)
|
| +1 :green_heart: | mvnsite | 2m 58s | | the patch passed |
| +1 :green_heart: | javadoc | 2m 19s | | the patch passed with JDK
Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javadoc | 2m 13s | | the patch passed with JDK
Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 4m 31s | | the patch passed |
| +1 :green_heart: | shadedclient | 22m 3s | | patch has no errors
when building and testing our client artifacts. |
_ Other Tests _ |
| +1 :green_heart: | unit | 18m 36s | | hadoop-common in the patch
passed. |
| +1 :green_heart: | unit | 2m 45s | | hadoop-azure in the patch
passed. |
| +1 :green_heart: | asflicense | 1m 22s | | The patch does not
generate ASF License warnings. |
| | | 237m 48s | | |
| Subsystem | Report/Notes |
|--:|:-|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base:
https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5117/5/artifact/out/Dockerfile
|
| GITHUB PR | https://github.com/apache/hadoop/pull/5117 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall
mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
| uname | Linux d6a7789fd8d3 4.15.0-191-generic #202-Ubuntu SMP Thu Aug 4
01:49:29 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / 5415ba36518ee06540d75cc5248d989120e59fa8 |
| Default Java | Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| Multi-JDK versions |
/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04
/usr/lib/jvm/java-8-openjdk-amd64:Private
Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| Test Results |
https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5117/5/testReport/ |
| Max. pr
[jira] [Commented] (HADOOP-18521) ABFS ReadBufferManager buffer sharing across concurrent HTTP requests
[
https://issues.apache.org/jira/browse/HADOOP-18521?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17638300#comment-17638300
]
ASF GitHub Bot commented on HADOOP-18521:
-
steveloughran commented on code in PR #5133:
URL: https://github.com/apache/hadoop/pull/5133#discussion_r1031522202
##
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/ReadBufferManager.java:
##
@@ -247,7 +247,7 @@ private synchronized boolean tryEvict() {
// first, try buffers where all bytes have been consumed (approximated as
first and last bytes consumed)
for (ReadBuffer buf : completedReadList) {
- if (buf.isFirstByteConsumed() && buf.isLastByteConsumed()) {
+ if (buf.getStream().isClosed() || (buf.isFirstByteConsumed() &&
buf.isLastByteConsumed())) {
Review Comment:
this doesn't quite do the right thing as evict() is looking for a completed
read with an allocated buffer. here it will also find any completed read whose
buffer was released prematurely.
i'm picking up this change in my pr, but including the check for having the
buffer, so it will *only* pick up those records whose stream was closed after a
successful read completed.
> ABFS ReadBufferManager buffer sharing across concurrent HTTP requests
> -
>
> Key: HADOOP-18521
> URL: https://issues.apache.org/jira/browse/HADOOP-18521
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs/azure
>Affects Versions: 3.3.2, 3.3.3, 3.3.4
>Reporter: Steve Loughran
>Assignee: Steve Loughran
>Priority: Critical
> Labels: pull-request-available
>
> AbfsInputStream.close() can trigger the return of buffers used for active
> prefetch GET requests into the ReadBufferManager free buffer pool.
> A subsequent prefetch by a different stream in the same process may acquire
> this same buffer. This can lead to risk of corruption of its own prefetched
> data, data which may then be returned to that other thread.
> On releases without the fix for this (3.3.2+), the bug can be avoided by
> disabling all prefetching
> {code}
> fs.azure.readaheadqueue.depth = 0
> {code}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[jira] [Commented] (HADOOP-18521) ABFS ReadBufferManager buffer sharing across concurrent HTTP requests
[
https://issues.apache.org/jira/browse/HADOOP-18521?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17634944#comment-17634944
]
ASF GitHub Bot commented on HADOOP-18521:
-
steveloughran commented on PR #5133:
URL: https://github.com/apache/hadoop/pull/5133#issuecomment-1317390002
the big read manager changes should be a followup...what's good here is
something lower risk and easier to backport
> ABFS ReadBufferManager buffer sharing across concurrent HTTP requests
> -
>
> Key: HADOOP-18521
> URL: https://issues.apache.org/jira/browse/HADOOP-18521
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs/azure
>Affects Versions: 3.3.2, 3.3.3, 3.3.4
>Reporter: Steve Loughran
>Assignee: Steve Loughran
>Priority: Critical
> Labels: pull-request-available
>
> AbfsInputStream.close() can trigger the return of buffers used for active
> prefetch GET requests into the ReadBufferManager free buffer pool.
> A subsequent prefetch by a different stream in the same process may acquire
> this same buffer. This can lead to risk of corruption of its own prefetched
> data, data which may then be returned to that other thread.
> On releases without the fix for this (3.3.2+), the bug can be avoided by
> disabling all prefetching
> {code}
> fs.azure.readaheadqueue.depth = 0
> {code}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[jira] [Commented] (HADOOP-18521) ABFS ReadBufferManager buffer sharing across concurrent HTTP requests
[
https://issues.apache.org/jira/browse/HADOOP-18521?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17634407#comment-17634407
]
ASF GitHub Bot commented on HADOOP-18521:
-
snvijaya commented on PR #5133:
URL: https://github.com/apache/hadoop/pull/5133#issuecomment-1315456701
> Having spent the last couple of weeks staring at this code, I think what I
would like from the module is
>
> * per file system instance
> * stats reporting to that fs (gauges on queues; actual fetch/hit/miss
numbers could come via abfs input stream)
> * demand creation of the buffer pool. Maybe when the last active stream is
closed it could even free them all, no that at the reference counting problem
to the mix.
> * when an abfs input stream is created for vector/random IO, prefetch
being automatically disabled.
> * anything we can do to improve testing, especially simulation of heavy
load from multiple tasks.
Thanks for your inputs @steveloughran . Agree that all these apply for the
long term transformation on the prefetch handling. I am going to take a day
and see what of these I can get into this change as well, but want to
prioritize a hotfix and remove the buggy bit quickly.
> ABFS ReadBufferManager buffer sharing across concurrent HTTP requests
> -
>
> Key: HADOOP-18521
> URL: https://issues.apache.org/jira/browse/HADOOP-18521
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs/azure
>Affects Versions: 3.3.2, 3.3.3, 3.3.4
>Reporter: Steve Loughran
>Assignee: Steve Loughran
>Priority: Critical
> Labels: pull-request-available
>
> AbfsInputStream.close() can trigger the return of buffers used for active
> prefetch GET requests into the ReadBufferManager free buffer pool.
> A subsequent prefetch by a different stream in the same process may acquire
> this same buffer. This can lead to risk of corruption of its own prefetched
> data, data which may then be returned to that other thread.
> On releases without the fix for this (3.3.2+), the bug can be avoided by
> disabling all prefetching
> {code}
> fs.azure.readaheadqueue.depth = 0
> {code}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[jira] [Commented] (HADOOP-18521) ABFS ReadBufferManager buffer sharing across concurrent HTTP requests
[
https://issues.apache.org/jira/browse/HADOOP-18521?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17633912#comment-17633912
]
ASF GitHub Bot commented on HADOOP-18521:
-
steveloughran commented on PR #5117:
URL: https://github.com/apache/hadoop/pull/5117#issuecomment-1313988205
see also #5134 which is the "disable readahead" patch for 3.3.5
> ABFS ReadBufferManager buffer sharing across concurrent HTTP requests
> -
>
> Key: HADOOP-18521
> URL: https://issues.apache.org/jira/browse/HADOOP-18521
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs/azure
>Affects Versions: 3.3.2, 3.3.3, 3.3.4
>Reporter: Steve Loughran
>Assignee: Steve Loughran
>Priority: Critical
> Labels: pull-request-available
>
> AbfsInputStream.close() can trigger the return of buffers used for active
> prefetch GET requests into the ReadBufferManager free buffer pool.
> A subsequent prefetch by a different stream in the same process may acquire
> this same buffer. This can lead to risk of corruption of its own prefetched
> data, data which may then be returned to that other thread.
> On releases without the fix for this (3.3.2 to 3.3.4), the bug can be avoided
> by disabling all prefetching
> {code}
> fs.azure.readaheadqueue.depth = 0
> {code}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[jira] [Commented] (HADOOP-18521) ABFS ReadBufferManager buffer sharing across concurrent HTTP requests
[
https://issues.apache.org/jira/browse/HADOOP-18521?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17633807#comment-17633807
]
ASF GitHub Bot commented on HADOOP-18521:
-
snvijaya commented on PR #5117:
URL: https://github.com/apache/hadoop/pull/5117#issuecomment-1313734822
Hi @steveloughran, Wanted to get your opinion on below change as a possible
replacement for this change :
[https://github.com/apache/hadoop/pull/5133](url)
A ReadBuffer with a valid Buffer assigned to it can be in certain states
when stream is closed, and with the above change, I am trying to address it as
below :
1. Is in QueueReadAheadList - No change, the earlier purge takes care of it
2. Is in CompletedList - No change again, the earlier purge takes care of it
3. Is InProgressList but yet to make the network call - If stream is closed,
stop network call and move the ReadBuffer as a failure into completed list
4. Is InProgressList , just finished the network call - If stream is closed,
network call was successful or not, move the ReadBuffer as a failure into
completed list
Now, when in state 3 or 4, the purge method might not pick it as it might
have executed first. In that case, to prioritize these ReadBuffers for
eviction, have added the check for stream is closed in the eviction code as
well.
Please let me know if you see value in this fix and I could pursue further
changes to incorporate validation code at queuing time and when getBlock finds
a hit in completed list, and will also add related test code.
> ABFS ReadBufferManager buffer sharing across concurrent HTTP requests
> -
>
> Key: HADOOP-18521
> URL: https://issues.apache.org/jira/browse/HADOOP-18521
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs/azure
>Affects Versions: 3.3.2, 3.3.3, 3.3.4
>Reporter: Steve Loughran
>Assignee: Steve Loughran
>Priority: Critical
> Labels: pull-request-available
>
> AbfsInputStream.close() can trigger the return of buffers used for active
> prefetch GET requests into the ReadBufferManager free buffer pool.
> A subsequent prefetch by a different stream in the same process may acquire
> this same buffer. This can lead to risk of corruption of its own prefetched
> data, data which may then be returned to that other thread.
> On releases without the fix for this (3.3.2 to 3.3.4), the bug can be avoided
> by disabling all prefetching
> {code}
> fs.azure.readaheadqueue.depth = 0
> {code}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[jira] [Commented] (HADOOP-18521) ABFS ReadBufferManager buffer sharing across concurrent HTTP requests
[
https://issues.apache.org/jira/browse/HADOOP-18521?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17632536#comment-17632536
]
ASF GitHub Bot commented on HADOOP-18521:
-
steveloughran commented on PR #5117:
URL: https://github.com/apache/hadoop/pull/5117#issuecomment-1312096951
thx for the comments; been away from this on the doc/replication side of
this "issue" all week. have been able to replicate the problem with avro
parsing, though there it always fails at the parser; on csv/text records that's
not guaranteed
> ABFS ReadBufferManager buffer sharing across concurrent HTTP requests
> -
>
> Key: HADOOP-18521
> URL: https://issues.apache.org/jira/browse/HADOOP-18521
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs/azure
>Affects Versions: 3.3.2, 3.3.3, 3.3.4
>Reporter: Steve Loughran
>Assignee: Steve Loughran
>Priority: Critical
> Labels: pull-request-available
>
> AbfsInputStream.close() can trigger the return of buffers used for active
> prefetch GET requests into the ReadBufferManager free buffer pool.
> A subsequent prefetch by a different stream in the same process may acquire
> this same buffer. This can lead to risk of corruption of its own prefetched
> data, data which may then be returned to that other thread.
> On releases without the fix for this (3.3.2 to 3.3.4), the bug can be avoided
> by disabling all prefetching
> {code}
> fs.azure.readaheadqueue.depth
> {code}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[jira] [Commented] (HADOOP-18521) ABFS ReadBufferManager buffer sharing across concurrent HTTP requests
[
https://issues.apache.org/jira/browse/HADOOP-18521?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17632535#comment-17632535
]
ASF GitHub Bot commented on HADOOP-18521:
-
steveloughran commented on code in PR #5117:
URL: https://github.com/apache/hadoop/pull/5117#discussion_r1020503409
##
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/ReadBufferManager.java:
##
@@ -454,31 +588,65 @@ ReadBuffer getNextBlockToRead() throws
InterruptedException {
*/
void doneReading(final ReadBuffer buffer, final ReadBufferStatus result,
final int bytesActuallyRead) {
if (LOGGER.isTraceEnabled()) {
- LOGGER.trace("ReadBufferWorker completed read file {} for offset {}
outcome {} bytes {}",
- buffer.getStream().getPath(), buffer.getOffset(), result,
bytesActuallyRead);
-}
-synchronized (this) {
- // If this buffer has already been purged during
- // close of InputStream then we don't update the lists.
- if (inProgressList.contains(buffer)) {
-inProgressList.remove(buffer);
+ LOGGER.trace("ReadBufferWorker completed file {} for offset {} bytes {};
{}",
+ buffer.getStream().getPath(), buffer.getOffset(),
bytesActuallyRead, buffer);
+}
+// decrement counter.
+buffer.prefetchFinished();
+
+try {
+ synchronized (this) {
+// remove from the list
+if (!inProgressList.remove(buffer)) {
+ // this is a sign of inconsistent state, so a major problem
+ String message =
+ String.format("Read completed from an operation not declared as
in progress %s",
+ buffer);
+ LOGGER.warn(message);
+ // release the buffer (which may raise an exception)
+ placeBufferOnFreeList("read not in progress", buffer);
+ // report the failure
+ throw new IllegalStateException(message);
+}
+
+boolean shouldFreeBuffer = false;
+String freeBufferReason = "";
if (result == ReadBufferStatus.AVAILABLE && bytesActuallyRead > 0) {
buffer.setStatus(ReadBufferStatus.AVAILABLE);
buffer.setLength(bytesActuallyRead);
} else {
- freeList.push(buffer.getBufferindex());
- // buffer will be deleted as per the eviction policy.
+ // read failed or there was no data, -the buffer can be returned to
the free list.
+ shouldFreeBuffer = true;
+ freeBufferReason = "failed read";
}
// completed list also contains FAILED read buffers
// for sending exception message to clients.
buffer.setStatus(result);
buffer.setTimeStamp(currentTimeMillis());
-completedReadList.add(buffer);
+if (!buffer.isStreamClosed()) {
+ // completed reads are added to the list.
+ LOGGER.trace("Adding buffer to completed list {}", buffer);
+ completedReadList.add(buffer);
Review Comment:
making some changes. this is a complex bit of code and why I plan to write
some unit tests to explore the results; i will take what you've done too
@pranavsaxena-microsoft
> ABFS ReadBufferManager buffer sharing across concurrent HTTP requests
> -
>
> Key: HADOOP-18521
> URL: https://issues.apache.org/jira/browse/HADOOP-18521
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs/azure
>Affects Versions: 3.3.2, 3.3.3, 3.3.4
>Reporter: Steve Loughran
>Assignee: Steve Loughran
>Priority: Critical
> Labels: pull-request-available
>
> AbfsInputStream.close() can trigger the return of buffers used for active
> prefetch GET requests into the ReadBufferManager free buffer pool.
> A subsequent prefetch by a different stream in the same process may acquire
> this same buffer. This can lead to risk of corruption of its own prefetched
> data, data which may then be returned to that other thread.
> On releases without the fix for this (3.3.2 to 3.3.4), the bug can be avoided
> by disabling all prefetching
> {code}
> fs.azure.readaheadqueue.depth
> {code}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[jira] [Commented] (HADOOP-18521) ABFS ReadBufferManager buffer sharing across concurrent HTTP requests
[
https://issues.apache.org/jira/browse/HADOOP-18521?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17632523#comment-17632523
]
ASF GitHub Bot commented on HADOOP-18521:
-
steveloughran commented on code in PR #5117:
URL: https://github.com/apache/hadoop/pull/5117#discussion_r1020491125
##
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/ReadBufferManager.java:
##
@@ -454,31 +588,65 @@ ReadBuffer getNextBlockToRead() throws
InterruptedException {
*/
void doneReading(final ReadBuffer buffer, final ReadBufferStatus result,
final int bytesActuallyRead) {
if (LOGGER.isTraceEnabled()) {
- LOGGER.trace("ReadBufferWorker completed read file {} for offset {}
outcome {} bytes {}",
- buffer.getStream().getPath(), buffer.getOffset(), result,
bytesActuallyRead);
-}
-synchronized (this) {
- // If this buffer has already been purged during
- // close of InputStream then we don't update the lists.
- if (inProgressList.contains(buffer)) {
-inProgressList.remove(buffer);
+ LOGGER.trace("ReadBufferWorker completed file {} for offset {} bytes {};
{}",
+ buffer.getStream().getPath(), buffer.getOffset(),
bytesActuallyRead, buffer);
+}
+// decrement counter.
+buffer.prefetchFinished();
+
+try {
+ synchronized (this) {
+// remove from the list
+if (!inProgressList.remove(buffer)) {
+ // this is a sign of inconsistent state, so a major problem
+ String message =
+ String.format("Read completed from an operation not declared as
in progress %s",
+ buffer);
+ LOGGER.warn(message);
+ // release the buffer (which may raise an exception)
+ placeBufferOnFreeList("read not in progress", buffer);
+ // report the failure
+ throw new IllegalStateException(message);
+}
+
+boolean shouldFreeBuffer = false;
+String freeBufferReason = "";
if (result == ReadBufferStatus.AVAILABLE && bytesActuallyRead > 0) {
buffer.setStatus(ReadBufferStatus.AVAILABLE);
buffer.setLength(bytesActuallyRead);
} else {
- freeList.push(buffer.getBufferindex());
- // buffer will be deleted as per the eviction policy.
+ // read failed or there was no data, -the buffer can be returned to
the free list.
+ shouldFreeBuffer = true;
+ freeBufferReason = "failed read";
}
// completed list also contains FAILED read buffers
// for sending exception message to clients.
buffer.setStatus(result);
Review Comment:
i've moved that check down. and with the move to an interface for abfs
interaction, now in a position to create a test to simulate the conditions,
including "available but empty" and "io error" on normal and prefetch reads
> ABFS ReadBufferManager buffer sharing across concurrent HTTP requests
> -
>
> Key: HADOOP-18521
> URL: https://issues.apache.org/jira/browse/HADOOP-18521
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs/azure
>Affects Versions: 3.3.2, 3.3.3, 3.3.4
>Reporter: Steve Loughran
>Assignee: Steve Loughran
>Priority: Critical
> Labels: pull-request-available
>
> AbfsInputStream.close() can trigger the return of buffers used for active
> prefetch GET requests into the ReadBufferManager free buffer pool.
> A subsequent prefetch by a different stream in the same process may acquire
> this same buffer. This can lead to risk of corruption of its own prefetched
> data, data which may then be returned to that other thread.
> On releases without the fix for this (3.3.2 to 3.3.4), the bug can be avoided
> by disabling all prefetching
> {code}
> fs.azure.readaheadqueue.depth
> {code}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[jira] [Commented] (HADOOP-18521) ABFS ReadBufferManager buffer sharing across concurrent HTTP requests
[
https://issues.apache.org/jira/browse/HADOOP-18521?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17632517#comment-17632517
]
ASF GitHub Bot commented on HADOOP-18521:
-
steveloughran commented on code in PR #5117:
URL: https://github.com/apache/hadoop/pull/5117#discussion_r1020487323
##
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/ReadBufferManager.java:
##
@@ -302,33 +325,33 @@ private synchronized boolean tryEvict() {
}
private boolean evict(final ReadBuffer buf) {
-// As failed ReadBuffers (bufferIndx = -1) are saved in completedReadList,
-// avoid adding it to freeList.
-if (buf.getBufferindex() != -1) {
- freeList.push(buf.getBufferindex());
-}
-
-completedReadList.remove(buf);
buf.setTracingContext(null);
if (LOGGER.isTraceEnabled()) {
LOGGER.trace("Evicting buffer idx {}; was used for file {} offset {}
length {}",
buf.getBufferindex(), buf.getStream().getPath(), buf.getOffset(),
buf.getLength());
}
+completedReadList.remove(buf);
Review Comment:
ok. i'd positioned where they were so the invariant "not in use" held.
maybe the validateReadManagerState() should go in at the end of the eviction
> ABFS ReadBufferManager buffer sharing across concurrent HTTP requests
> -
>
> Key: HADOOP-18521
> URL: https://issues.apache.org/jira/browse/HADOOP-18521
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs/azure
>Affects Versions: 3.3.2, 3.3.3, 3.3.4
>Reporter: Steve Loughran
>Assignee: Steve Loughran
>Priority: Critical
> Labels: pull-request-available
>
> AbfsInputStream.close() can trigger the return of buffers used for active
> prefetch GET requests into the ReadBufferManager free buffer pool.
> A subsequent prefetch by a different stream in the same process may acquire
> this same buffer. This can lead to risk of corruption of its own prefetched
> data, data which may then be returned to that other thread.
> On releases without the fix for this (3.3.2 to 3.3.4), the bug can be avoided
> by disabling all prefetching
> {code}
> fs.azure.readaheadqueue.depth
> {code}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[jira] [Commented] (HADOOP-18521) ABFS ReadBufferManager buffer sharing across concurrent HTTP requests
[
https://issues.apache.org/jira/browse/HADOOP-18521?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17632307#comment-17632307
]
ASF GitHub Bot commented on HADOOP-18521:
-
anmolanmol1234 commented on code in PR #5117:
URL: https://github.com/apache/hadoop/pull/5117#discussion_r1020173444
##
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/ReadBufferManager.java:
##
@@ -454,31 +588,65 @@ ReadBuffer getNextBlockToRead() throws
InterruptedException {
*/
void doneReading(final ReadBuffer buffer, final ReadBufferStatus result,
final int bytesActuallyRead) {
if (LOGGER.isTraceEnabled()) {
- LOGGER.trace("ReadBufferWorker completed read file {} for offset {}
outcome {} bytes {}",
- buffer.getStream().getPath(), buffer.getOffset(), result,
bytesActuallyRead);
-}
-synchronized (this) {
- // If this buffer has already been purged during
- // close of InputStream then we don't update the lists.
- if (inProgressList.contains(buffer)) {
-inProgressList.remove(buffer);
+ LOGGER.trace("ReadBufferWorker completed file {} for offset {} bytes {};
{}",
+ buffer.getStream().getPath(), buffer.getOffset(),
bytesActuallyRead, buffer);
+}
+// decrement counter.
+buffer.prefetchFinished();
+
+try {
+ synchronized (this) {
+// remove from the list
+if (!inProgressList.remove(buffer)) {
+ // this is a sign of inconsistent state, so a major problem
+ String message =
+ String.format("Read completed from an operation not declared as
in progress %s",
+ buffer);
+ LOGGER.warn(message);
+ // release the buffer (which may raise an exception)
+ placeBufferOnFreeList("read not in progress", buffer);
+ // report the failure
+ throw new IllegalStateException(message);
+}
+
+boolean shouldFreeBuffer = false;
+String freeBufferReason = "";
if (result == ReadBufferStatus.AVAILABLE && bytesActuallyRead > 0) {
buffer.setStatus(ReadBufferStatus.AVAILABLE);
buffer.setLength(bytesActuallyRead);
} else {
- freeList.push(buffer.getBufferindex());
- // buffer will be deleted as per the eviction policy.
+ // read failed or there was no data, -the buffer can be returned to
the free list.
+ shouldFreeBuffer = true;
+ freeBufferReason = "failed read";
}
// completed list also contains FAILED read buffers
// for sending exception message to clients.
buffer.setStatus(result);
buffer.setTimeStamp(currentTimeMillis());
-completedReadList.add(buffer);
+if (!buffer.isStreamClosed()) {
+ // completed reads are added to the list.
+ LOGGER.trace("Adding buffer to completed list {}", buffer);
+ completedReadList.add(buffer);
Review Comment:
Agree with this because the current flow might lead to double addition in
free list or inconsistency during addition.
> ABFS ReadBufferManager buffer sharing across concurrent HTTP requests
> -
>
> Key: HADOOP-18521
> URL: https://issues.apache.org/jira/browse/HADOOP-18521
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs/azure
>Affects Versions: 3.3.2, 3.3.3, 3.3.4
>Reporter: Steve Loughran
>Assignee: Steve Loughran
>Priority: Critical
> Labels: pull-request-available
>
> AbfsInputStream.close() can trigger the return of buffers used for active
> prefetch GET requests into the ReadBufferManager free buffer pool.
> A subsequent prefetch by a different stream in the same process may acquire
> this same buffer. This can lead to risk of corruption of its own prefetched
> data, data which may then be returned to that other thread.
> On releases without the fix for this (3.3.2 to 3.3.4), the bug can be avoided
> by disabling all prefetching
> {code}
> fs.azure.readaheadqueue.depth
> {code}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[jira] [Commented] (HADOOP-18521) ABFS ReadBufferManager buffer sharing across concurrent HTTP requests
[
https://issues.apache.org/jira/browse/HADOOP-18521?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17632301#comment-17632301
]
ASF GitHub Bot commented on HADOOP-18521:
-
anmolanmol1234 commented on code in PR #5117:
URL: https://github.com/apache/hadoop/pull/5117#discussion_r1020171703
##
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/ReadBufferManager.java:
##
@@ -302,33 +325,33 @@ private synchronized boolean tryEvict() {
}
private boolean evict(final ReadBuffer buf) {
-// As failed ReadBuffers (bufferIndx = -1) are saved in completedReadList,
-// avoid adding it to freeList.
-if (buf.getBufferindex() != -1) {
- freeList.push(buf.getBufferindex());
-}
-
-completedReadList.remove(buf);
buf.setTracingContext(null);
if (LOGGER.isTraceEnabled()) {
LOGGER.trace("Evicting buffer idx {}; was used for file {} offset {}
length {}",
buf.getBufferindex(), buf.getStream().getPath(), buf.getOffset(),
buf.getLength());
}
+completedReadList.remove(buf);
Review Comment:
Buffer should be removed from completed list after it has been added to the
free list.
> ABFS ReadBufferManager buffer sharing across concurrent HTTP requests
> -
>
> Key: HADOOP-18521
> URL: https://issues.apache.org/jira/browse/HADOOP-18521
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs/azure
>Affects Versions: 3.3.2, 3.3.3, 3.3.4
>Reporter: Steve Loughran
>Assignee: Steve Loughran
>Priority: Critical
> Labels: pull-request-available
>
> AbfsInputStream.close() can trigger the return of buffers used for active
> prefetch GET requests into the ReadBufferManager free buffer pool.
> A subsequent prefetch by a different stream in the same process may acquire
> this same buffer. This can lead to risk of corruption of its own prefetched
> data, data which may then be returned to that other thread.
> On releases without the fix for this (3.3.2 to 3.3.4), the bug can be avoided
> by disabling all prefetching
> {code}
> fs.azure.readaheadqueue.depth
> {code}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[jira] [Commented] (HADOOP-18521) ABFS ReadBufferManager buffer sharing across concurrent HTTP requests
[
https://issues.apache.org/jira/browse/HADOOP-18521?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17632293#comment-17632293
]
ASF GitHub Bot commented on HADOOP-18521:
-
anmolanmol1234 commented on PR #5117:
URL: https://github.com/apache/hadoop/pull/5117#issuecomment-1311623500
The checkstyle errors needs fixing.
> ABFS ReadBufferManager buffer sharing across concurrent HTTP requests
> -
>
> Key: HADOOP-18521
> URL: https://issues.apache.org/jira/browse/HADOOP-18521
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs/azure
>Affects Versions: 3.3.2, 3.3.3, 3.3.4
>Reporter: Steve Loughran
>Assignee: Steve Loughran
>Priority: Critical
> Labels: pull-request-available
>
> AbfsInputStream.close() can trigger the return of buffers used for active
> prefetch GET requests into the ReadBufferManager free buffer pool.
> A subsequent prefetch by a different stream in the same process may acquire
> this same buffer. This can lead to risk of corruption of its own prefetched
> data, data which may then be returned to that other thread.
> On releases without the fix for this (3.3.2 to 3.3.4), the bug can be avoided
> by disabling all prefetching
> {code}
> fs.azure.readaheadqueue.depth
> {code}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[jira] [Commented] (HADOOP-18521) ABFS ReadBufferManager buffer sharing across concurrent HTTP requests
[
https://issues.apache.org/jira/browse/HADOOP-18521?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17632208#comment-17632208
]
ASF GitHub Bot commented on HADOOP-18521:
-
pranavsaxena-microsoft commented on code in PR #5117:
URL: https://github.com/apache/hadoop/pull/5117#discussion_r1020040801
##
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/ReadBufferManager.java:
##
@@ -454,31 +588,65 @@ ReadBuffer getNextBlockToRead() throws
InterruptedException {
*/
void doneReading(final ReadBuffer buffer, final ReadBufferStatus result,
final int bytesActuallyRead) {
if (LOGGER.isTraceEnabled()) {
- LOGGER.trace("ReadBufferWorker completed read file {} for offset {}
outcome {} bytes {}",
- buffer.getStream().getPath(), buffer.getOffset(), result,
bytesActuallyRead);
-}
-synchronized (this) {
- // If this buffer has already been purged during
- // close of InputStream then we don't update the lists.
- if (inProgressList.contains(buffer)) {
-inProgressList.remove(buffer);
+ LOGGER.trace("ReadBufferWorker completed file {} for offset {} bytes {};
{}",
+ buffer.getStream().getPath(), buffer.getOffset(),
bytesActuallyRead, buffer);
+}
+// decrement counter.
+buffer.prefetchFinished();
+
+try {
+ synchronized (this) {
+// remove from the list
+if (!inProgressList.remove(buffer)) {
+ // this is a sign of inconsistent state, so a major problem
+ String message =
+ String.format("Read completed from an operation not declared as
in progress %s",
+ buffer);
+ LOGGER.warn(message);
+ // release the buffer (which may raise an exception)
+ placeBufferOnFreeList("read not in progress", buffer);
+ // report the failure
+ throw new IllegalStateException(message);
+}
+
+boolean shouldFreeBuffer = false;
+String freeBufferReason = "";
if (result == ReadBufferStatus.AVAILABLE && bytesActuallyRead > 0) {
buffer.setStatus(ReadBufferStatus.AVAILABLE);
buffer.setLength(bytesActuallyRead);
} else {
- freeList.push(buffer.getBufferindex());
- // buffer will be deleted as per the eviction policy.
+ // read failed or there was no data, -the buffer can be returned to
the free list.
+ shouldFreeBuffer = true;
+ freeBufferReason = "failed read";
}
// completed list also contains FAILED read buffers
// for sending exception message to clients.
buffer.setStatus(result);
buffer.setTimeStamp(currentTimeMillis());
-completedReadList.add(buffer);
+if (!buffer.isStreamClosed()) {
+ // completed reads are added to the list.
+ LOGGER.trace("Adding buffer to completed list {}", buffer);
+ completedReadList.add(buffer);
Review Comment:
It would be better if we do placeBufferOnFreeList before trying to add in
completedList, since, we would add it in freeList before completeListAddition
(which can try itself to add in freeList on evict()) -> this will force never
addition to freeList from completedList.
##
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/ReadBufferManager.java:
##
@@ -454,31 +588,65 @@ ReadBuffer getNextBlockToRead() throws
InterruptedException {
*/
void doneReading(final ReadBuffer buffer, final ReadBufferStatus result,
final int bytesActuallyRead) {
if (LOGGER.isTraceEnabled()) {
- LOGGER.trace("ReadBufferWorker completed read file {} for offset {}
outcome {} bytes {}",
- buffer.getStream().getPath(), buffer.getOffset(), result,
bytesActuallyRead);
-}
-synchronized (this) {
- // If this buffer has already been purged during
- // close of InputStream then we don't update the lists.
- if (inProgressList.contains(buffer)) {
-inProgressList.remove(buffer);
+ LOGGER.trace("ReadBufferWorker completed file {} for offset {} bytes {};
{}",
+ buffer.getStream().getPath(), buffer.getOffset(),
bytesActuallyRead, buffer);
+}
+// decrement counter.
+buffer.prefetchFinished();
+
+try {
+ synchronized (this) {
+// remove from the list
+if (!inProgressList.remove(buffer)) {
+ // this is a sign of inconsistent state, so a major problem
+ String message =
+ String.format("Read completed from an operation not declared as
in progress %s",
+ buffer);
+ LOGGER.warn(message);
+ // release the buffer (which may raise an exception)
+ placeBufferOnFreeList("read not in progress", buffer);
+ // report the failure
+ throw new IllegalStateException(message);
+}
[jira] [Commented] (HADOOP-18521) ABFS ReadBufferManager buffer sharing across concurrent HTTP requests
[
https://issues.apache.org/jira/browse/HADOOP-18521?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17632120#comment-17632120
]
ASF GitHub Bot commented on HADOOP-18521:
-
pranavsaxena-microsoft commented on code in PR #5117:
URL: https://github.com/apache/hadoop/pull/5117#discussion_r1019839338
##
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/ReadBufferManager.java:
##
@@ -454,31 +588,65 @@ ReadBuffer getNextBlockToRead() throws
InterruptedException {
*/
void doneReading(final ReadBuffer buffer, final ReadBufferStatus result,
final int bytesActuallyRead) {
if (LOGGER.isTraceEnabled()) {
- LOGGER.trace("ReadBufferWorker completed read file {} for offset {}
outcome {} bytes {}",
- buffer.getStream().getPath(), buffer.getOffset(), result,
bytesActuallyRead);
-}
-synchronized (this) {
- // If this buffer has already been purged during
- // close of InputStream then we don't update the lists.
- if (inProgressList.contains(buffer)) {
-inProgressList.remove(buffer);
+ LOGGER.trace("ReadBufferWorker completed file {} for offset {} bytes {};
{}",
+ buffer.getStream().getPath(), buffer.getOffset(),
bytesActuallyRead, buffer);
+}
+// decrement counter.
+buffer.prefetchFinished();
+
+try {
+ synchronized (this) {
+// remove from the list
+if (!inProgressList.remove(buffer)) {
+ // this is a sign of inconsistent state, so a major problem
+ String message =
+ String.format("Read completed from an operation not declared as
in progress %s",
+ buffer);
+ LOGGER.warn(message);
+ // release the buffer (which may raise an exception)
+ placeBufferOnFreeList("read not in progress", buffer);
+ // report the failure
+ throw new IllegalStateException(message);
+}
+
+boolean shouldFreeBuffer = false;
+String freeBufferReason = "";
if (result == ReadBufferStatus.AVAILABLE && bytesActuallyRead > 0) {
buffer.setStatus(ReadBufferStatus.AVAILABLE);
buffer.setLength(bytesActuallyRead);
} else {
- freeList.push(buffer.getBufferindex());
- // buffer will be deleted as per the eviction policy.
+ // read failed or there was no data, -the buffer can be returned to
the free list.
+ shouldFreeBuffer = true;
+ freeBufferReason = "failed read";
}
// completed list also contains FAILED read buffers
// for sending exception message to clients.
buffer.setStatus(result);
buffer.setTimeStamp(currentTimeMillis());
-completedReadList.add(buffer);
+if (!buffer.isStreamClosed()) {
+ // completed reads are added to the list.
+ LOGGER.trace("Adding buffer to completed list {}", buffer);
+ completedReadList.add(buffer);
Review Comment:
Lets not add buffer in completedList in the cases where we are going to add
in freeList(due to byteRead == 0).
- Got exception:
```
2022-11-10 20:48:22,516 TRACE [ABFS-prefetch-7]: services.ReadBufferManager
(ReadBufferManager.java:doneReading(591)) - ReadBufferWorker completed file
/testfilefb393e327a88 for offset 4194304 bytes 0;
org.apache.hadoop.fs.azurebfs.services.ReadBuffer@6777a743{
status=READING_IN_PROGRESS, offset=4194304, length=0, requestedLength=4194304,
bufferindex=0, timeStamp=0, isFirstByteConsumed=false,
isLastByteConsumed=false, isAnyByteConsumed=false, errException=null,
stream=9d85521627a8, stream closed=false}
2022-11-10 20:48:22,516 TRACE [ABFS-prefetch-7]: services.ReadBufferManager
(ReadBufferManager.java:doneReading(633)) - Adding buffer to completed list
org.apache.hadoop.fs.azurebfs.services.ReadBuffer@6777a743{ status=AVAILABLE,
offset=4194304, length=0, requestedLength=4194304, bufferindex=0,
timeStamp=86052487, isFirstByteConsumed=false, isLastByteConsumed=false,
isAnyByteConsumed=false, errException=null, stream=9d85521627a8, stream
closed=false}
2022-11-10 20:48:22,516 DEBUG [ABFS-prefetch-7]: services.ReadBufferManager
(ReadBufferManager.java:placeBufferOnFreeList(407)) - Returning buffer index 0
to free list for 'failed read'; owner
org.apache.hadoop.fs.azurebfs.services.ReadBuffer@6777a743{ status=AVAILABLE,
offset=4194304, length=0, requestedLength=4194304, bufferindex=0,
timeStamp=86052487, isFirstByteConsumed=false, isLastByteConsumed=false,
isAnyByteConsumed=false, errException=null, stream=9d85521627a8, stream
closed=false}
2022-11-10 20:48:22,517 TRACE [ABFS-prefetch-7]: services.ReadBufferWorker
(ReadBufferWorker.java:run(95)) - Exception received:
org.apache.hadoop.fs.PathIOException: `/testfilefb393e327a88': Input/output
error: Buffer index 0 found in buffer collection completedReadList
[jira] [Commented] (HADOOP-18521) ABFS ReadBufferManager buffer sharing across concurrent HTTP requests
[
https://issues.apache.org/jira/browse/HADOOP-18521?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17631333#comment-17631333
]
ASF GitHub Bot commented on HADOOP-18521:
-
hadoop-yetus commented on PR #5117:
URL: https://github.com/apache/hadoop/pull/5117#issuecomment-1309518862
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|::|--:|:|::|:---:|
| +0 :ok: | reexec | 0m 47s | | 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 3 new or modified test files. |
_ trunk Compile Tests _ |
| +0 :ok: | mvndep | 16m 25s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 26m 8s | | trunk passed |
| +1 :green_heart: | compile | 23m 24s | | trunk passed with JDK
Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | compile | 20m 48s | | trunk passed with JDK
Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | checkstyle | 4m 17s | | trunk passed |
| +1 :green_heart: | mvnsite | 3m 2s | | trunk passed |
| +1 :green_heart: | javadoc | 2m 41s | | trunk passed with JDK
Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javadoc | 2m 17s | | trunk passed with JDK
Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 4m 42s | | trunk passed |
| +1 :green_heart: | shadedclient | 22m 13s | | branch has no errors
when building and testing our client artifacts. |
_ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 28s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 1m 44s | | the patch passed |
| +1 :green_heart: | compile | 22m 41s | | the patch passed with JDK
Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javac | 22m 41s | | the patch passed |
| +1 :green_heart: | compile | 20m 46s | | the patch passed with JDK
Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | javac | 20m 46s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks
issues. |
| -0 :warning: | checkstyle | 3m 57s |
[/results-checkstyle-root.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5117/4/artifact/out/results-checkstyle-root.txt)
| root: The patch generated 36 new + 1 unchanged - 0 fixed = 37 total (was 1)
|
| +1 :green_heart: | mvnsite | 3m 15s | | the patch passed |
| +1 :green_heart: | javadoc | 2m 33s | | the patch passed with JDK
Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javadoc | 2m 17s | | the patch passed with JDK
Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 4m 37s | | the patch passed |
| +1 :green_heart: | shadedclient | 21m 48s | | patch has no errors
when building and testing our client artifacts. |
_ Other Tests _ |
| +1 :green_heart: | unit | 18m 49s | | hadoop-common in the patch
passed. |
| +1 :green_heart: | unit | 2m 45s | | hadoop-azure in the patch
passed. |
| +1 :green_heart: | asflicense | 1m 16s | | The patch does not
generate ASF License warnings. |
| | | 239m 1s | | |
| Subsystem | Report/Notes |
|--:|:-|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base:
https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5117/4/artifact/out/Dockerfile
|
| GITHUB PR | https://github.com/apache/hadoop/pull/5117 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall
mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
| uname | Linux 1cea54f46634 4.15.0-191-generic #202-Ubuntu SMP Thu Aug 4
01:49:29 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / 1ee18eeb4922d18168bd1fc8ec4a5c75610447cc |
| Default Java | Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| Multi-JDK versions |
/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04
/usr/lib/jvm/java-8-openjdk-amd64:Private
Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| Test Results |
https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5117/4/testReport/ |
| Max. pr
[jira] [Commented] (HADOOP-18521) ABFS ReadBufferManager buffer sharing across concurrent HTTP requests
[
https://issues.apache.org/jira/browse/HADOOP-18521?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17630800#comment-17630800
]
ASF GitHub Bot commented on HADOOP-18521:
-
pranavsaxena-microsoft commented on code in PR #5117:
URL: https://github.com/apache/hadoop/pull/5117#discussion_r1017458506
##
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/ReadBufferManager.java:
##
@@ -534,17 +676,31 @@ void callTryEvict() {
tryEvict();
}
-
/**
* Purging the buffers associated with an {@link AbfsInputStream}
* from {@link ReadBufferManager} when stream is closed.
+ * Before HADOOP-18521 this would purge in progress reads, which
+ * would return the active buffer to the free pool while it was
+ * still in use.
* @param stream input stream.
*/
public synchronized void purgeBuffersForStream(AbfsInputStream stream) {
LOGGER.debug("Purging stale buffers for AbfsInputStream {} ", stream);
+
+// remove from the queue
+int before = readAheadQueue.size();
readAheadQueue.removeIf(readBuffer -> readBuffer.getStream() == stream);
-purgeList(stream, completedReadList);
-purgeList(stream, inProgressList);
+int readaheadPurged = readAheadQueue.size() - before;
Review Comment:
By the thread reaches this line, maybe some more blocks would be added in
readAheadQueue, this may bloat the metric. Also, before should >=
readAheadQueue.size() (in case no additional blocks are ahead), this would
result in negative addition.
##
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/ReadBufferManager.java:
##
@@ -454,31 +588,39 @@ ReadBuffer getNextBlockToRead() throws
InterruptedException {
*/
void doneReading(final ReadBuffer buffer, final ReadBufferStatus result,
final int bytesActuallyRead) {
if (LOGGER.isTraceEnabled()) {
- LOGGER.trace("ReadBufferWorker completed read file {} for offset {}
outcome {} bytes {}",
- buffer.getStream().getPath(), buffer.getOffset(), result,
bytesActuallyRead);
-}
-synchronized (this) {
- // If this buffer has already been purged during
- // close of InputStream then we don't update the lists.
- if (inProgressList.contains(buffer)) {
-inProgressList.remove(buffer);
+ LOGGER.trace("ReadBufferWorker completed file {} for offset {} bytes {}",
+ buffer.getStream().getPath(), buffer.getOffset(),
bytesActuallyRead);
+}
+try {
+ synchronized (this) {
+checkState(inProgressList.remove(buffer),
+"Read completed from an operation not declared as in progress %s",
buffer);
+// If this buffer has already been purged during
+// close of InputStream then we don't update the lists.
if (result == ReadBufferStatus.AVAILABLE && bytesActuallyRead > 0) {
buffer.setStatus(ReadBufferStatus.AVAILABLE);
buffer.setLength(bytesActuallyRead);
} else {
- freeList.push(buffer.getBufferindex());
- // buffer will be deleted as per the eviction policy.
+ // there is no data, so it is immediately returned to the free list.
+ placeBufferOnFreeList("failed read", buffer);
Review Comment:
This may result in IllegalStateException propogating to AbfsInputStream.
This line will add the buffer into freeList, from which this index shall be
taken by readBuffer b1.
Now, after sometime, let this buffer from completedList needs to be evicted,
it would come to
https://github.com/steveloughran/hadoop/blob/azure/HADOOP-18521-buffer-manager/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/ReadBufferManager.java#L408,
two things can happen:
1. freeList still has this index: it will throw IllegalStateException
2. freeList doesn't have: it will throw IllegalStateException from
https://github.com/steveloughran/hadoop/blob/azure/HADOOP-18521-buffer-manager/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/ReadBufferManager.java#L411.
##
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/ReadBufferManager.java:
##
@@ -454,31 +588,39 @@ ReadBuffer getNextBlockToRead() throws
InterruptedException {
*/
void doneReading(final ReadBuffer buffer, final ReadBufferStatus result,
final int bytesActuallyRead) {
if (LOGGER.isTraceEnabled()) {
- LOGGER.trace("ReadBufferWorker completed read file {} for offset {}
outcome {} bytes {}",
- buffer.getStream().getPath(), buffer.getOffset(), result,
bytesActuallyRead);
-}
-synchronized (this) {
- // If this buffer has already been purged during
- // close of InputStream then we don't update the lists.
- if (inProgressList.contains(buffer)) {
-inProgressList.remove(buffer);
+ LO
[jira] [Commented] (HADOOP-18521) ABFS ReadBufferManager buffer sharing across concurrent HTTP requests
[
https://issues.apache.org/jira/browse/HADOOP-18521?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17630503#comment-17630503
]
ASF GitHub Bot commented on HADOOP-18521:
-
hadoop-yetus commented on PR #5117:
URL: https://github.com/apache/hadoop/pull/5117#issuecomment-1307460345
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|::|--:|:|::|:---:|
| +0 :ok: | reexec | 0m 50s | | 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 3 new or modified test files. |
_ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 41m 44s | | trunk passed |
| +1 :green_heart: | compile | 0m 48s | | trunk passed with JDK
Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | compile | 0m 43s | | trunk passed with JDK
Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | checkstyle | 0m 48s | | trunk passed |
| +1 :green_heart: | mvnsite | 1m 3s | | trunk passed |
| +1 :green_heart: | javadoc | 0m 48s | | trunk passed with JDK
Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javadoc | 0m 38s | | trunk passed with JDK
Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 1m 22s | | trunk passed |
| +1 :green_heart: | shadedclient | 23m 54s | | branch has no errors
when building and testing our client artifacts. |
_ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 0m 34s | | the patch passed |
| +1 :green_heart: | compile | 0m 37s | | the patch passed with JDK
Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javac | 0m 37s | | the patch passed |
| +1 :green_heart: | compile | 0m 31s | | the patch passed with JDK
Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | javac | 0m 31s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks
issues. |
| -0 :warning: | checkstyle | 0m 22s |
[/results-checkstyle-hadoop-tools_hadoop-azure.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5117/3/artifact/out/results-checkstyle-hadoop-tools_hadoop-azure.txt)
| hadoop-tools/hadoop-azure: The patch generated 22 new + 1 unchanged - 0
fixed = 23 total (was 1) |
| +1 :green_heart: | mvnsite | 0m 35s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 28s | | the patch passed with JDK
Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javadoc | 0m 25s | | the patch passed with JDK
Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 1m 9s | | the patch passed |
| +1 :green_heart: | shadedclient | 23m 15s | | patch has no errors
when building and testing our client artifacts. |
_ Other Tests _ |
| +1 :green_heart: | unit | 2m 7s | | hadoop-azure in the patch
passed. |
| +1 :green_heart: | asflicense | 0m 40s | | The patch does not
generate ASF License warnings. |
| | | 104m 34s | | |
| Subsystem | Report/Notes |
|--:|:-|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base:
https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5117/3/artifact/out/Dockerfile
|
| GITHUB PR | https://github.com/apache/hadoop/pull/5117 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall
mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
| uname | Linux 1cf2fc9c924a 4.15.0-191-generic #202-Ubuntu SMP Thu Aug 4
01:49:29 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / c295fb241de7c160a1cba5b955f29f2f30376520 |
| Default Java | Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| Multi-JDK versions |
/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04
/usr/lib/jvm/java-8-openjdk-amd64:Private
Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| Test Results |
https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5117/3/testReport/ |
| Max. process+thread count | 606 (vs. ulimit of 5500) |
| modules | C: hadoop-tools/hadoop-azure U: hadoop-tools/hadoop-azure |
| Console output |
https://ci-hadoop.apache.org/job/hadoop
[jira] [Commented] (HADOOP-18521) ABFS ReadBufferManager buffer sharing across concurrent HTTP requests
[
https://issues.apache.org/jira/browse/HADOOP-18521?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17630413#comment-17630413
]
ASF GitHub Bot commented on HADOOP-18521:
-
steveloughran commented on PR #5117:
URL: https://github.com/apache/hadoop/pull/5117#issuecomment-1307225146
going to have to rebase this.
i also intend to have prefetch threads update the input streams with
* gauge of active prefetches
* amount of data/duration
measuring how much data was discarded for a stream would be interesting too,
but a bit fiddlier
reader thread jsut gets iostats off the stream, casts to IOStatisticsStore
then uses it as a duration tracker and gauge stats source; straightforward
> ABFS ReadBufferManager buffer sharing across concurrent HTTP requests
> -
>
> Key: HADOOP-18521
> URL: https://issues.apache.org/jira/browse/HADOOP-18521
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs/azure
>Affects Versions: 3.3.2, 3.3.3, 3.3.4
>Reporter: Steve Loughran
>Assignee: Steve Loughran
>Priority: Critical
> Labels: pull-request-available
>
> AbfsInputStream.close() can trigger the return of buffers used for active
> prefetch GET requests into the ReadBufferManager free buffer pool.
> A subsequent prefetch by a different stream in the same process may acquire
> this same buffer. This can lead to risk of corruption of its own prefetched
> data, data which may then be returned to that other thread.
> On releases without the fix for this (3.3.2 to 3.3.4), the bug can be avoided
> by disabling all prefetching
> {code}
> fs.azure.readaheadqueue.depth
> {code}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[jira] [Commented] (HADOOP-18521) ABFS ReadBufferManager buffer sharing across concurrent HTTP requests
[
https://issues.apache.org/jira/browse/HADOOP-18521?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17629990#comment-17629990
]
ASF GitHub Bot commented on HADOOP-18521:
-
hadoop-yetus commented on PR #5117:
URL: https://github.com/apache/hadoop/pull/5117#issuecomment-1306093551
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|::|--:|:|::|:---:|
| +0 :ok: | reexec | 0m 52s | | 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 4 new or modified test files. |
_ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 42m 0s | | trunk passed |
| +1 :green_heart: | compile | 0m 48s | | trunk passed with JDK
Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | compile | 0m 41s | | trunk passed with JDK
Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | checkstyle | 0m 40s | | trunk passed |
| +1 :green_heart: | mvnsite | 0m 47s | | trunk passed |
| +1 :green_heart: | javadoc | 0m 46s | | trunk passed with JDK
Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javadoc | 0m 37s | | trunk passed with JDK
Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 1m 25s | | trunk passed |
| +1 :green_heart: | shadedclient | 24m 2s | | branch has no errors
when building and testing our client artifacts. |
_ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 0m 34s | | the patch passed |
| +1 :green_heart: | compile | 0m 37s | | the patch passed with JDK
Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javac | 0m 37s | | the patch passed |
| +1 :green_heart: | compile | 0m 31s | | the patch passed with JDK
Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | javac | 0m 31s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks
issues. |
| -0 :warning: | checkstyle | 0m 21s |
[/results-checkstyle-hadoop-tools_hadoop-azure.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5117/2/artifact/out/results-checkstyle-hadoop-tools_hadoop-azure.txt)
| hadoop-tools/hadoop-azure: The patch generated 22 new + 3 unchanged - 0
fixed = 25 total (was 3) |
| +1 :green_heart: | mvnsite | 0m 34s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 26s | | the patch passed with JDK
Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javadoc | 0m 25s | | the patch passed with JDK
Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 1m 11s | | the patch passed |
| +1 :green_heart: | shadedclient | 23m 32s | | patch has no errors
when building and testing our client artifacts. |
_ Other Tests _ |
| +1 :green_heart: | unit | 2m 7s | | hadoop-azure in the patch
passed. |
| +1 :green_heart: | asflicense | 0m 38s | | The patch does not
generate ASF License warnings. |
| | | 104m 39s | | |
| Subsystem | Report/Notes |
|--:|:-|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base:
https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5117/2/artifact/out/Dockerfile
|
| GITHUB PR | https://github.com/apache/hadoop/pull/5117 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall
mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
| uname | Linux d1321bc3e889 4.15.0-191-generic #202-Ubuntu SMP Thu Aug 4
01:49:29 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / ddd1a32528a819c8f05eb87624148eb24b324931 |
| Default Java | Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| Multi-JDK versions |
/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04
/usr/lib/jvm/java-8-openjdk-amd64:Private
Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| Test Results |
https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5117/2/testReport/ |
| Max. process+thread count | 594 (vs. ulimit of 5500) |
| modules | C: hadoop-tools/hadoop-azure U: hadoop-tools/hadoop-azure |
| Console output |
https://ci-hadoop.apache.org/job/hadoop
[jira] [Commented] (HADOOP-18521) ABFS ReadBufferManager buffer sharing across concurrent HTTP requests
[
https://issues.apache.org/jira/browse/HADOOP-18521?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17629975#comment-17629975
]
ASF GitHub Bot commented on HADOOP-18521:
-
hadoop-yetus commented on PR #5117:
URL: https://github.com/apache/hadoop/pull/5117#issuecomment-1306078980
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|::|--:|:|::|:---:|
| +0 :ok: | reexec | 2m 50s | | 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 4 new or modified test files. |
_ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 39m 34s | | trunk passed |
| +1 :green_heart: | compile | 0m 54s | | trunk passed with JDK
Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | compile | 0m 51s | | trunk passed with JDK
Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | checkstyle | 0m 43s | | trunk passed |
| +1 :green_heart: | mvnsite | 0m 53s | | trunk passed |
| +1 :green_heart: | javadoc | 0m 53s | | trunk passed with JDK
Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javadoc | 0m 46s | | trunk passed with JDK
Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 1m 23s | | trunk passed |
| +1 :green_heart: | shadedclient | 20m 45s | | branch has no errors
when building and testing our client artifacts. |
_ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 0m 38s | | the patch passed |
| +1 :green_heart: | compile | 0m 38s | | the patch passed with JDK
Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javac | 0m 38s | | the patch passed |
| +1 :green_heart: | compile | 0m 34s | | the patch passed with JDK
Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | javac | 0m 34s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks
issues. |
| -0 :warning: | checkstyle | 0m 25s |
[/results-checkstyle-hadoop-tools_hadoop-azure.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5117/1/artifact/out/results-checkstyle-hadoop-tools_hadoop-azure.txt)
| hadoop-tools/hadoop-azure: The patch generated 22 new + 3 unchanged - 0
fixed = 25 total (was 3) |
| +1 :green_heart: | mvnsite | 0m 37s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 27s | | the patch passed with JDK
Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javadoc | 0m 28s | | the patch passed with JDK
Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 1m 10s | | the patch passed |
| +1 :green_heart: | shadedclient | 20m 39s | | patch has no errors
when building and testing our client artifacts. |
_ Other Tests _ |
| +1 :green_heart: | unit | 2m 15s | | hadoop-azure in the patch
passed. |
| +1 :green_heart: | asflicense | 0m 43s | | The patch does not
generate ASF License warnings. |
| | | 100m 14s | | |
| Subsystem | Report/Notes |
|--:|:-|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base:
https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5117/1/artifact/out/Dockerfile
|
| GITHUB PR | https://github.com/apache/hadoop/pull/5117 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall
mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
| uname | Linux f5eb08c3f643 4.15.0-191-generic #202-Ubuntu SMP Thu Aug 4
01:49:29 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / 86a8177aedb9f311384bd28038ffdc464c188168 |
| Default Java | Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| Multi-JDK versions |
/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04
/usr/lib/jvm/java-8-openjdk-amd64:Private
Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| Test Results |
https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5117/1/testReport/ |
| Max. process+thread count | 699 (vs. ulimit of 5500) |
| modules | C: hadoop-tools/hadoop-azure U: hadoop-tools/hadoop-azure |
| Console output |
https://ci-hadoop.apache.org/job/hadoop
[jira] [Commented] (HADOOP-18521) ABFS ReadBufferManager buffer sharing across concurrent HTTP requests
[
https://issues.apache.org/jira/browse/HADOOP-18521?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17629937#comment-17629937
]
ASF GitHub Bot commented on HADOOP-18521:
-
steveloughran opened a new pull request, #5117:
URL: https://github.com/apache/hadoop/pull/5117
Addresses the issue by not trying to cancel in-progress reads when a stream
is closed()...they are allowed to continue and then their data discarded.
To enable discarding, AbfsInputStreams export their `closed` state in
which is now AtomicBool internally so reader threads can probe it.
The shared buffers now have owner tracking, which will reject
* attempts to acquire an owned buffer
* attempts to return a buffer not owned
Plus
* Lots of other invariants added to validate the state
* useful to string values
Also adds path and stream capability probe for the fix;
cloudstore "pathcapability" probe can report this.
Hadoop 3.3.2 added the path capability
"fs.capability.paths.acls", so two probes can
determine if abfs is exposed:
not vulnerable
!hasPathCability("fs.capability.paths.acls")
|| hasPathCability("fs.azure.capability.prefetch.safe")
vulnerable
hasPathCability("fs.capability.paths.acls")
&& !hasPathCability("fs.azure.capability.prefetch.safe")
It can also be demanded in an openFile() call.
That block the code ever working on a version without
the race condition. Possibly a bit excessive.
### How was this patch tested?
needs more tests with multi GB csv files to validate the patch.
Unable to come up with good tests to recreate the failure condition.
### For code changes:
- [X] Does the title or this PR starts with the corresponding JIRA issue id
(e.g. 'HADOOP-17799. Your PR title ...')?
- [X] 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](http://www.apache.org/legal/resolved.html#category-a)?
- [ ] If applicable, have you updated the `LICENSE`, `LICENSE-binary`,
`NOTICE-binary` files?
> ABFS ReadBufferManager buffer sharing across concurrent HTTP requests
> -
>
> Key: HADOOP-18521
> URL: https://issues.apache.org/jira/browse/HADOOP-18521
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs/azure
>Affects Versions: 3.3.2, 3.3.3, 3.3.4
>Reporter: Steve Loughran
>Assignee: Steve Loughran
>Priority: Critical
>
> AbfsInputStream.close() can trigger the return of buffers used for active
> prefetch GET requests into the ReadBufferManager free buffer pool.
> A subsequent prefetch by a different stream in the same process may acquire
> this same buffer. This can lead to risk of corruption of its own prefetched
> data, data which may then be returned to that other thread.
> On releases without the fix for this (3.3.2 to 3.3.4), the bug can be avoided
> by disabling all prefetching
> {code}
> fs.azure.readaheadqueue.depth
> {code}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[jira] [Commented] (HADOOP-18521) ABFS ReadBufferManager buffer sharing across concurrent HTTP requests
[
https://issues.apache.org/jira/browse/HADOOP-18521?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17629339#comment-17629339
]
Steve Loughran commented on HADOOP-18521:
-
This bug can be fixed by deleting one line from
{{ReadBufferManager.purgeBuffersForStream()}}.
I am not going to provide a test for this as you need a multi GB CSV file and a
build of spark configured to use your hadoop dist.
The latest build of cloudstore (https://github.com/steveloughran/cloudstore)
has a command {{mkcsv}} which can create the file; the man page includes the
spark binding info:
https://github.com/steveloughran/cloudstore/blob/trunk/src/main/site/mkcsv.md
along with the fix, i am going to include a stream capability which the fs and
stream can be probed for to declare that the fix is in. this allows for
programmatic verification of the safety of releases, including with the
cloudstore pathcapabilities command
> ABFS ReadBufferManager buffer sharing across concurrent HTTP requests
> -
>
> Key: HADOOP-18521
> URL: https://issues.apache.org/jira/browse/HADOOP-18521
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs/azure
>Affects Versions: 3.3.2, 3.3.3, 3.3.4
>Reporter: Steve Loughran
>Assignee: Steve Loughran
>Priority: Critical
>
> AbfsInputStream.close() can trigger the return of buffers used for active
> prefetch GET requests into the ReadBufferManager free buffer pool.
> A subsequent prefetch by a different stream in the same process may acquire
> this same buffer. This can lead to risk of corruption of its own prefetched
> data, data which may then be returned to that other thread.
> On releases without the fix for this (3.3.2 to 3.3.4), the bug can be avoided
> by disabling all prefetching
> {code}
> fs.azure.readaheadqueue.depth
> {code}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
