YARN-11103. SLS cleanup after previously merged SLS refactor jiras #4119

szilard-nemeth · 2022-03-29T19:52:11Z

Description of PR

How was this patch tested?

For code changes:

Does the title or this PR starts with the corresponding JIRA issue id (e.g. 'HADOOP-17799. Your PR title ...')?
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?
If applicable, have you updated the LICENSE, LICENSE-binary, NOTICE-binary files?

hadoop-yetus · 2022-03-29T21:41:53Z

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	1m 5s		Docker mode activated.
			_ Prechecks _
+1 💚	dupname	0m 0s		No case conflicting files found.
+0 🆗	codespell	0m 0s		codespell was not available.
+1 💚	@author	0m 1s		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 _
+1 💚	mvninstall	37m 43s		trunk passed
+1 💚	compile	0m 28s		trunk passed with JDK Ubuntu-11.0.14+9-Ubuntu-0ubuntu2.20.04
+1 💚	compile	0m 25s		trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	checkstyle	0m 22s		trunk passed
+1 💚	mvnsite	0m 30s		trunk passed
+1 💚	javadoc	0m 28s		trunk passed with JDK Ubuntu-11.0.14+9-Ubuntu-0ubuntu2.20.04
+1 💚	javadoc	0m 25s		trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
-1 ❌	spotbugs	0m 50s	/branch-spotbugs-hadoop-tools_hadoop-sls-warnings.html	hadoop-tools/hadoop-sls in trunk has 2 extant spotbugs warnings.
+1 💚	shadedclient	25m 8s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+1 💚	mvninstall	0m 21s		the patch passed
+1 💚	compile	0m 21s		the patch passed with JDK Ubuntu-11.0.14+9-Ubuntu-0ubuntu2.20.04
+1 💚	javac	0m 21s		the patch passed
+1 💚	compile	0m 19s		the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	javac	0m 19s		the patch passed
-1 ❌	blanks	0m 0s	/blanks-eol.txt	The patch has 1 line(s) that end in blanks. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply
-0 ⚠️	checkstyle	0m 13s	/results-checkstyle-hadoop-tools_hadoop-sls.txt	hadoop-tools/hadoop-sls: The patch generated 14 new + 23 unchanged - 24 fixed = 37 total (was 47)
+1 💚	mvnsite	0m 21s		the patch passed
+1 💚	javadoc	0m 19s		hadoop-tools_hadoop-sls-jdkUbuntu-11.0.14+9-Ubuntu-0ubuntu2.20.04 with JDK Ubuntu-11.0.14+9-Ubuntu-0ubuntu2.20.04 generated 0 new + 19 unchanged - 6 fixed = 19 total (was 25)
+1 💚	javadoc	0m 17s		hadoop-tools_hadoop-sls-jdkPrivateBuild-1.8.0_312-8u312-b07-0ubuntu1~~20.04-b07 with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~~20.04-b07 generated 0 new + 19 unchanged - 6 fixed = 19 total (was 25)
-1 ❌	spotbugs	0m 53s	/new-spotbugs-hadoop-tools_hadoop-sls.html	hadoop-tools/hadoop-sls generated 1 new + 1 unchanged - 1 fixed = 2 total (was 2)
+1 💚	shadedclient	23m 47s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
+1 💚	unit	12m 4s		hadoop-sls in the patch passed.
+1 💚	asflicense	0m 30s		The patch does not generate ASF License warnings.
		108m 14s

Reason	Tests
SpotBugs	module:hadoop-tools/hadoop-sls
	Write to static field org.apache.hadoop.yarn.sls.AMRunner.remainingApps from instance method org.apache.hadoop.yarn.sls.AMRunner.startAM() At AMRunner.java:from instance method org.apache.hadoop.yarn.sls.AMRunner.startAM() At AMRunner.java:[line 117]

Subsystem	Report/Notes
Docker	ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4119/1/artifact/out/Dockerfile
GITHUB PR	#4119
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell
uname	Linux 8ad2c5275e18 4.15.0-163-generic #171-Ubuntu SMP Fri Nov 5 11:55:11 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/bin/hadoop.sh
git revision	trunk / 981827bbd38e481e26d37c4aebba051f8f7ea377
Default Java	Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
Multi-JDK versions	/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.14+9-Ubuntu-0ubuntu2.20.04 /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-4119/1/testReport/
Max. process+thread count	522 (vs. ulimit of 5500)
modules	C: hadoop-tools/hadoop-sls U: hadoop-tools/hadoop-sls
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4119/1/console
versions	git=2.25.1 maven=3.6.3 spotbugs=4.2.2
Powered by	Apache Yetus 0.14.0-SNAPSHOT https://yetus.apache.org

This message was automatically generated.

hadoop-yetus · 2022-03-29T21:54:38Z

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	1m 21s		Docker mode activated.
			_ Prechecks _
+1 💚	dupname	0m 0s		No case conflicting files found.
+0 🆗	codespell	0m 0s		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 3 new or modified test files.
			_ trunk Compile Tests _
+1 💚	mvninstall	37m 46s		trunk passed
+1 💚	compile	0m 30s		trunk passed with JDK Ubuntu-11.0.14+9-Ubuntu-0ubuntu2.20.04
+1 💚	compile	0m 24s		trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	checkstyle	0m 21s		trunk passed
+1 💚	mvnsite	0m 29s		trunk passed
+1 💚	javadoc	0m 29s		trunk passed with JDK Ubuntu-11.0.14+9-Ubuntu-0ubuntu2.20.04
+1 💚	javadoc	0m 23s		trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
-1 ❌	spotbugs	0m 49s	/branch-spotbugs-hadoop-tools_hadoop-sls-warnings.html	hadoop-tools/hadoop-sls in trunk has 2 extant spotbugs warnings.
+1 💚	shadedclient	23m 47s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+1 💚	mvninstall	0m 20s		the patch passed
+1 💚	compile	0m 24s		the patch passed with JDK Ubuntu-11.0.14+9-Ubuntu-0ubuntu2.20.04
+1 💚	javac	0m 24s		the patch passed
+1 💚	compile	0m 20s		the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	javac	0m 20s		the patch passed
-1 ❌	blanks	0m 0s	/blanks-eol.txt	The patch has 1 line(s) that end in blanks. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply
-0 ⚠️	checkstyle	0m 15s	/results-checkstyle-hadoop-tools_hadoop-sls.txt	hadoop-tools/hadoop-sls: The patch generated 13 new + 53 unchanged - 29 fixed = 66 total (was 82)
+1 💚	mvnsite	0m 22s		the patch passed
+1 💚	javadoc	0m 21s		hadoop-tools_hadoop-sls-jdkUbuntu-11.0.14+9-Ubuntu-0ubuntu2.20.04 with JDK Ubuntu-11.0.14+9-Ubuntu-0ubuntu2.20.04 generated 0 new + 19 unchanged - 6 fixed = 19 total (was 25)
+1 💚	javadoc	0m 17s		hadoop-tools_hadoop-sls-jdkPrivateBuild-1.8.0_312-8u312-b07-0ubuntu1~~20.04-b07 with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~~20.04-b07 generated 0 new + 19 unchanged - 6 fixed = 19 total (was 25)
-1 ❌	spotbugs	0m 56s	/new-spotbugs-hadoop-tools_hadoop-sls.html	hadoop-tools/hadoop-sls generated 1 new + 1 unchanged - 1 fixed = 2 total (was 2)
+1 💚	shadedclient	23m 30s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
+1 💚	unit	12m 3s		hadoop-sls in the patch passed.
+1 💚	asflicense	0m 29s		The patch does not generate ASF License warnings.
		106m 59s

Reason	Tests
SpotBugs	module:hadoop-tools/hadoop-sls
	Write to static field org.apache.hadoop.yarn.sls.AMRunner.remainingApps from instance method org.apache.hadoop.yarn.sls.AMRunner.startAM() At AMRunner.java:from instance method org.apache.hadoop.yarn.sls.AMRunner.startAM() At AMRunner.java:[line 116]

Subsystem	Report/Notes
Docker	ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4119/2/artifact/out/Dockerfile
GITHUB PR	#4119
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell
uname	Linux 81a7671907b1 4.15.0-163-generic #171-Ubuntu SMP Fri Nov 5 11:55:11 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/bin/hadoop.sh
git revision	trunk / f4382e448deee7e74269d510e850a492cf97498a
Default Java	Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
Multi-JDK versions	/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.14+9-Ubuntu-0ubuntu2.20.04 /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-4119/2/testReport/
Max. process+thread count	593 (vs. ulimit of 5500)
modules	C: hadoop-tools/hadoop-sls U: hadoop-tools/hadoop-sls
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4119/2/console
versions	git=2.25.1 maven=3.6.3 spotbugs=4.2.2
Powered by	Apache Yetus 0.14.0-SNAPSHOT https://yetus.apache.org

This message was automatically generated.

brumi1024

Thanks @szilard-nemeth for the patch. I had one minor comment.

hadoop-tools/hadoop-sls/src/main/java/org/apache/hadoop/yarn/sls/AMRunner.java

hadoop-yetus · 2022-03-30T20:11:53Z

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	1m 4s		Docker mode activated.
			_ Prechecks _
+1 💚	dupname	0m 1s		No case conflicting files found.
+0 🆗	codespell	0m 0s		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 3 new or modified test files.
			_ trunk Compile Tests _
+1 💚	mvninstall	38m 2s		trunk passed
+1 💚	compile	0m 35s		trunk passed with JDK Ubuntu-11.0.14+9-Ubuntu-0ubuntu2.20.04
+1 💚	compile	0m 39s		trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	checkstyle	0m 33s		trunk passed
+1 💚	mvnsite	0m 46s		trunk passed
+1 💚	javadoc	0m 44s		trunk passed with JDK Ubuntu-11.0.14+9-Ubuntu-0ubuntu2.20.04
+1 💚	javadoc	0m 38s		trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
-1 ❌	spotbugs	1m 8s	/branch-spotbugs-hadoop-tools_hadoop-sls-warnings.html	hadoop-tools/hadoop-sls in trunk has 3 extant spotbugs warnings.
+1 💚	shadedclient	25m 13s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+1 💚	mvninstall	0m 22s		the patch passed
+1 💚	compile	0m 22s		the patch passed with JDK Ubuntu-11.0.14+9-Ubuntu-0ubuntu2.20.04
+1 💚	javac	0m 21s		the patch passed
+1 💚	compile	0m 18s		the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	javac	0m 18s		the patch passed
-1 ❌	blanks	0m 0s	/blanks-eol.txt	The patch has 1 line(s) that end in blanks. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply
-0 ⚠️	checkstyle	0m 13s	/results-checkstyle-hadoop-tools_hadoop-sls.txt	hadoop-tools/hadoop-sls: The patch generated 13 new + 59 unchanged - 24 fixed = 72 total (was 83)
+1 💚	mvnsite	0m 20s		the patch passed
+1 💚	javadoc	0m 19s		hadoop-tools_hadoop-sls-jdkUbuntu-11.0.14+9-Ubuntu-0ubuntu2.20.04 with JDK Ubuntu-11.0.14+9-Ubuntu-0ubuntu2.20.04 generated 0 new + 20 unchanged - 6 fixed = 20 total (was 26)
+1 💚	javadoc	0m 17s		hadoop-tools_hadoop-sls-jdkPrivateBuild-1.8.0_312-8u312-b07-0ubuntu1~~20.04-b07 with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~~20.04-b07 generated 0 new + 20 unchanged - 6 fixed = 20 total (was 26)
-1 ❌	spotbugs	0m 54s	/new-spotbugs-hadoop-tools_hadoop-sls.html	hadoop-tools/hadoop-sls generated 1 new + 2 unchanged - 1 fixed = 3 total (was 3)
+1 💚	shadedclient	25m 25s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
+1 💚	unit	12m 16s		hadoop-sls in the patch passed.
+1 💚	asflicense	0m 33s		The patch does not generate ASF License warnings.
		111m 58s

Reason	Tests
SpotBugs	module:hadoop-tools/hadoop-sls
	Write to static field org.apache.hadoop.yarn.sls.AMRunner.remainingApps from instance method org.apache.hadoop.yarn.sls.AMRunner.startAM() At AMRunner.java:from instance method org.apache.hadoop.yarn.sls.AMRunner.startAM() At AMRunner.java:[line 115]

Subsystem	Report/Notes
Docker	ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4119/3/artifact/out/Dockerfile
GITHUB PR	#4119
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell
uname	Linux 0999cbd810d1 4.15.0-163-generic #171-Ubuntu SMP Fri Nov 5 11:55:11 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/bin/hadoop.sh
git revision	trunk / `66493fa`
Default Java	Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
Multi-JDK versions	/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.14+9-Ubuntu-0ubuntu2.20.04 /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-4119/3/testReport/
Max. process+thread count	523 (vs. ulimit of 5500)
modules	C: hadoop-tools/hadoop-sls U: hadoop-tools/hadoop-sls
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4119/3/console
versions	git=2.25.1 maven=3.6.3 spotbugs=4.2.2
Powered by	Apache Yetus 0.14.0-SNAPSHOT https://yetus.apache.org

This message was automatically generated.

9uapaw

Thanks for the PR @szilard-nemeth. It is straightforward and a good clean-up change. Let us wait for YARN-11102 to be merged in order not to increase spotbugs warning. I will commit it afterwards.

hadoop-tools/hadoop-sls/src/main/java/org/apache/hadoop/yarn/sls/AMRunner.java

szilard-nemeth · 2022-03-31T11:00:52Z

@9uapaw
Thanks for your comment. This is an independent change and the spotbugs warning was in place before this patch, so I suggest to commit this first then I will fix the spotbugs in the other one.
Thanks

brumi1024 · 2022-03-31T12:37:19Z

Thanks @szilard-nemeth for the PR, since a different PR exists for the spotbugs issue I committed this one to trunk. Fixed the indentation checkstyle issues before merging.

brumi1024 reviewed Mar 30, 2022

View reviewed changes

hadoop-tools/hadoop-sls/src/main/java/org/apache/hadoop/yarn/sls/AMRunner.java Outdated Show resolved Hide resolved

szilard-nemeth added 5 commits March 30, 2022 20:17

Fix cleanup comments from YARN-10548

393e36f

Fix cleanup comments from YARN-10552

3a1cb47

Cleanup traceType / traceLocation parsing

b5c0651

Revert remainingApps int to AtomicInteger changes as it caused issues

e55839b

Generic code cleanup

66493fa

szilard-nemeth force-pushed the YARN-11103 branch from f4382e4 to 66493fa Compare March 30, 2022 18:18

9uapaw approved these changes Mar 31, 2022

View reviewed changes

hadoop-tools/hadoop-sls/src/main/java/org/apache/hadoop/yarn/sls/AMRunner.java Outdated Show resolved Hide resolved

brumi1024 closed this Mar 31, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

YARN-11103. SLS cleanup after previously merged SLS refactor jiras #4119

YARN-11103. SLS cleanup after previously merged SLS refactor jiras #4119

Uh oh!

szilard-nemeth commented Mar 29, 2022

Uh oh!

hadoop-yetus commented Mar 29, 2022

Uh oh!

hadoop-yetus commented Mar 29, 2022

Uh oh!

brumi1024 left a comment

Uh oh!

Uh oh!

hadoop-yetus commented Mar 30, 2022

Uh oh!

9uapaw left a comment

Uh oh!

Uh oh!

szilard-nemeth commented Mar 31, 2022

Uh oh!

brumi1024 commented Mar 31, 2022

Uh oh!

Uh oh!

YARN-11103. SLS cleanup after previously merged SLS refactor jiras #4119

YARN-11103. SLS cleanup after previously merged SLS refactor jiras #4119

Uh oh!

Conversation

szilard-nemeth commented Mar 29, 2022

Description of PR

How was this patch tested?

For code changes:

Uh oh!

hadoop-yetus commented Mar 29, 2022

Uh oh!

hadoop-yetus commented Mar 29, 2022

Uh oh!

brumi1024 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

hadoop-yetus commented Mar 30, 2022

Uh oh!

9uapaw left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

szilard-nemeth commented Mar 31, 2022

Uh oh!

brumi1024 commented Mar 31, 2022

Uh oh!

Uh oh!