HADOOP-18455. s3a prefetching Executor should be closed #4879

virajjasani · 2022-09-12T04:30:34Z

No description provided.

… submitting work (ADDENDUM)

virajjasani · 2022-09-12T04:34:03Z

Tested against us-west-2:

$ mvn -Dparallel-tests -DtestsThreadCount=8 -Dscale clean verify

[INFO] --- maven-surefire-plugin:3.0.0-M1:test (default-test) @ hadoop-aws ---
[INFO] 
[INFO] -------------------------------------------------------
[INFO]  T E S T S
[INFO] -------------------------------------------------------

[INFO] Results:
[INFO] 
[WARNING] Tests run: 436, Failures: 0, Errors: 0, Skipped: 4


[INFO] --- maven-failsafe-plugin:3.0.0-M1:integration-test (default-integration-test) @ hadoop-aws ---
[INFO] 
[INFO] -------------------------------------------------------
[INFO]  T E S T S
[INFO] -------------------------------------------------------

[INFO] Results:
[INFO] 
[WARNING] Tests run: 1150, Failures: 0, Errors: 0, Skipped: 146


[INFO] --- maven-failsafe-plugin:3.0.0-M1:integration-test (sequential-integration-tests) @ hadoop-aws ---
[INFO] 
[INFO] -------------------------------------------------------
[INFO]  T E S T S
[INFO] -------------------------------------------------------

[INFO] Results:
[INFO] 
[ERROR] Errors: 
[ERROR]   ITestS3AContractRootDir>AbstractContractRootDirectoryTest.testRecursiveRootListing:267 » TestTimedOut
[INFO] 
[ERROR] Tests run: 124, Failures: 0, Errors: 1, Skipped: 10

ITestS3AContractRootDir tests are successful when run individually.

virajjasani · 2022-09-12T04:34:35Z

@steveloughran @mehakmeet could you please review this addendum patch?

hadoop-yetus · 2022-09-12T08:47:23Z

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 57s		Docker mode activated.
			_ Prechecks _
+1 💚	dupname	0m 0s		No case conflicting files found.
+0 🆗	codespell	0m 0s		codespell was not available.
+0 🆗	detsecrets	0m 0s		detect-secrets was not available.
+1 💚	@author	0m 0s		The patch does not contain any @author tags.
-1 ❌	test4tests	0m 0s		The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
			_ trunk Compile Tests _
+0 🆗	mvndep	14m 35s		Maven dependency ordering for branch
+1 💚	mvninstall	29m 39s		trunk passed
+1 💚	compile	27m 18s		trunk passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04
+1 💚	compile	23m 12s		trunk passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
+1 💚	checkstyle	4m 34s		trunk passed
+1 💚	mvnsite	3m 10s		trunk passed
+1 💚	javadoc	2m 9s		trunk passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04
+1 💚	javadoc	1m 50s		trunk passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
+1 💚	spotbugs	4m 23s		trunk passed
+1 💚	shadedclient	24m 27s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+0 🆗	mvndep	0m 25s		Maven dependency ordering for patch
+1 💚	mvninstall	1m 42s		the patch passed
+1 💚	compile	26m 18s		the patch passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04
+1 💚	javac	26m 18s		the patch passed
+1 💚	compile	23m 10s		the patch passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
+1 💚	javac	23m 10s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
+1 💚	checkstyle	4m 23s		the patch passed
+1 💚	mvnsite	2m 59s		the patch passed
+1 💚	javadoc	2m 0s		the patch passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04
+1 💚	javadoc	1m 51s		the patch passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
-1 ❌	spotbugs	1m 43s	/new-spotbugs-hadoop-tools_hadoop-aws.html	hadoop-tools/hadoop-aws generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0)
+1 💚	shadedclient	24m 58s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
+1 💚	unit	18m 39s		hadoop-common in the patch passed.
+1 💚	unit	3m 1s		hadoop-aws in the patch passed.
+1 💚	asflicense	1m 10s		The patch does not generate ASF License warnings.
		255m 31s

Reason	Tests
SpotBugs	module:hadoop-tools/hadoop-aws
	Inconsistent synchronization of org.apache.hadoop.fs.s3a.S3AFileSystem.futurePool; locked 50% of time Unsynchronized access at S3AFileSystem.java:50% of time Unsynchronized access at S3AFileSystem.java:[line 788]

Subsystem	Report/Notes
Docker	ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4879/1/artifact/out/Dockerfile
GITHUB PR	#4879
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets
uname	Linux 4d522fd2f660 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 / `87a9788`
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-4879/1/testReport/
Max. process+thread count	3144 (vs. ulimit of 5500)
modules	C: hadoop-common-project/hadoop-common hadoop-tools/hadoop-aws U: .
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4879/1/console
versions	git=2.25.1 maven=3.6.3 spotbugs=4.2.2
Powered by	Apache Yetus 0.14.0 https://yetus.apache.org

This message was automatically generated.

virajjasani · 2022-09-13T22:03:16Z

@mukund-thakur could you please also review?

hadoop-yetus · 2022-09-14T01:38:24Z

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 56s		Docker mode activated.
			_ Prechecks _
+1 💚	dupname	0m 0s		No case conflicting files found.
+0 🆗	codespell	0m 1s		codespell was not available.
+0 🆗	detsecrets	0m 1s		detect-secrets was not available.
+1 💚	@author	0m 0s		The patch does not contain any @author tags.
-1 ❌	test4tests	0m 0s		The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
			_ trunk Compile Tests _
+0 🆗	mvndep	14m 41s		Maven dependency ordering for branch
+1 💚	mvninstall	28m 39s		trunk passed
+1 💚	compile	25m 19s		trunk passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04
+1 💚	compile	22m 10s		trunk passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
+1 💚	checkstyle	4m 29s		trunk passed
+1 💚	mvnsite	3m 0s		trunk passed
+1 💚	javadoc	2m 12s		trunk passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04
+1 💚	javadoc	1m 53s		trunk passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
+1 💚	spotbugs	4m 24s		trunk passed
+1 💚	shadedclient	24m 45s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+0 🆗	mvndep	0m 26s		Maven dependency ordering for patch
+1 💚	mvninstall	1m 40s		the patch passed
+1 💚	compile	25m 38s		the patch passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04
+1 💚	javac	25m 38s		the patch passed
+1 💚	compile	23m 45s		the patch passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
+1 💚	javac	23m 45s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
+1 💚	checkstyle	5m 2s		the patch passed
+1 💚	mvnsite	3m 18s		the patch passed
+1 💚	javadoc	2m 14s		the patch passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04
+1 💚	javadoc	2m 12s		the patch passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
-1 ❌	spotbugs	1m 52s	/new-spotbugs-hadoop-tools_hadoop-aws.html	hadoop-tools/hadoop-aws generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0)
+1 💚	shadedclient	26m 30s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
+1 💚	unit	20m 16s		hadoop-common in the patch passed.
+1 💚	unit	3m 51s		hadoop-aws in the patch passed.
+1 💚	asflicense	1m 30s		The patch does not generate ASF License warnings.
		258m 16s

Reason	Tests
SpotBugs	module:hadoop-tools/hadoop-aws
	Inconsistent synchronization of org.apache.hadoop.fs.s3a.S3AFileSystem.futurePool; locked 60% of time Unsynchronized access at S3AFileSystem.java:60% of time Unsynchronized access at S3AFileSystem.java:[line 782]

Subsystem	Report/Notes
Docker	ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4879/2/artifact/out/Dockerfile
GITHUB PR	#4879
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets
uname	Linux 3bc1ef1a9a76 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 / `a92de6f`
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-4879/2/testReport/
Max. process+thread count	2766 (vs. ulimit of 5500)
modules	C: hadoop-common-project/hadoop-common hadoop-tools/hadoop-aws U: .
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4879/2/console
versions	git=2.25.1 maven=3.6.3 spotbugs=4.2.2
Powered by	Apache Yetus 0.14.0 https://yetus.apache.org

This message was automatically generated.

mehakmeet

LGTM pending yetus errors, Also I think it's better to create this as a separate Hadoop Jira ticket, just to keep track of the changes and explain what the changes are. We can always add the previous Hadoop Jira as the follow-up in the commit message.

mukund-thakur

I have never seen reopening jira's in Hadoop unless it is a critical BUG and that too we always fix via a new JIRA.
I see the issue here is not shutting down the thread pool. Could you please create a new jira and update it this same PR. you can add "follow on <original_jira_id> " in the commit message.

...adoop-common/src/main/java/org/apache/hadoop/fs/impl/prefetch/ExecutorServiceFuturePool.java

mukund-thakur · 2022-09-16T19:53:29Z

I have never seen reopening jira's in Hadoop unless it is a critical BUG and that too we always fix via a new JIRA. I see the issue here is not shutting down the thread pool. Could you please create a new jira and update it this same PR. you can add "follow on <original_jira_id> " in the commit message.

Nice @mehakmeet already commented the same above.

virajjasani · 2022-09-16T19:55:53Z

@mehakmeet @mukund-thakur I will create a new follow-up Jira, thanks

virajjasani · 2022-09-16T20:02:03Z

Updated the PR title with new Jira.

virajjasani · 2022-09-17T20:21:56Z

Inconsistent synchronization of org.apache.hadoop.fs.s3a.S3AFileSystem.futurePool; locked 60% of time Unsynchronized access at S3AFileSystem.java:60% of time Unsynchronized access at S3AFileSystem.java:[line 782]

This would be the case for other thread pools also that are shutdown as part of the filesystem close.

mukund-thakur

LGTM +1
I am wondering how can we ignore this yetus failure and let it run successfully for build and checkstyle.
I agree that spotbug is not an issue here. It is just a false alarm I guess. Will work with @steveloughran to merge this.

steveloughran · 2022-09-20T10:04:07Z

edit the spotbugs file to tell it to STFU

…unk1

hadoop-yetus · 2022-09-21T00:11:59Z

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	1m 6s		Docker mode activated.
			_ Prechecks _
+1 💚	dupname	0m 0s		No case conflicting files found.
+0 🆗	codespell	0m 0s		codespell was not available.
+0 🆗	detsecrets	0m 0s		detect-secrets was not available.
+0 🆗	xmllint	0m 0s		xmllint was not available.
+1 💚	@author	0m 0s		The patch does not contain any @author tags.
-1 ❌	test4tests	0m 0s		The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
			_ trunk Compile Tests _
+0 🆗	mvndep	15m 13s		Maven dependency ordering for branch
+1 💚	mvninstall	32m 10s		trunk passed
+1 💚	compile	29m 39s		trunk passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04
+1 💚	compile	25m 8s		trunk passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
+1 💚	checkstyle	4m 50s		trunk passed
+1 💚	mvnsite	3m 20s		trunk passed
+1 💚	javadoc	2m 19s		trunk passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04
+1 💚	javadoc	2m 9s		trunk passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
+1 💚	spotbugs	5m 4s		trunk passed
+1 💚	shadedclient	27m 32s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+0 🆗	mvndep	0m 30s		Maven dependency ordering for patch
+1 💚	mvninstall	1m 59s		the patch passed
+1 💚	compile	28m 8s		the patch passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04
+1 💚	javac	28m 8s		the patch passed
+1 💚	compile	26m 1s		the patch passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
+1 💚	javac	26m 1s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
+1 💚	checkstyle	4m 50s		the patch passed
+1 💚	mvnsite	3m 41s		the patch passed
+1 💚	javadoc	2m 45s		the patch passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04
+1 💚	javadoc	2m 23s		the patch passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
+1 💚	spotbugs	5m 26s		the patch passed
+1 💚	shadedclient	29m 32s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
+1 💚	unit	20m 27s		hadoop-common in the patch passed.
+1 💚	unit	3m 44s		hadoop-aws in the patch passed.
+1 💚	asflicense	1m 19s		The patch does not generate ASF License warnings.
		283m 53s

Subsystem	Report/Notes
Docker	ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4879/3/artifact/out/Dockerfile
GITHUB PR	#4879
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets xmllint
uname	Linux df2cd3e8f38f 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 / `9123a61`
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-4879/3/testReport/
Max. process+thread count	1263 (vs. ulimit of 5500)
modules	C: hadoop-common-project/hadoop-common hadoop-tools/hadoop-aws U: .
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4879/3/console
versions	git=2.25.1 maven=3.6.3 spotbugs=4.2.2
Powered by	Apache Yetus 0.14.0 https://yetus.apache.org

This message was automatically generated.

mukund-thakur

LGTM +1,
Yetus failure because of no new tests, so ignoring that.

steveloughran · 2022-09-22T13:05:41Z

that spotbugs change is too broad, now it will ignore all inconsistent uses in the class. Can you do a followup which restricts it only to those methods which have problems. otherwise i'll be reverting this...

virajjasani · 2022-09-22T18:08:35Z

I tried applying to stopAllServices but it didn't work. Let me check if this can still be applied to some other granular level.

virajjasani · 2022-09-22T18:10:20Z

Moreover, we also have the same suppress rule applied to S3AInputStream as well:

  <Match>
    <Class name="org.apache.hadoop.fs.s3a.S3AInputStream"/>
    <Bug pattern="IS2_INCONSISTENT_SYNC"/>
  </Match>

virajjasani · 2022-09-22T19:20:14Z

We can still suppress it at field level, if not at method level. Will create PR for HADOOP-18466 soon.

virajjasani · 2022-09-25T19:17:10Z

@steveloughran @mukund-thakur @mehakmeet the spotbugs follow up PR is ready for review: #4926

follow-on patch to HADOOP-18186. Contributed by: Viraj Jasani

HADOOP-18186. s3a prefetching to use SemaphoredDelegatingExecutor for…

87a9788

… submitting work (ADDENDUM)

addendum

a92de6f

mehakmeet self-requested a review September 14, 2022 01:40

mehakmeet reviewed Sep 16, 2022

View reviewed changes

mukund-thakur requested changes Sep 16, 2022

View reviewed changes

...adoop-common/src/main/java/org/apache/hadoop/fs/impl/prefetch/ExecutorServiceFuturePool.java Show resolved Hide resolved

virajjasani changed the title ~~HADOOP-18186. s3a prefetching to use SemaphoredDelegatingExecutor for submitting work (ADDENDUM)~~ HADOOP-18455. s3a prefetching Executor should be closed Sep 16, 2022

virajjasani requested review from mehakmeet and mukund-thakur and removed request for mehakmeet and mukund-thakur September 19, 2022 18:03

mukund-thakur reviewed Sep 19, 2022

View reviewed changes

virajjasani added 2 commits September 20, 2022 11:55

Merge branch 'trunk' of github.com:apache/hadoop into HADOOP-18186-tr…

2ece699

…unk1

update spotbug file to exclude IS2_INCONSISTENT_SYNC

9123a61

mukund-thakur approved these changes Sep 21, 2022

View reviewed changes

mukund-thakur merged commit 084b68e into apache:trunk Sep 21, 2022

HarshitGupta11 pushed a commit to HarshitGupta11/hadoop that referenced this pull request Nov 28, 2022

HADOOP-18455. S3A prefetching executor should be closed (apache#4879)

4c92cc2

follow-on patch to HADOOP-18186. Contributed by: Viraj Jasani

ahmarsuhail mentioned this pull request Apr 17, 2023

HADOOP-18028. High performance S3A input stream (#4752) #5559

Closed

4 tasks

asfgit pushed a commit that referenced this pull request Apr 24, 2023

HADOOP-18455. S3A prefetching executor should be closed (#4879)

5569b41

follow-on patch to HADOOP-18186. Contributed by: Viraj Jasani

asfgit pushed a commit that referenced this pull request Apr 25, 2023

HADOOP-18455. S3A prefetching executor should be closed (#4879)

c395076

follow-on patch to HADOOP-18186. Contributed by: Viraj Jasani

asfgit pushed a commit that referenced this pull request Apr 26, 2023

HADOOP-18455. S3A prefetching executor should be closed (#4879)

267c7f9

follow-on patch to HADOOP-18186. Contributed by: Viraj Jasani

asfgit pushed a commit that referenced this pull request Apr 28, 2023

HADOOP-18455. S3A prefetching executor should be closed (#4879)

f07be3b

follow-on patch to HADOOP-18186. Contributed by: Viraj Jasani

HADOOP-18455. s3a prefetching Executor should be closed #4879

HADOOP-18455. s3a prefetching Executor should be closed #4879

Uh oh!

Conversation

virajjasani commented Sep 12, 2022

Uh oh!

virajjasani commented Sep 12, 2022

Uh oh!

virajjasani commented Sep 12, 2022

Uh oh!

hadoop-yetus commented Sep 12, 2022

Uh oh!

virajjasani commented Sep 13, 2022

Uh oh!

hadoop-yetus commented Sep 14, 2022

Uh oh!

mehakmeet left a comment

Choose a reason for hiding this comment

Uh oh!

mukund-thakur left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mukund-thakur commented Sep 16, 2022

Uh oh!

virajjasani commented Sep 16, 2022

Uh oh!

virajjasani commented Sep 16, 2022

Uh oh!

virajjasani commented Sep 17, 2022

Uh oh!

mukund-thakur left a comment

Choose a reason for hiding this comment

Uh oh!

steveloughran commented Sep 20, 2022

Uh oh!

hadoop-yetus commented Sep 21, 2022

Uh oh!

mukund-thakur left a comment

Choose a reason for hiding this comment

Uh oh!

steveloughran commented Sep 22, 2022

Uh oh!

virajjasani commented Sep 22, 2022

Uh oh!

virajjasani commented Sep 22, 2022

Uh oh!

virajjasani commented Sep 22, 2022

Uh oh!

virajjasani commented Sep 25, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants