HDFS-16702. MiniDFSCluster should report cause of exception in assertion error #4671

virajjasani · 2022-08-02T00:41:26Z

Description of PR

MiniDFSCluster should report cause of exception in assertion error
Improve message of ExitException to include cause

…ion error

hadoop-yetus · 2022-08-02T08:49:20Z

🎊 +1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 46s		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 _
+0 🆗	mvndep	15m 18s		Maven dependency ordering for branch
+1 💚	mvninstall	25m 33s		trunk passed
+1 💚	compile	23m 25s		trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚	compile	23m 16s		trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	checkstyle	3m 55s		trunk passed
+1 💚	mvnsite	3m 26s		trunk passed
+1 💚	javadoc	2m 18s		trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚	javadoc	2m 27s		trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	spotbugs	6m 7s		trunk passed
+1 💚	shadedclient	24m 29s		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	2m 18s		the patch passed
+1 💚	compile	24m 19s		the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚	javac	24m 19s		the patch passed
+1 💚	compile	21m 55s		the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	javac	21m 55s		the patch passed
+1 💚	blanks	0m 1s		The patch has no blanks issues.
+1 💚	checkstyle	3m 36s		the patch passed
+1 💚	mvnsite	3m 20s		the patch passed
+1 💚	javadoc	2m 9s		the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚	javadoc	2m 22s		the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	spotbugs	6m 26s		the patch passed
+1 💚	shadedclient	23m 34s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
+1 💚	unit	18m 37s		hadoop-common in the patch passed.
+1 💚	unit	245m 34s		hadoop-hdfs in the patch passed.
+1 💚	asflicense	1m 17s		The patch does not generate ASF License warnings.
		486m 33s

Subsystem	Report/Notes
Docker	ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4671/1/artifact/out/Dockerfile
GITHUB PR	#4671
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets
uname	Linux a39be1242bf9 4.15.0-169-generic #177-Ubuntu SMP Thu Feb 3 10:50:38 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/bin/hadoop.sh
git revision	trunk / `00eacc4`
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-4671/1/testReport/
Max. process+thread count	3775 (vs. ulimit of 5500)
modules	C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs U: .
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4671/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-08-02T21:35:37Z

@ayushtkn @snmvaughan could you please review this PR?

ayushtkn · 2022-08-02T21:47:34Z

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/ExitUtil.java

-      return Integer.toString(status) + ": " + message;
+      String finalString = status + ": " + message;
+      Throwable cause = getCause();
+      if (cause != null) {
+        finalString = finalString + " : \n" + "cause: " + cause.getMessage();
+      }
+      return finalString;


Are we changing the response string, I am not sure about the compatibility here.
The above javadoc says

String value does not include exception type, just exit code and message.

We shouldn't make prod level API changes for MiniDFSCluster

just exit code and message

In a way, we are just improving the message by also including cause message if any cause is associated. I just thought it would be good improvement. We are still not including exception type even with this patch as the Javadoc claims. Only if we print the cause itself, then we would have included exception type. WDYT?

But if you still think we should not include cause.getMessage() in addition to this.getMessage(), I can remove it. It's just that we would be able to include one more layer of message to the existing message if we can include cause.getMessage() as well, just a minor improvement.

snmvaughan · 2022-08-03T01:28:08Z

The original identified problem wasn't about the message of the thrown error, but the lack of a stack trace as part of that exception.

snmvaughan · 2022-08-03T01:31:01Z

     LOG.info("Shutting down the Mini HDFS Cluster");
     if (checkExitOnShutdown)  {
       if (ExitUtil.terminateCalled()) {
-        LOG.error("Test resulted in an unexpected exit",
-            ExitUtil.getFirstExitException());
+        Exception cause = ExitUtil.getFirstExitException();
+        LOG.error("Test resulted in an unexpected exit", cause);
         ExitUtil.resetFirstExitException();
-        throw new AssertionError("Test resulted in an unexpected exit");
+        throw new AssertionError("Test resulted in an unexpected exit", cause);
       }
     }
     if (closeFileSystem) {

virajjasani · 2022-08-03T01:31:03Z

I understand, but I would like to know if improving the toString() is useful. If no, I will remove it from this PR. If yes, and if it should be done separately, we can do that as well.

@ayushtkn @snmvaughan thoughts?

snmvaughan · 2022-08-03T01:33:31Z

The updated message looks like it could be useful.

ayushtkn · 2022-08-03T07:42:22Z

This message change and stuff I doubt can land up with compatibility issues if some downstream or so is asserting the string output and then this will lead to too much of noise.
No doubt the change to the toString() is good, but better separate it in another Jira, and we can think more about that there..

…n assertion error" This reverts commit 00eacc4.

virajjasani · 2022-08-03T17:52:14Z

Sounds reasonable @ayushtkn. Updated the PR.

ayushtkn · 2022-08-03T18:09:35Z

Hmm, there are two PR now linked to the Jira and both to trunk? @snmvaughan / @virajjasani
Both seems identical, can you folks figure out

virajjasani · 2022-08-03T18:44:18Z

Hmm, there are two PR now linked to the Jira and both to trunk?

Ah, didn't realize that. @snmvaughan you can get your PR merged if you would like, I just felt taking this up only because the Jira was not assigned to you by the time I raised PR, but anything is fine. All upto you :)

hadoop-yetus · 2022-08-04T04:55:11Z

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	1m 10s		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 appears to include 1 new or modified test files.
			_ trunk Compile Tests _
+0 🆗	mvndep	15m 47s		Maven dependency ordering for branch
+1 💚	mvninstall	28m 33s		trunk passed
+1 💚	compile	25m 28s		trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚	compile	22m 9s		trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	checkstyle	4m 32s		trunk passed
+1 💚	mvnsite	4m 5s		trunk passed
+1 💚	javadoc	3m 4s		trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚	javadoc	3m 7s		trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	spotbugs	7m 2s		trunk passed
+1 💚	shadedclient	27m 18s		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	2m 28s		the patch passed
+1 💚	compile	24m 43s		the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚	javac	24m 43s		the patch passed
+1 💚	compile	22m 42s		the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	javac	22m 42s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
+1 💚	checkstyle	4m 37s		the patch passed
+1 💚	mvnsite	4m 23s		the patch passed
+1 💚	javadoc	2m 59s		the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚	javadoc	3m 13s		the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	spotbugs	7m 19s		the patch passed
+1 💚	shadedclient	28m 28s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
+1 💚	unit	20m 44s		hadoop-common in the patch passed.
-1 ❌	unit	396m 41s	/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt	hadoop-hdfs in the patch passed.
+1 💚	asflicense	1m 34s		The patch does not generate ASF License warnings.
		662m 19s

Reason	Tests
Failed junit tests	hadoop.hdfs.server.namenode.ha.TestSeveralNameNodes

Subsystem	Report/Notes
Docker	ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4671/2/artifact/out/Dockerfile
GITHUB PR	#4671
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets
uname	Linux a2c2eaa1f5f5 4.15.0-175-generic #184-Ubuntu SMP Thu Mar 24 17:48:36 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/bin/hadoop.sh
git revision	trunk / `48302ed`
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-4671/2/testReport/
Max. process+thread count	1981 (vs. ulimit of 5500)
modules	C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs U: .
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4671/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.

HDFS-16702. MiniDFSCluster should report cause of exception in assert…

00eacc4

…ion error

ayushtkn reviewed Aug 2, 2022

View reviewed changes

virajjasani added 2 commits August 3, 2022 10:49

Revert "HDFS-16702. MiniDFSCluster should report cause of exception i…

d4e5d9a

…n assertion error" This reverts commit 00eacc4.

addressing review - remove ExitUtil changes

48302ed

virajjasani closed this Aug 5, 2022

virajjasani deleted the HDFS-16702-trunk branch August 5, 2022 02:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HDFS-16702. MiniDFSCluster should report cause of exception in assertion error #4671

HDFS-16702. MiniDFSCluster should report cause of exception in assertion error #4671

virajjasani commented Aug 2, 2022

hadoop-yetus commented Aug 2, 2022

virajjasani commented Aug 2, 2022

ayushtkn Aug 2, 2022

virajjasani Aug 2, 2022 •

edited

Loading

virajjasani Aug 2, 2022

snmvaughan commented Aug 3, 2022

snmvaughan commented Aug 3, 2022 •

edited

Loading

virajjasani commented Aug 3, 2022

snmvaughan commented Aug 3, 2022

ayushtkn commented Aug 3, 2022

virajjasani commented Aug 3, 2022

ayushtkn commented Aug 3, 2022

virajjasani commented Aug 3, 2022

hadoop-yetus commented Aug 4, 2022

HDFS-16702. MiniDFSCluster should report cause of exception in assertion error #4671

HDFS-16702. MiniDFSCluster should report cause of exception in assertion error #4671

Conversation

virajjasani commented Aug 2, 2022

Description of PR

hadoop-yetus commented Aug 2, 2022

virajjasani commented Aug 2, 2022

ayushtkn Aug 2, 2022

Choose a reason for hiding this comment

virajjasani Aug 2, 2022 • edited Loading

Choose a reason for hiding this comment

virajjasani Aug 2, 2022

Choose a reason for hiding this comment

snmvaughan commented Aug 3, 2022

snmvaughan commented Aug 3, 2022 • edited Loading

virajjasani commented Aug 3, 2022

snmvaughan commented Aug 3, 2022

ayushtkn commented Aug 3, 2022

virajjasani commented Aug 3, 2022

ayushtkn commented Aug 3, 2022

virajjasani commented Aug 3, 2022

hadoop-yetus commented Aug 4, 2022

virajjasani Aug 2, 2022 •

edited

Loading

snmvaughan commented Aug 3, 2022 •

edited

Loading