Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backport HADOOP-18546. ABFS: disable purging list of in progress reads. #5528

Closed
wants to merge 5 commits into from

Conversation

anujmodi2021
Copy link
Contributor

Description of PR

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.
Parent JIRA: https://issues.apache.org/jira/browse/HADOOP-18521
In this PR, we are disabling the purging of the inprogressList. The readBuffers in InProgressList will get to ReadBufferWorker and get processed and finally get into completedList. After a thresholdAgeMilliseconds, the readBuffer would be evicted

steveloughran and others added 5 commits April 3, 2023 00:59

This is a followup to the original HADOOP-18546
patch; cherry-picks of that should include this
or follow up with it.

Removes risk of race conditions in assertions
of ITestReadBufferManager on the state of the in-progress
and completed queues by removing assertions brittle
to race conditions in scheduling/network IO

* Waits for all the executor pool shutdown to complete before
  making any assertions
* Assertions that there are no in progress reads MUST be
  cut as there may be some and they won't be cancelled.
* Assertions that the completed list is without buffers
  of a closed stream are brittle because if there was
  an in progress stream which completed after stream.close()
  then it will end up in the list.

Contributed by Steve Loughran
Followup patch to  HADOOP-18456 as part of HADOOP-18521,
ABFS ReadBufferManager buffer sharing across concurrent HTTP requests

Add probes of readahead fix aid in checking safety of
hadoop ABFS client across different releases.

* ReadBufferManager constructor logs the fact it is safe at TRACE
* AbfsInputStream declares it is fixed in toString()
  by including fs.azure.capability.readahead.safe" in the
  result.

The ABFS FileSystem hasPathCapability("fs.azure.capability.readahead.safe")
probe returns true to indicate the client's readahead manager has been fixed
to be safe when prefetching.

All Hadoop releases for which probe this returns false
and for which the probe "fs.capability.etags.available"
returns true at risk of returning invalid data when reading
ADLS Gen2/Azure storage data.

Contributed by Steve Loughran.
@anujmodi2021 anujmodi2021 deleted the branch-3.3.2 branch April 3, 2023 12:10
@hadoop-yetus
Copy link

💔 -1 overall

Vote Subsystem Runtime Logfile Comment
+0 🆗 reexec 10m 2s Docker mode activated.
_ Prechecks _
+1 💚 dupname 0m 0s No case conflicting files found.
+0 🆗 codespell 0m 1s codespell was not available.
+1 💚 @author 0m 0s The patch does not contain any @author tags.
+1 💚 test4tests 0m 0s The patch appears to include 5 new or modified test files.
_ branch-3.3.2 Compile Tests _
+0 🆗 mvndep 13m 8s Maven dependency ordering for branch
+1 💚 mvninstall 26m 4s branch-3.3.2 passed
+1 💚 compile 18m 0s branch-3.3.2 passed
+1 💚 checkstyle 2m 57s branch-3.3.2 passed
+1 💚 mvnsite 2m 13s branch-3.3.2 passed
+1 💚 javadoc 2m 15s branch-3.3.2 passed
+1 💚 spotbugs 3m 47s branch-3.3.2 passed
+1 💚 shadedclient 25m 27s branch has no errors when building and testing our client artifacts.
_ Patch Compile Tests _
+0 🆗 mvndep 0m 22s Maven dependency ordering for patch
-1 ❌ mvninstall 0m 25s /patch-mvninstall-hadoop-tools_hadoop-azure.txt hadoop-azure in the patch failed.
-1 ❌ compile 16m 44s /patch-compile-root.txt root in the patch failed.
-1 ❌ javac 16m 44s /patch-compile-root.txt root in the patch failed.
+1 💚 blanks 0m 0s The patch has no blanks issues.
-0 ⚠️ checkstyle 2m 49s /results-checkstyle-root.txt root: The patch generated 1 new + 4 unchanged - 0 fixed = 5 total (was 4)
-1 ❌ mvnsite 0m 41s /patch-mvnsite-hadoop-tools_hadoop-azure.txt hadoop-azure in the patch failed.
+1 💚 xml 0m 1s The patch has no ill-formed XML file.
-1 ❌ javadoc 0m 34s /patch-javadoc-hadoop-tools_hadoop-azure.txt hadoop-azure in the patch failed.
-1 ❌ spotbugs 0m 40s /patch-spotbugs-hadoop-tools_hadoop-azure.txt hadoop-azure in the patch failed.
+1 💚 shadedclient 26m 45s patch has no errors when building and testing our client artifacts.
_ Other Tests _
+1 💚 unit 16m 44s hadoop-common in the patch passed.
-1 ❌ unit 0m 39s /patch-unit-hadoop-tools_hadoop-azure.txt hadoop-azure in the patch failed.
+1 💚 asflicense 0m 50s The patch does not generate ASF License warnings.
179m 0s
Subsystem Report/Notes
Docker ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5528/1/artifact/out/Dockerfile
GITHUB PR #5528
Optional Tests dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient codespell xml spotbugs checkstyle
uname Linux 9fa81833ed3d 4.15.0-206-generic #217-Ubuntu SMP Fri Feb 3 19:10:13 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality dev-support/bin/hadoop.sh
git revision branch-3.3.2 / f9681a7
Default Java Private Build-1.8.0_362-8u362-ga-0ubuntu1~18.04.1-b09
Test Results https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5528/1/testReport/
Max. process+thread count 1962 (vs. ulimit of 5500)
modules C: hadoop-common-project/hadoop-common hadoop-tools/hadoop-azure U: .
Console output https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5528/1/console
versions git=2.17.1 maven=3.6.0 spotbugs=4.2.2
Powered by Apache Yetus 0.14.0-SNAPSHOT https://yetus.apache.org

This message was automatically generated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants