TEZ-4561: Improve reported exception when DAGAppMaster is shutting down. by ayushtkn · Pull Request #365 · apache/tez

ayushtkn · 2024-08-16T08:24:44Z

No description provided.

ayushtkn · 2024-08-16T09:49:13Z

I think we can live with this checkstyle warning, creating an accessor method looks like an overkill to me

ayushtkn · 2024-08-16T10:44:25Z

Added a test.
Sharing the output of exception
When State is INITED

org.apache.tez.dag.api.TezUncheckedException: Cannot get ApplicationACLs before all services have started, The current service state is INITED

When State is STOPPED

org.apache.tez.dag.api.TezUncheckedException: Cannot get ApplicationACLs before all services have started, The current service state is STOPPED. The shutdown hook started at Fri Aug 16 16:08:09 IST 2024

tez-yetus · 2024-08-16T15:07:48Z

🎊 +1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	0m 30s	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 💚	test4tests	0m 0s	The patch appears to include 1 new or modified test files.
		_ master Compile Tests _
+1 💚	mvninstall	16m 24s	master passed
+1 💚	compile	0m 43s	master passed with JDK Ubuntu-11.0.24+8-post-Ubuntu-1ubuntu322.04
+1 💚	compile	0m 38s	master passed with JDK Private Build-1.8.0_422-8u422-b05-1~22.04-b05
+1 💚	checkstyle	1m 30s	master passed
+1 💚	javadoc	0m 37s	master passed with JDK Ubuntu-11.0.24+8-post-Ubuntu-1ubuntu322.04
+1 💚	javadoc	0m 21s	master passed with JDK Private Build-1.8.0_422-8u422-b05-1~22.04-b05
+0 🆗	spotbugs	1m 51s	Used deprecated FindBugs config; considering switching to SpotBugs.
+1 💚	findbugs	1m 48s	master passed
		_ Patch Compile Tests _
+1 💚	mvninstall	0m 28s	the patch passed
+1 💚	compile	0m 31s	the patch passed with JDK Ubuntu-11.0.24+8-post-Ubuntu-1ubuntu322.04
+1 💚	javac	0m 31s	the patch passed
+1 💚	compile	0m 26s	the patch passed with JDK Private Build-1.8.0_422-8u422-b05-1~22.04-b05
+1 💚	javac	0m 26s	the patch passed
-0 ⚠️	checkstyle	0m 21s	tez-dag: The patch generated 1 new + 57 unchanged - 3 fixed = 58 total (was 60)
+1 💚	whitespace	0m 0s	The patch has no whitespace issues.
+1 💚	javadoc	0m 10s	the patch passed with JDK Ubuntu-11.0.24+8-post-Ubuntu-1ubuntu322.04
+1 💚	javadoc	0m 10s	the patch passed with JDK Private Build-1.8.0_422-8u422-b05-1~22.04-b05
+1 💚	findbugs	1m 14s	the patch passed
		_ Other Tests _
+1 💚	unit	5m 8s	tez-dag in the patch passed.
+1 💚	asflicense	0m 17s	The patch does not generate ASF License warnings.
		32m 16s

Subsystem	Report/Notes
Docker	ClientAPI=1.44 ServerAPI=1.44 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-365/3/artifact/out/Dockerfile
GITHUB PR	#365
JIRA Issue	TEZ-4561
Optional Tests	dupname asflicense javac javadoc unit spotbugs findbugs checkstyle compile
uname	Linux 06ebcb967f9f 5.15.0-94-generic #104-Ubuntu SMP Tue Jan 9 15:25:40 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	personality/tez.sh
git revision	master / `cd6ceec`
Default Java	Private Build-1.8.0_422-8u422-b05-1~22.04-b05
Multi-JDK versions	/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.24+8-post-Ubuntu-1ubuntu322.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_422-8u422-b05-1~22.04-b05
checkstyle	https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-365/3/artifact/out/diff-checkstyle-tez-dag.txt
Test Results	https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-365/3/testReport/
Max. process+thread count	222 (vs. ulimit of 5500)
modules	C: tez-dag U: tez-dag
Console output	https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-365/3/console
versions	git=2.34.1 maven=3.6.3 findbugs=3.0.1
Powered by	Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

abstractdog · 2024-08-27T16:07:49Z


+ private String getShutdownTimeString() {
+ if (shutdownHandler != null && shutdownHandler.shutdownTime != null) {
+ return ". The shutdown hook started at " + shutdownHandler.shutdownTime;


It's strange that this separate string generation method takes care of the closing point of the previous sentence, I believe that should go back to getApplicationACLs

I moved the "." back to the {{getApplicationACLs}}

tez-yetus · 2024-08-28T08:27:12Z

🎊 +1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	27m 8s	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 💚	test4tests	0m 0s	The patch appears to include 1 new or modified test files.
		_ master Compile Tests _
+1 💚	mvninstall	16m 59s	master passed
+1 💚	compile	0m 44s	master passed with JDK Ubuntu-11.0.24+8-post-Ubuntu-1ubuntu322.04
+1 💚	compile	0m 39s	master passed with JDK Private Build-1.8.0_422-8u422-b05-1~22.04-b05
+1 💚	checkstyle	1m 31s	master passed
+1 💚	javadoc	0m 36s	master passed with JDK Ubuntu-11.0.24+8-post-Ubuntu-1ubuntu322.04
+1 💚	javadoc	0m 23s	master passed with JDK Private Build-1.8.0_422-8u422-b05-1~22.04-b05
+0 🆗	spotbugs	1m 53s	Used deprecated FindBugs config; considering switching to SpotBugs.
+1 💚	findbugs	1m 51s	master passed
		_ Patch Compile Tests _
+1 💚	mvninstall	0m 28s	the patch passed
+1 💚	compile	0m 31s	the patch passed with JDK Ubuntu-11.0.24+8-post-Ubuntu-1ubuntu322.04
+1 💚	javac	0m 31s	the patch passed
+1 💚	compile	0m 27s	the patch passed with JDK Private Build-1.8.0_422-8u422-b05-1~22.04-b05
+1 💚	javac	0m 27s	the patch passed
-0 ⚠️	checkstyle	0m 20s	tez-dag: The patch generated 1 new + 57 unchanged - 3 fixed = 58 total (was 60)
+1 💚	whitespace	0m 0s	The patch has no whitespace issues.
+1 💚	javadoc	0m 10s	the patch passed with JDK Ubuntu-11.0.24+8-post-Ubuntu-1ubuntu322.04
+1 💚	javadoc	0m 10s	the patch passed with JDK Private Build-1.8.0_422-8u422-b05-1~22.04-b05
+1 💚	findbugs	1m 15s	the patch passed
		_ Other Tests _
+1 💚	unit	5m 6s	tez-dag in the patch passed.
+1 💚	asflicense	0m 17s	The patch does not generate ASF License warnings.
		59m 40s

Subsystem	Report/Notes
Docker	ClientAPI=1.47 ServerAPI=1.47 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-365/4/artifact/out/Dockerfile
GITHUB PR	#365
JIRA Issue	TEZ-4561
Optional Tests	dupname asflicense javac javadoc unit spotbugs findbugs checkstyle compile
uname	Linux 249eda176fba 5.15.0-119-generic #129-Ubuntu SMP Fri Aug 2 19:25:20 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	personality/tez.sh
git revision	master / `174d4e3`
Default Java	Private Build-1.8.0_422-8u422-b05-1~22.04-b05
Multi-JDK versions	/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.24+8-post-Ubuntu-1ubuntu322.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_422-8u422-b05-1~22.04-b05
checkstyle	https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-365/4/artifact/out/diff-checkstyle-tez-dag.txt
Test Results	https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-365/4/testReport/
Max. process+thread count	205 (vs. ulimit of 5500)
modules	C: tez-dag U: tez-dag
Console output	https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-365/4/console
versions	git=2.34.1 maven=3.6.3 findbugs=3.0.1
Powered by	Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

abstractdog · 2024-08-28T11:05:46Z

nit: what about changing shutdownTime to private?
not necessarily because of the checkstyle warning (private + accessor methods), but instead to keep it inline with other fields of DAGAppMasterShutdownHandler, like:

 private AtomicBoolean shutdownHandled = new AtomicBoolean(false);
private long sleepTimeBeforeExit = TezConstants.TEZ_DAG_SLEEP_TIME_BEFORE_EXIT;

I guess the usage scope of those are similar

tez-yetus · 2024-08-28T16:44:13Z

🎊 +1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	0m 34s	Docker mode activated.
		_ Prechecks _
+1 💚	dupname	0m 1s	No case conflicting files found.
+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.
		_ master Compile Tests _
+1 💚	mvninstall	19m 8s	master passed
+1 💚	compile	0m 45s	master passed with JDK Ubuntu-11.0.24+8-post-Ubuntu-1ubuntu322.04
+1 💚	compile	0m 40s	master passed with JDK Private Build-1.8.0_422-8u422-b05-1~22.04-b05
+1 💚	checkstyle	1m 42s	master passed
+1 💚	javadoc	0m 40s	master passed with JDK Ubuntu-11.0.24+8-post-Ubuntu-1ubuntu322.04
+1 💚	javadoc	0m 23s	master passed with JDK Private Build-1.8.0_422-8u422-b05-1~22.04-b05
+0 🆗	spotbugs	2m 0s	Used deprecated FindBugs config; considering switching to SpotBugs.
+1 💚	findbugs	1m 58s	master passed
		_ Patch Compile Tests _
+1 💚	mvninstall	0m 27s	the patch passed
+1 💚	compile	0m 32s	the patch passed with JDK Ubuntu-11.0.24+8-post-Ubuntu-1ubuntu322.04
+1 💚	javac	0m 32s	the patch passed
+1 💚	compile	0m 26s	the patch passed with JDK Private Build-1.8.0_422-8u422-b05-1~22.04-b05
+1 💚	javac	0m 26s	the patch passed
+1 💚	checkstyle	0m 21s	tez-dag: The patch generated 0 new + 57 unchanged - 3 fixed = 57 total (was 60)
+1 💚	whitespace	0m 0s	The patch has no whitespace issues.
+1 💚	javadoc	0m 10s	the patch passed with JDK Ubuntu-11.0.24+8-post-Ubuntu-1ubuntu322.04
+1 💚	javadoc	0m 9s	the patch passed with JDK Private Build-1.8.0_422-8u422-b05-1~22.04-b05
+1 💚	findbugs	1m 14s	the patch passed
		_ Other Tests _
+1 💚	unit	5m 7s	tez-dag in the patch passed.
+1 💚	asflicense	0m 19s	The patch does not generate ASF License warnings.
		35m 42s

Subsystem	Report/Notes
Docker	ClientAPI=1.47 ServerAPI=1.47 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-365/6/artifact/out/Dockerfile
GITHUB PR	#365
JIRA Issue	TEZ-4561
Optional Tests	dupname asflicense javac javadoc unit spotbugs findbugs checkstyle compile
uname	Linux 125dfebe230f 5.15.0-119-generic #129-Ubuntu SMP Fri Aug 2 19:25:20 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	personality/tez.sh
git revision	master / `174d4e3`
Default Java	Private Build-1.8.0_422-8u422-b05-1~22.04-b05
Multi-JDK versions	/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.24+8-post-Ubuntu-1ubuntu322.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_422-8u422-b05-1~22.04-b05
Test Results	https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-365/6/testReport/
Max. process+thread count	233 (vs. ulimit of 5500)
modules	C: tez-dag U: tez-dag
Console output	https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-365/6/console
versions	git=2.34.1 maven=3.6.3 findbugs=3.0.1
Powered by	Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

ayushtkn · 2024-08-29T05:36:43Z

@abstractdog I have changed the variable to private & used setter & getter, there was some FindBugs warning, which I have dodged, lemme know if things look good

abstractdog

+1

TEZ-4561: Improve reported exception when DAGAppMaster is shutting down.
00a4dfa

This comment was marked as outdated.
Sign in to view

Add Test Case.
115fc4b

This comment was marked as outdated.
Sign in to view

Fix Checkstyle.
616981d

ayushtkn requested a review from abstractdog August 21, 2024 14:33

abstractdog reviewed Aug 27, 2024
View reviewed changes

Move FullStop Above.
9dc1b51

Use setter & getter.
1790462

This comment was marked as outdated.
Sign in to view

FindBugs
fa43d37

abstractdog self-requested a review September 3, 2024 09:57

abstractdog approved these changes Sep 3, 2024
View reviewed changes

ayushtkn merged commit 76490d4 into apache:masterSep 3, 2024

Navigation Menu

Uh oh!

TEZ-4561: Improve reported exception when DAGAppMaster is shutting down. - #365

TEZ-4561: Improve reported exception when DAGAppMaster is shutting down.#365
ayushtkn merged 6 commits into
apache:masterfrom
ayushtkn:TEZ-4561

ayushtkn commented Aug 16, 2024

Uh oh!

This comment was marked as outdated.

ayushtkn commented Aug 16, 2024

Uh oh!

ayushtkn commented Aug 16, 2024

Uh oh!

This comment was marked as outdated.

tez-yetus commented Aug 16, 2024

Uh oh!

abstractdogAug 27, 2024

Uh oh!

ayushtknAug 28, 2024

Uh oh!

tez-yetus commented Aug 28, 2024

Uh oh!

abstractdog commented Aug 28, 2024

Uh oh!

This comment was marked as outdated.

tez-yetus commented Aug 28, 2024

Uh oh!

ayushtkn commented Aug 29, 2024

Uh oh!

abstractdog left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

ayushtkn commented Aug 16, 2024

Uh oh!

This comment was marked as outdated.

ayushtkn commented Aug 16, 2024

Uh oh!

ayushtkn commented Aug 16, 2024

Uh oh!

This comment was marked as outdated.

tez-yetus commented Aug 16, 2024

Uh oh!

abstractdogAug 27, 2024

Choose a reason for hiding this comment

Uh oh!

ayushtknAug 28, 2024

Choose a reason for hiding this comment

Uh oh!

tez-yetus commented Aug 28, 2024

Uh oh!

abstractdog commented Aug 28, 2024

Uh oh!

This comment was marked as outdated.

tez-yetus commented Aug 28, 2024

Uh oh!

ayushtkn commented Aug 29, 2024

Uh oh!

abstractdog left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants