HADOOP-18397. Shutdown AWSSecurityTokenService when its resources are no longer in use #4722

virajjasani · 2022-08-09T06:03:27Z

Description of PR

AWSSecurityTokenService resources can be released whenever they are no longer in use. The documentation of AWSSecurityTokenService#shutdown says while it is not important for client to compulsorily shutdown the token service, client can definitely perform early release whenever client no longer requires token service resources. We achieve this by making STSClient closable, so we can certainly utilize it in all places where it's suitable.

…e no longer in use

virajjasani · 2022-08-09T06:04:05Z

Tested against endpoint us-west-2:

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

[INFO] --- maven-dependency-plugin:3.0.2:copy-dependencies (copy) @ hadoop-aws ---
[INFO] 
[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: 406, Failures: 0, Errors: 0, Skipped: 4
[INFO] 
[INFO] 


[INFO] --- maven-site-plugin:3.11.0:attach-descriptor (attach-descriptor) @ hadoop-aws ---
[INFO] Skipping because packaging 'jar' is not pom.
[INFO] 
[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: 1139, Failures: 0, Errors: 0, Skipped: 186
[INFO] 


[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] 
[WARNING] Tests run: 124, Failures: 0, Errors: 0, Skipped: 84
[INFO] 
[INFO]

steveloughran · 2022-08-09T11:29:35Z

back when we did that the sts client shutdown just threw UnsupportedOperationException. is that no longer the case? if so, we should stop swallowing it in our STSClient

virajjasani · 2022-08-09T19:09:30Z

back when we did that the sts client shutdown just threw UnsupportedOperationException. is that no longer the case? if so, we should stop swallowing it in our STSClient

That is true, we no longer need to catch UnsupportedOperationException. Based on the implementations that implement AWSSecurityTokenService, it seems that this Exception is now only thrown by AbstractAWSSecurityTokenService, which we don't use anyways.

AbstractAWSSecurityTokenService:

    @Override
    public void shutdown() {
        throw new java.lang.UnsupportedOperationException();
    }

AWSSecurityTokenServiceAsyncClient:

    @Override
    public void shutdown() {
        super.shutdown();
        executorService.shutdownNow();
    }

AWSSecurityTokenServiceClient:

    @Override
    public void shutdown() {
        super.shutdown();
    }

We are good here. Just ran all the tests against new commit. Sharing the test results in the next comment.

virajjasani · 2022-08-09T19:11:46Z

Tested the new commit changes against endpoint us-west-2:

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

[INFO] --- maven-dependency-plugin:3.0.2:copy-dependencies (copy) @ hadoop-aws ---
[INFO] 
[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: 406, Failures: 0, Errors: 0, Skipped: 4
[INFO] 
[INFO] 


[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: 1139, Failures: 0, Errors: 0, Skipped: 186
[INFO] 


[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] 
[WARNING] Tests run: 124, Failures: 0, Errors: 0, Skipped: 84
[INFO]

hadoop-yetus · 2022-08-09T20:53:11Z

🎊 +1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 53s		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 appears to include 1 new or modified test files.
			_ trunk Compile Tests _
+1 💚	mvninstall	42m 1s		trunk passed
+1 💚	compile	1m 1s		trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚	compile	0m 48s		trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	checkstyle	0m 43s		trunk passed
+1 💚	mvnsite	0m 55s		trunk passed
+1 💚	javadoc	0m 41s		trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚	javadoc	0m 44s		trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	spotbugs	1m 30s		trunk passed
+1 💚	shadedclient	24m 12s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+1 💚	mvninstall	0m 41s		the patch passed
+1 💚	compile	0m 49s		the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚	javac	0m 49s		the patch passed
+1 💚	compile	0m 34s		the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	javac	0m 34s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
+1 💚	checkstyle	0m 24s		the patch passed
+1 💚	mvnsite	0m 41s		the patch passed
+1 💚	javadoc	0m 18s		the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚	javadoc	0m 28s		the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	spotbugs	1m 18s		the patch passed
+1 💚	shadedclient	24m 3s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
+1 💚	unit	3m 0s		hadoop-aws in the patch passed.
+1 💚	asflicense	0m 42s		The patch does not generate ASF License warnings.
		107m 6s

Subsystem	Report/Notes
Docker	ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4722/2/artifact/out/Dockerfile
GITHUB PR	#4722
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets
uname	Linux 457c4552f045 4.15.0-166-generic #174-Ubuntu SMP Wed Dec 8 19:07:44 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/bin/hadoop.sh
git revision	trunk / `f2afaf2`
Default Java	Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
Multi-JDK versions	/usr/lib/jvm/java-11-openjdk-amd64:Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
Test Results	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4722/2/testReport/
Max. process+thread count	618 (vs. ulimit of 5500)
modules	C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4722/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.

virajjasani · 2022-08-09T21:59:28Z

Re-ran all the tests with scale profile, all tests are passing (three tests flaked but when they were run individually, they passed).

[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] 
[ERROR] Failures: 
[ERROR]   ITestS3AContractUnbuffer>AbstractContractUnbufferTest.testMultipleUnbuffers:100->AbstractContractUnbufferTest.validateFullFileContents:132->AbstractContractUnbufferTest.validateFileContents:139->Assert.assertEquals:647->Assert.failNotEquals:835->Assert.fail:89 failed to read expected number of bytes from stream. This may be transient expected:<1024> but was:<605>
[ERROR]   ITestS3AContractUnbuffer>AbstractContractUnbufferTest.testUnbufferAfterRead:53->AbstractContractUnbufferTest.validateFullFileContents:132->AbstractContractUnbufferTest.validateFileContents:139->Assert.assertEquals:647->Assert.failNotEquals:835->Assert.fail:89 failed to read expected number of bytes from stream. This may be transient expected:<1024> but was:<605>
[INFO] 
[ERROR] Tests run: 1139, Failures: 2, Errors: 0, Skipped: 146
[INFO] 

[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
[INFO]

All tests in ITestS3AContractUnbuffer and ITestS3AContractRootDir passed when run individually.

With scale profile, the skipped tests count is much smaller. But as expected, with scale profile, the overall test suite run takes a lot of time, but it's still worth running.

steveloughran · 2022-08-12T10:29:05Z

unbuffer underflow, but the read() apis used are needed for coverage. don't worry too much there

steveloughran · 2022-08-12T10:30:52Z

But as expected, with scale profile, the overall test suite run takes a lot of time, but it's still worth running.

get an m1 MBP; cuts test time in half. doesn't do much for shading the client api's though, so it must be better parallelism than single core throughput.

the scale tests test big multipart uploads and a real terasort, so are important. i like to run them at least weekly

steveloughran

+1

… no longer in use (#4722) Contributed by Viraj Jasani.

steveloughran · 2022-08-12T14:37:17Z

merged to branch-3.3. too. thanks

virajjasani · 2022-08-12T19:14:33Z

get an m1 MBP; cuts test time in half.

Awesome, in fact I am just about to get m1 MBP!

i like to run them at least weekly

Nice, i will also run them weekly then, thanks for sharing @steveloughran !

… no longer in use (apache#4722) Contributed by Viraj Jasani.

HADOOP-18397. Shutdown AWSSecurityTokenService when it's resources ar…

88d58fe

…e no longer in use

no longer need to catch UnsupportedOperationException

f2afaf2

virajjasani changed the title ~~HADOOP-18397. Shutdown AWSSecurityTokenService when it's resources are no longer in use~~ HADOOP-18397. Shutdown AWSSecurityTokenService when its resources are no longer in use Aug 11, 2022

apache deleted a comment from hadoop-yetus Aug 12, 2022

steveloughran approved these changes Aug 12, 2022

View reviewed changes

steveloughran merged commit 8c9533a into apache:trunk Aug 12, 2022

asfgit pushed a commit that referenced this pull request Aug 12, 2022

HADOOP-18397. Shutdown AWSSecurityTokenService when its resources are…

6b7c132

… no longer in use (#4722) Contributed by Viraj Jasani.

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

HADOOP-18397. Shutdown AWSSecurityTokenService when its resources are…

8d7ddec

… no longer in use (apache#4722) Contributed by Viraj Jasani.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HADOOP-18397. Shutdown AWSSecurityTokenService when its resources are no longer in use #4722

HADOOP-18397. Shutdown AWSSecurityTokenService when its resources are no longer in use #4722

virajjasani commented Aug 9, 2022

virajjasani commented Aug 9, 2022

steveloughran commented Aug 9, 2022

virajjasani commented Aug 9, 2022

virajjasani commented Aug 9, 2022

hadoop-yetus commented Aug 9, 2022

virajjasani commented Aug 9, 2022 •

edited

Loading

steveloughran commented Aug 12, 2022

steveloughran commented Aug 12, 2022

steveloughran left a comment

steveloughran commented Aug 12, 2022

virajjasani commented Aug 12, 2022

HADOOP-18397. Shutdown AWSSecurityTokenService when its resources are no longer in use #4722

HADOOP-18397. Shutdown AWSSecurityTokenService when its resources are no longer in use #4722

Conversation

virajjasani commented Aug 9, 2022

Description of PR

virajjasani commented Aug 9, 2022

steveloughran commented Aug 9, 2022

virajjasani commented Aug 9, 2022

virajjasani commented Aug 9, 2022

hadoop-yetus commented Aug 9, 2022

virajjasani commented Aug 9, 2022 • edited Loading

steveloughran commented Aug 12, 2022

steveloughran commented Aug 12, 2022

steveloughran left a comment

Choose a reason for hiding this comment

steveloughran commented Aug 12, 2022

virajjasani commented Aug 12, 2022

virajjasani commented Aug 9, 2022 •

edited

Loading