YARN-10553. Refactor TestDistributedShell #2581

amahussein · 2020-12-31T16:39:12Z

NOTICE

Please create an issue in ASF JIRA before opening a pull request,
and you need to set the title of the pull request which starts with
the corresponding JIRA issue number. (e.g. HADOOP-XXXXX. Fix a typo in YYY.)
For more details, please see https://cwiki.apache.org/confluence/display/HADOOP/How+To+Contribute

hadoop-yetus · 2020-12-31T20:18:35Z

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 32s		Docker mode activated.
			_ Prechecks _
+1 💚	dupname	0m 0s		No case conflicting files found.
+1 💚	@author	0m 0s		The patch does not contain any @author tags.
+1 💚		0m 0s	test4tests	The patch appears to include 5 new or modified test files.
			_ trunk Compile Tests _
+0 🆗	mvndep	13m 45s		Maven dependency ordering for branch
+1 💚	mvninstall	20m 57s		trunk passed
+1 💚	compile	20m 9s		trunk passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.18.04
+1 💚	compile	17m 15s		trunk passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01
+1 💚	checkstyle	2m 42s		trunk passed
+1 💚	mvnsite	1m 29s		trunk passed
+1 💚	shadedclient	19m 57s		branch has no errors when building and testing our client artifacts.
+1 💚	javadoc	1m 21s		trunk passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.18.04
+1 💚	javadoc	1m 18s		trunk passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01
+0 🆗	spotbugs	0m 55s		Used deprecated FindBugs config; considering switching to SpotBugs.
+1 💚	findbugs	1m 52s		trunk passed
			_ Patch Compile Tests _
+0 🆗	mvndep	0m 27s		Maven dependency ordering for patch
-1 ❌	mvninstall	0m 19s	/patch-mvninstall-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-applications_hadoop-yarn-applications-distributedshell.txt	hadoop-yarn-applications-distributedshell in the patch failed.
-1 ❌	compile	10m 11s	/patch-compile-root-jdkUbuntu-11.0.9.1+1-Ubuntu-0ubuntu1.18.04.txt	root in the patch failed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.18.04.
-1 ❌	javac	10m 11s	/patch-compile-root-jdkUbuntu-11.0.9.1+1-Ubuntu-0ubuntu1.18.04.txt	root in the patch failed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.18.04.
-1 ❌	compile	8m 56s	/patch-compile-root-jdkPrivateBuild-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01.txt	root in the patch failed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01.
-1 ❌	javac	8m 56s	/patch-compile-root-jdkPrivateBuild-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01.txt	root in the patch failed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01.
-0 ⚠️	checkstyle	2m 20s	/diff-checkstyle-root.txt	root: The patch generated 6 new + 5 unchanged - 12 fixed = 11 total (was 17)
-1 ❌	mvnsite	0m 26s	/patch-mvnsite-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-applications_hadoop-yarn-applications-distributedshell.txt	hadoop-yarn-applications-distributedshell in the patch failed.
+1 💚	whitespace	0m 0s		The patch has no whitespace issues.
+1 💚	shadedclient	15m 5s		patch has no errors when building and testing our client artifacts.
+1 💚	javadoc	0m 49s		the patch passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.18.04
+1 💚	javadoc	0m 48s		the patch passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01
-1 ❌	findbugs	0m 25s	/patch-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-applications_hadoop-yarn-applications-distributedshell.txt	hadoop-yarn-applications-distributedshell in the patch failed.
			_ Other Tests _
-1 ❌	unit	0m 26s	/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-applications_hadoop-yarn-applications-distributedshell.txt	hadoop-yarn-applications-distributedshell in the patch failed.
-1 ❌	unit	0m 29s	/patch-unit-hadoop-tools_hadoop-dynamometer_hadoop-dynamometer-infra.txt	hadoop-dynamometer-infra in the patch passed.
+1 💚	asflicense	0m 36s		The patch does not generate ASF License warnings.
		144m 15s

Reason	Tests
Failed junit tests	hadoop.tools.dynamometer.TestDynamometerInfra

Subsystem	Report/Notes
Docker	ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2581/1/artifact/out/Dockerfile
GITHUB PR	#2581
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle
uname	Linux 7fc68d3db63c 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/bin/hadoop.sh
git revision	trunk / `9b2956e`
Default Java	Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01
Multi-JDK versions	/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.18.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01
Test Results	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2581/1/testReport/
Max. process+thread count	670 (vs. ulimit of 5500)
modules	C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-applications-distributedshell hadoop-tools/hadoop-dynamometer/hadoop-dynamometer-infra U: .
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2581/1/console
versions	git=2.17.1 maven=3.6.0 findbugs=4.0.6
Powered by	Apache Yetus 0.13.0-SNAPSHOT https://yetus.apache.org

This message was automatically generated.

goiri · 2020-12-31T20:21:11Z

...test/java/org/apache/hadoop/yarn/applications/distributedshell/DistributedShellBaseTest.java

+    }, 10, 60000);
+  }
+
+


Avoid double spaces? A high level javadoc wouldn't hurt either.

...test/java/org/apache/hadoop/yarn/applications/distributedshell/DistributedShellBaseTest.java

jiwq

@amahussein Could you have time to fix the conflicts?

hadoop-yetus · 2021-01-06T03:03:01Z

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	29m 37s		Docker mode activated.
			_ Prechecks _
+1 💚	dupname	0m 0s		No case conflicting files found.
+1 💚	@author	0m 0s		The patch does not contain any @author tags.
+1 💚		0m 0s	test4tests	The patch appears to include 8 new or modified test files.
			_ trunk Compile Tests _
+0 🆗	mvndep	13m 45s		Maven dependency ordering for branch
+1 💚	mvninstall	21m 15s		trunk passed
+1 💚	compile	20m 59s		trunk passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.18.04
+1 💚	compile	17m 41s		trunk passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01
+1 💚	checkstyle	2m 53s		trunk passed
+1 💚	mvnsite	1m 52s		trunk passed
+1 💚	shadedclient	20m 23s		branch has no errors when building and testing our client artifacts.
+1 💚	javadoc	1m 34s		trunk passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.18.04
+1 💚	javadoc	1m 33s		trunk passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01
+0 🆗	spotbugs	0m 47s		Used deprecated FindBugs config; considering switching to SpotBugs.
+0 🆗	findbugs	0m 37s		branch/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-tests no findbugs output file (findbugsXml.xml)
			_ Patch Compile Tests _
+0 🆗	mvndep	0m 27s		Maven dependency ordering for patch
+1 💚	mvninstall	1m 10s		the patch passed
+1 💚	compile	20m 1s		the patch passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.18.04
+1 💚	javac	20m 1s		the patch passed
+1 💚	compile	17m 42s		the patch passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01
+1 💚	javac	17m 42s		the patch passed
+1 💚	checkstyle	2m 36s		root: The patch generated 0 new + 164 unchanged - 18 fixed = 164 total (was 182)
+1 💚	mvnsite	1m 49s		the patch passed
-1 ❌	whitespace	0m 0s	/whitespace-eol.txt	The patch has 1 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply
+1 💚	shadedclient	15m 38s		patch has no errors when building and testing our client artifacts.
+1 💚	javadoc	1m 38s		the patch passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.18.04
+1 💚	javadoc	1m 34s		the patch passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01
+0 🆗	findbugs	0m 34s		hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-tests has no data from findbugs
			_ Other Tests _
+1 💚	unit	3m 19s		hadoop-yarn-server-tests in the patch passed.
-1 ❌	unit	31m 46s	/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-applications_hadoop-yarn-applications-distributedshell.txt	hadoop-yarn-applications-distributedshell in the patch passed.
-1 ❌	unit	0m 37s	/patch-unit-hadoop-tools_hadoop-dynamometer_hadoop-dynamometer-infra.txt	hadoop-dynamometer-infra in the patch passed.
+1 💚	asflicense	0m 48s		The patch does not generate ASF License warnings.
		235m 40s

Reason	Tests
Failed junit tests	hadoop.yarn.applications.distributedshell.TestDSShellTimelineV10
	hadoop.yarn.applications.distributedshell.TestDSShellTimelineV15
	hadoop.yarn.applications.distributedshell.TestDSShellTimelineV20
	hadoop.yarn.applications.distributedshell.TestDSWithMultipleNodeManager
	hadoop.tools.dynamometer.TestDynamometerInfra

Subsystem	Report/Notes
Docker	ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2581/2/artifact/out/Dockerfile
GITHUB PR	#2581
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle
uname	Linux ccac8175045e 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/bin/hadoop.sh
git revision	trunk / `b1abb10`
Default Java	Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01
Multi-JDK versions	/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.18.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01
Test Results	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2581/2/testReport/
Max. process+thread count	635 (vs. ulimit of 5500)
modules	C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-tests hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-applications-distributedshell hadoop-tools/hadoop-dynamometer/hadoop-dynamometer-infra U: .
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2581/2/console
versions	git=2.17.1 maven=3.6.0 findbugs=4.0.6
Powered by	Apache Yetus 0.13.0-SNAPSHOT https://yetus.apache.org

This message was automatically generated.

goiri · 2021-01-06T22:44:23Z

...tributedshell/src/main/java/org/apache/hadoop/yarn/applications/distributedshell/Client.java

    yarnClient.stop();
-    yarnClient.waitForServiceToStop(clientTimeout);
+    //yarnClient.waitForServiceToStop(clientTimeout);


Oh! I forgot to delete that line.
Waiting for the service to stop is not necessary.

amahussein · 2021-01-06T22:52:35Z

@goiri the code changes fix {{testDSShellWithEnforceExecutionType}}.
The problem with the test was that it launches two containers executing cmd date. Apparently the two containers would exit fast. The unit test will stay blocked waiting for the containers to be exactly "2".
This does not take into consideration that the containers count is 3 including the AMContainer.

The fix was to get rid of the equality in the check, and change the application command to ls

amahussein · 2021-01-06T23:00:09Z

CC: @iwasakims . This includes a fix to testDSShellWithEnforceExecutionType

goiri · 2021-01-06T23:17:05Z

@goiri the code changes fix {{testDSShellWithEnforceExecutionType}}.
The problem with the test was that it launches two containers executing cmd date. Apparently the two containers would exit fast. The unit test will stay blocked waiting for the containers to be exactly "2".
This does not take into consideration that the containers count is 3 including the AMContainer.

The fix was to get rid of the equality in the check, and change the application command to ls

That makes sense, does it make sense to make the assert more general than an equality instead of getting rid of the equality?

...er/hadoop-yarn-server-tests/src/test/java/org/apache/hadoop/yarn/server/MiniYARNCluster.java

amahussein · 2021-01-07T00:35:33Z

@goiri the code changes fix {{testDSShellWithEnforceExecutionType}}.
The problem with the test was that it launches two containers executing cmd date. Apparently the two containers would exit fast. The unit test will stay blocked waiting for the containers to be exactly "2".
This does not take into consideration that the containers count is 3 including the AMContainer.
The fix was to get rid of the equality in the check, and change the application command to ls

That makes sense, does it make sense to make the assert more general than an equality instead of getting rid of the equality?

In DistributedShellBaseTest.waitForContainersLaunch(), I used inequality if (containers.size() < nContainers) { return false; }

hadoop-yetus · 2021-01-07T01:25:55Z

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 31s		Docker mode activated.
			_ Prechecks _
+1 💚	dupname	0m 0s		No case conflicting files found.
+1 💚	@author	0m 0s		The patch does not contain any @author tags.
+1 💚		0m 0s	test4tests	The patch appears to include 8 new or modified test files.
			_ trunk Compile Tests _
+0 🆗	mvndep	13m 45s		Maven dependency ordering for branch
+1 💚	mvninstall	21m 6s		trunk passed
+1 💚	compile	20m 3s		trunk passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.18.04
+1 💚	compile	17m 19s		trunk passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01
+1 💚	checkstyle	2m 45s		trunk passed
+1 💚	mvnsite	2m 10s		trunk passed
+1 💚	shadedclient	20m 54s		branch has no errors when building and testing our client artifacts.
+1 💚	javadoc	1m 52s		trunk passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.18.04
+1 💚	javadoc	1m 53s		trunk passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01
+0 🆗	spotbugs	0m 54s		Used deprecated FindBugs config; considering switching to SpotBugs.
+0 🆗	findbugs	0m 44s		branch/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-tests no findbugs output file (findbugsXml.xml)
			_ Patch Compile Tests _
+0 🆗	mvndep	0m 28s		Maven dependency ordering for patch
+1 💚	mvninstall	1m 15s		the patch passed
+1 💚	compile	19m 27s		the patch passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.18.04
+1 💚	javac	19m 27s		the patch passed
+1 💚	compile	17m 23s		the patch passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01
+1 💚	javac	17m 23s		the patch passed
-0 ⚠️	checkstyle	3m 15s	/diff-checkstyle-root.txt	root: The patch generated 4 new + 164 unchanged - 18 fixed = 168 total (was 182)
+1 💚	mvnsite	2m 12s		the patch passed
+1 💚	whitespace	0m 0s		The patch has no whitespace issues.
+1 💚	shadedclient	15m 27s		patch has no errors when building and testing our client artifacts.
+1 💚	javadoc	1m 54s		the patch passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.18.04
+1 💚	javadoc	1m 52s		the patch passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01
+0 🆗	findbugs	0m 41s		hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-tests has no data from findbugs
			_ Other Tests _
+1 💚	unit	3m 26s		hadoop-yarn-server-tests in the patch passed.
+1 💚	unit	23m 44s		hadoop-yarn-applications-distributedshell in the patch passed.
-1 ❌	unit	0m 43s	/patch-unit-hadoop-tools_hadoop-dynamometer_hadoop-dynamometer-infra.txt	hadoop-dynamometer-infra in the patch passed.
+1 💚	asflicense	0m 56s		The patch does not generate ASF License warnings.
		200m 17s

Reason	Tests
Failed junit tests	hadoop.tools.dynamometer.TestDynamometerInfra

Subsystem	Report/Notes
Docker	ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2581/3/artifact/out/Dockerfile
GITHUB PR	#2581
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle
uname	Linux 72dec302ac86 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/bin/hadoop.sh
git revision	trunk / `b612c31`
Default Java	Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01
Multi-JDK versions	/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.18.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01
Test Results	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2581/3/testReport/
Max. process+thread count	722 (vs. ulimit of 5500)
modules	C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-tests hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-applications-distributedshell hadoop-tools/hadoop-dynamometer/hadoop-dynamometer-infra U: .
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2581/3/console
versions	git=2.17.1 maven=3.6.0 findbugs=4.0.6
Powered by	Apache Yetus 0.13.0-SNAPSHOT https://yetus.apache.org

This message was automatically generated.

iwasakims · 2021-01-07T02:40:27Z

Is the fix of o.a.h.tools.dynamometer.Cliet related to TestDistributedShell? It should be addressed in another JIRA if not.

...c/test/java/org/apache/hadoop/yarn/applications/distributedshell/TestDSShellTimelineV10.java

hadoop-yetus · 2021-01-07T06:34:20Z

🎊 +1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 35s		Docker mode activated.
			_ Prechecks _
+1 💚	dupname	0m 0s		No case conflicting files found.
+1 💚	@author	0m 0s		The patch does not contain any @author tags.
+1 💚		0m 0s	test4tests	The patch appears to include 7 new or modified test files.
			_ trunk Compile Tests _
+1 💚	mvninstall	32m 41s		trunk passed
+1 💚	compile	0m 29s		trunk passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.18.04
+1 💚	compile	0m 28s		trunk passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01
+1 💚	checkstyle	0m 29s		trunk passed
+1 💚	mvnsite	0m 31s		trunk passed
+1 💚	shadedclient	16m 11s		branch has no errors when building and testing our client artifacts.
+1 💚	javadoc	0m 28s		trunk passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.18.04
+1 💚	javadoc	0m 25s		trunk passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01
+0 🆗	spotbugs	0m 46s		Used deprecated FindBugs config; considering switching to SpotBugs.
+1 💚	findbugs	0m 43s		trunk passed
			_ Patch Compile Tests _
+1 💚	mvninstall	0m 25s		the patch passed
+1 💚	compile	0m 21s		the patch passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.18.04
+1 💚	javac	0m 21s		the patch passed
+1 💚	compile	0m 19s		the patch passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01
+1 💚	javac	0m 19s		the patch passed
+1 💚	checkstyle	0m 17s		hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-applications-distributedshell: The patch generated 0 new + 138 unchanged - 18 fixed = 138 total (was 156)
+1 💚	mvnsite	0m 22s		the patch passed
+1 💚	whitespace	0m 1s		The patch has no whitespace issues.
+1 💚	shadedclient	14m 52s		patch has no errors when building and testing our client artifacts.
+1 💚	javadoc	0m 22s		the patch passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.18.04
+1 💚	javadoc	0m 21s		the patch passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01
+1 💚	findbugs	0m 45s		the patch passed
			_ Other Tests _
+1 💚	unit	23m 36s		hadoop-yarn-applications-distributedshell in the patch passed.
+1 💚	asflicense	0m 34s		The patch does not generate ASF License warnings.
		97m 52s

Subsystem	Report/Notes
Docker	ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2581/4/artifact/out/Dockerfile
GITHUB PR	#2581
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle
uname	Linux 84c1beb5da96 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/bin/hadoop.sh
git revision	trunk / `b612c31`
Default Java	Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01
Multi-JDK versions	/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.18.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01
Test Results	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2581/4/testReport/
Max. process+thread count	718 (vs. ulimit of 5500)
modules	C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-applications-distributedshell U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-applications-distributedshell
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2581/4/console
versions	git=2.17.1 maven=3.6.0 findbugs=4.0.6
Powered by	Apache Yetus 0.13.0-SNAPSHOT https://yetus.apache.org

This message was automatically generated.

iwasakims · 2021-01-07T06:53:08Z

...ll/src/test/java/org/apache/hadoop/yarn/applications/distributedshell/TestDSTimelineV20.java

+      yarnClient.init(new Configuration(getYarnClusterConfiguration()));
+      yarnClient.start();
+
+      waitForContainersLaunch(yarnClient, 2, appAttemptReportRef,


nContainers should be 3 if need to take AM container into account?

iwasakims · 2021-01-07T06:53:42Z

...tributedshell/src/main/java/org/apache/hadoop/yarn/applications/distributedshell/Client.java

@@ -1330,7 +1330,7 @@ private void setAMResourceCapability(ApplicationSubmissionContext appContext,
    }
    if (amVCores == -1) {
      amVCores = DEFAULT_AM_VCORES;
-      LOG.warn("AM vcore not specified, use " + DEFAULT_AM_VCORES
+      LOG.warn("AM vcore not specified, use {}" + DEFAULT_AM_VCORES


looks like misused place holder.

iwasakims · 2021-01-07T06:56:44Z

Thank for working on this, @amahussein. LGTM overall pending some nits.

While it is too late here, it is hard to follow which part of the TestDistributedShell is modified if splitting the class and refactoring are mixed up in a single commit. Doing one thing in one PR makes reviewing and cherry-picking easier.

iwasakims · 2021-01-07T06:58:26Z

The implementation has lots of code redundancy.

It is inefficient in the setup and tearing down. The large percentage of time execution is exhausted by starting cluster and stopping the services.

I think there is a gap between the PR and the above description of YARN-10553. Just splitting the TestDistributedShell does not reduce code nor minicluster ramp-up time. It would be nice to update the JIRA description reflecting what is addressed.

amahussein · 2021-01-07T13:59:38Z

Thank for working on this, @amahussein. LGTM overall pending some nits.

While it is too late here, it is hard to follow which part of the TestDistributedShell is modified if splitting the class and refactoring are mixed up in a single commit. Doing one thing in one PR makes reviewing and cherry-picking easier.

That's good point @iwasakims . Sorry that I made it hard.
After my first commit, I received feedback from Inigio to transform busy-waiting loops into lambda. It is my bad I haven't managed to separate between refactoring and splitting.

amahussein · 2021-01-07T14:02:54Z

The implementation has lots of code redundancy.

It is inefficient in the setup and tearing down. The large percentage of time execution is exhausted by starting cluster and stopping the services.

I think there is a gap between the PR and the above description of YARN-10553. Just splitting the TestDistributedShell does not reduce code nor minicluster ramp-up time. It would be nice to update the JIRA description reflecting what is addressed.

I will update the Jira description.

amahussein · 2021-01-07T14:06:44Z

Thank for working on this, @amahussein. LGTM overall pending some nits.

While it is too late here, it is hard to follow which part of the TestDistributedShell is modified if splitting the class and refactoring are mixed up in a single commit. Doing one thing in one PR makes reviewing and cherry-picking easier.

Thanks for the review @iwasakims and @goiri !
Just quick point. Can you please use "start review" feature instead of sending separate comments? single comments do not indicate whether the reviewer is done with his reviews or not. Therefore, I could start a new commit before receiving all the feedback.
Thanks again guys. I understand that the PR was not straightforward to review.

hadoop-yetus · 2021-01-07T19:49:10Z

🎊 +1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	1m 16s		Docker mode activated.
			_ Prechecks _
+1 💚	dupname	0m 0s		No case conflicting files found.
+1 💚	@author	0m 0s		The patch does not contain any @author tags.
+1 💚		0m 0s	test4tests	The patch appears to include 7 new or modified test files.
			_ trunk Compile Tests _
+1 💚	mvninstall	35m 29s		trunk passed
+1 💚	compile	0m 28s		trunk passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.18.04
+1 💚	compile	0m 22s		trunk passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01
+1 💚	checkstyle	0m 23s		trunk passed
+1 💚	mvnsite	0m 26s		trunk passed
+1 💚	shadedclient	18m 17s		branch has no errors when building and testing our client artifacts.
+1 💚	javadoc	0m 23s		trunk passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.18.04
+1 💚	javadoc	0m 21s		trunk passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01
+0 🆗	spotbugs	0m 41s		Used deprecated FindBugs config; considering switching to SpotBugs.
+1 💚	findbugs	0m 39s		trunk passed
			_ Patch Compile Tests _
+1 💚	mvninstall	0m 23s		the patch passed
+1 💚	compile	0m 19s		the patch passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.18.04
+1 💚	javac	0m 19s		the patch passed
+1 💚	compile	0m 17s		the patch passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01
+1 💚	javac	0m 17s		the patch passed
+1 💚	checkstyle	0m 14s		hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-applications-distributedshell: The patch generated 0 new + 138 unchanged - 18 fixed = 138 total (was 156)
+1 💚	mvnsite	0m 19s		the patch passed
+1 💚	whitespace	0m 0s		The patch has no whitespace issues.
+1 💚	shadedclient	16m 48s		patch has no errors when building and testing our client artifacts.
+1 💚	javadoc	0m 18s		the patch passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.18.04
+1 💚	javadoc	0m 18s		the patch passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01
+1 💚	findbugs	0m 43s		the patch passed
			_ Other Tests _
+1 💚	unit	22m 8s		hadoop-yarn-applications-distributedshell in the patch passed.
+1 💚	asflicense	0m 30s		The patch does not generate ASF License warnings.
		102m 35s

Subsystem	Report/Notes
Docker	ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2581/5/artifact/out/Dockerfile
GITHUB PR	#2581
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle
uname	Linux a58770647bcd 4.15.0-128-generic #131-Ubuntu SMP Wed Dec 9 06:57:35 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/bin/hadoop.sh
git revision	trunk / `b612c31`
Default Java	Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01
Multi-JDK versions	/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.18.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01
Test Results	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2581/5/testReport/
Max. process+thread count	583 (vs. ulimit of 5500)
modules	C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-applications-distributedshell U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-applications-distributedshell
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2581/5/console
versions	git=2.17.1 maven=3.6.0 findbugs=4.0.6
Powered by	Apache Yetus 0.13.0-SNAPSHOT https://yetus.apache.org

This message was automatically generated.

iwasakims · 2021-01-08T03:22:49Z

The latest commit seemed to introduce errors.

$ mvn test -Dtest='TestDS*'
...
[ERROR] Errors:
[ERROR]   TestDSTimelineV20>DistributedShellBaseTest.setup:159->DistributedShellBaseTest.setupInternal:476 » YarnRuntime
[ERROR]   TestDSTimelineV20>DistributedShellBaseTest.setup:159->DistributedShellBaseTest.setupInternal:476 » YarnRuntime
[ERROR]   TestDSTimelineV20>DistributedShellBaseTest.setup:159->DistributedShellBaseTest.setupInternal:476 » YarnRuntime
[ERROR]   TestDSTimelineV20>DistributedShellBaseTest.setup:159->DistributedShellBaseTest.setupInternal:476 » YarnRuntime
[ERROR]   TestDSTimelineV20>DistributedShellBaseTest.setup:159->DistributedShellBaseTest.setupInternal:476 » YarnRuntime
[ERROR]   TestDSTimelineV20>DistributedShellBaseTest.setup:159->DistributedShellBaseTest.setupInternal:476 » YarnRuntime
[INFO]
[ERROR] Tests run: 40, Failures: 0, Errors: 6, Skipped: 0

amahussein · 2021-01-08T03:54:06Z

The latest commit seemed to introduce errors.

$ mvn test -Dtest='TestDS*'
...
[ERROR] Errors:
[ERROR]   TestDSTimelineV20>DistributedShellBaseTest.setup:159->DistributedShellBaseTest.setupInternal:476 » YarnRuntime
[ERROR]   TestDSTimelineV20>DistributedShellBaseTest.setup:159->DistributedShellBaseTest.setupInternal:476 » YarnRuntime
[ERROR]   TestDSTimelineV20>DistributedShellBaseTest.setup:159->DistributedShellBaseTest.setupInternal:476 » YarnRuntime
[ERROR]   TestDSTimelineV20>DistributedShellBaseTest.setup:159->DistributedShellBaseTest.setupInternal:476 » YarnRuntime
[ERROR]   TestDSTimelineV20>DistributedShellBaseTest.setup:159->DistributedShellBaseTest.setupInternal:476 » YarnRuntime
[ERROR]   TestDSTimelineV20>DistributedShellBaseTest.setup:159->DistributedShellBaseTest.setupInternal:476 » YarnRuntime
[INFO]
[ERROR] Tests run: 40, Failures: 0, Errors: 6, Skipped: 0

Hey @iwasakims!
I was looking into this failure. TestDSTimelineV20 fails when it runs after TestDSTimelineV15.
The exception is thrown as follows

	at org.apache.hadoop.yarn.server.timelineservice.storage.FileSystemTimelineWriterImpl.mkdirsWithRetries(FileSystemTimelineWriterImpl.java:214)
	at org.apache.hadoop.yarn.server.timelineservice.storage.FileSystemTimelineWriterImpl.serviceStart(FileSystemTimelineWriterImpl.java:188)

This could be something that missing in the configurations of the unitTest or a bug in either timelineserviceV1.5 or timelineserviceV2.0
I have verified that all the configurations are maintained when the code is split.
The reason this failure appears only now could be due to the fact that having all the tests in one file did not let V1.5 tests run immediately before V2.0 tests.

I haven't figured it yet, but probably this would need filing another jira.

amahussein · 2021-01-08T04:00:21Z

@iwasakims and @goiri Do you have insights what could the problem be ?
When you run TestDSTimelineV10 -> then TestDSTimelineV20. things will be successful.
Only TestDSTimelineV20 fails to initialize the RM if it runs immediately after TestDSTimelineV15

amahussein · 2021-01-08T04:06:26Z

I did an experiment on the original TestDistributedShell

if you run

mvn test -Dtest=TestDistributedShell#testDSShellWithoutDomainV1_5;
mvn test -Dtest=TestDistributedShell#testDSShellWithoutDomainV2

Then the testDSShellWithoutDomainV2 will fail with the same exception. It seems that there was an old bug and not introduced by my changes.

iwasakims · 2021-01-08T04:17:52Z

My stack trace indicates that TestDSTimelineV20 is configured to use HDFS but there is no running MiniDFSCluster. YARN daemons are failed to start due to this.

iwasakims · 2021-01-08T04:23:01Z

Running mvn test -Dtest=TestDSTimelineV20 alone worked as you said. TestDSTimelineV15 seems to affect the configuration of TestDSTimelineV20.

iwasakims · 2021-01-08T05:16:45Z

It seems that there was an old bug and not introduced by my changes.

It is easily surfaced by just running mvn test now as a result of the class splitting. This should be fixed here.

...test/java/org/apache/hadoop/yarn/applications/distributedshell/DistributedShellBaseTest.java

YARN-10553. fix checkstyles

hadoop-yetus · 2021-01-08T21:55:35Z

🎊 +1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 31s		Docker mode activated.
			_ Prechecks _
+1 💚	dupname	0m 0s		No case conflicting files found.
+1 💚	@author	0m 0s		The patch does not contain any @author tags.
+1 💚		0m 0s	test4tests	The patch appears to include 7 new or modified test files.
			_ trunk Compile Tests _
+1 💚	mvninstall	32m 41s		trunk passed
+1 💚	compile	0m 31s		trunk passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.18.04
+1 💚	compile	0m 28s		trunk passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01
+1 💚	checkstyle	0m 29s		trunk passed
+1 💚	mvnsite	0m 31s		trunk passed
+1 💚	shadedclient	16m 20s		branch has no errors when building and testing our client artifacts.
+1 💚	javadoc	0m 27s		trunk passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.18.04
+1 💚	javadoc	0m 25s		trunk passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01
+0 🆗	spotbugs	0m 47s		Used deprecated FindBugs config; considering switching to SpotBugs.
+1 💚	findbugs	0m 44s		trunk passed
			_ Patch Compile Tests _
+1 💚	mvninstall	0m 26s		the patch passed
+1 💚	compile	0m 21s		the patch passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.18.04
+1 💚	javac	0m 21s		the patch passed
+1 💚	compile	0m 19s		the patch passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01
+1 💚	javac	0m 19s		the patch passed
+1 💚	checkstyle	0m 17s		hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-applications-distributedshell: The patch generated 0 new + 138 unchanged - 18 fixed = 138 total (was 156)
+1 💚	mvnsite	0m 21s		the patch passed
+1 💚	whitespace	0m 0s		The patch has no whitespace issues.
+1 💚	shadedclient	14m 54s		patch has no errors when building and testing our client artifacts.
+1 💚	javadoc	0m 23s		the patch passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.18.04
+1 💚	javadoc	0m 22s		the patch passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01
+1 💚	findbugs	0m 44s		the patch passed
			_ Other Tests _
+1 💚	unit	22m 21s		hadoop-yarn-applications-distributedshell in the patch passed.
+1 💚	asflicense	0m 35s		The patch does not generate ASF License warnings.
		96m 36s

Subsystem	Report/Notes
Docker	ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2581/6/artifact/out/Dockerfile
GITHUB PR	#2581
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle
uname	Linux 2d824317a2ec 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/bin/hadoop.sh
git revision	trunk / `77435a0`
Default Java	Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01
Multi-JDK versions	/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.18.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01
Test Results	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2581/6/testReport/
Max. process+thread count	617 (vs. ulimit of 5500)
modules	C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-applications-distributedshell U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-applications-distributedshell
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2581/6/console
versions	git=2.17.1 maven=3.6.0 findbugs=4.0.6
Powered by	Apache Yetus 0.13.0-SNAPSHOT https://yetus.apache.org

This message was automatically generated.

iwasakims

Thanks for the update, @amahussein. +1.

amahussein · 2021-01-11T13:28:18Z

Thanks for the update, @amahussein. +1.

Thanks for the review @iwasakims and @goiri

(cherry picked from commit 890f2da) Conflicts: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-applications-distributedshell/src/test/java/org/apache/hadoop/yarn/applications/distributedshell/TestDSWithMultipleNodeManager.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-applications-distributedshell/src/test/java/org/apache/hadoop/yarn/applications/distributedshell/TestDistributedShell.java

goiri changed the title ~~YARN-10553. split TestDistributedShell~~ YARN-10553. Refactor TestDistributedShell Dec 31, 2020

goiri approved these changes Dec 31, 2020

View reviewed changes

jiwq reviewed Jan 5, 2021

View reviewed changes

amahussein force-pushed the yarn-10553 branch from 7f2977b to 1cecb0a Compare January 5, 2021 23:06

amahussein force-pushed the yarn-10553 branch from 1cecb0a to 93b17c9 Compare January 6, 2021 22:04

goiri reviewed Jan 6, 2021

View reviewed changes

...er/hadoop-yarn-server-tests/src/test/java/org/apache/hadoop/yarn/server/MiniYARNCluster.java Outdated Show resolved Hide resolved

iwasakims reviewed Jan 7, 2021

View reviewed changes

...c/test/java/org/apache/hadoop/yarn/applications/distributedshell/TestDSShellTimelineV10.java Outdated Show resolved Hide resolved

amahussein force-pushed the yarn-10553 branch from 93b17c9 to be01fb6 Compare January 7, 2021 04:55

iwasakims reviewed Jan 7, 2021

View reviewed changes

iwasakims reviewed Jan 8, 2021

View reviewed changes

...test/java/org/apache/hadoop/yarn/applications/distributedshell/DistributedShellBaseTest.java Outdated Show resolved Hide resolved

amahussein added 4 commits January 8, 2021 12:50

YARN-10553. split TestDistributedShell

8d13366

YARN-10553. fix checkstyles

YARN-10553. make abstract class and optimize setup

4835b31

YARN-10533. fix yarn-site reloading bug

0d94ca6

--amend

11df20a

amahussein force-pushed the yarn-10553 branch from 7430912 to 11df20a Compare January 8, 2021 20:17

iwasakims approved these changes Jan 8, 2021

View reviewed changes

iwasakims merged commit 890f2da into apache:trunk Jan 8, 2021

YARN-10553. Refactor TestDistributedShell #2581

YARN-10553. Refactor TestDistributedShell #2581

Uh oh!

Conversation

amahussein commented Dec 31, 2020

NOTICE

Uh oh!

hadoop-yetus commented Dec 31, 2020

Uh oh!

goiri Dec 31, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jiwq left a comment

Choose a reason for hiding this comment

Uh oh!

hadoop-yetus commented Jan 6, 2021

Uh oh!

goiri Jan 6, 2021

Choose a reason for hiding this comment

Uh oh!

amahussein Jan 6, 2021

Choose a reason for hiding this comment

Uh oh!

amahussein commented Jan 6, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

amahussein commented Jan 6, 2021

Uh oh!

goiri commented Jan 6, 2021

Uh oh!

Uh oh!

amahussein commented Jan 7, 2021

Uh oh!

hadoop-yetus commented Jan 7, 2021

Uh oh!

iwasakims commented Jan 7, 2021

Uh oh!

Uh oh!

hadoop-yetus commented Jan 7, 2021

Uh oh!

iwasakims Jan 7, 2021

Choose a reason for hiding this comment

Uh oh!

iwasakims Jan 7, 2021

Choose a reason for hiding this comment

Uh oh!

iwasakims commented Jan 7, 2021

Uh oh!

iwasakims commented Jan 7, 2021

Uh oh!

amahussein commented Jan 7, 2021

Uh oh!

amahussein commented Jan 7, 2021

Uh oh!

amahussein commented Jan 7, 2021

Uh oh!

hadoop-yetus commented Jan 7, 2021

Uh oh!

iwasakims commented Jan 8, 2021

Uh oh!

amahussein commented Jan 8, 2021

Uh oh!

amahussein commented Jan 8, 2021

Uh oh!

amahussein commented Jan 8, 2021

Uh oh!

iwasakims commented Jan 8, 2021

Uh oh!

iwasakims commented Jan 8, 2021

Uh oh!

iwasakims commented Jan 8, 2021

Uh oh!

Uh oh!

hadoop-yetus commented Jan 8, 2021

Uh oh!

iwasakims left a comment

amahussein commented Jan 6, 2021 •

edited

Loading