HDFS-16957. RBF: Exit status of dfsrouteradmin -rm should be non-zero for unsuccessful attempt #5487

virajjasani · 2023-03-16T22:43:07Z

DFS router admin returns non-zero status code for unsuccessful attempt to add or update mount point. However, same is not the case with removal of mount point.

For instance,

bin/hdfs dfsrouteradmin -add /data4 ns1 /data4
..
..

Cannot add destination at ns1 /data4


echo $?
255

bin/hdfs dfsrouteradmin -rm /data4
..
..
Cannot remove mount point /data4


echo $?
0

Removal of mount point should stay consistent with other options and return non-zero (unsuccessful) status code.

… for unsuccessful attempt

hadoop-yetus · 2023-03-17T00:42:10Z

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 36s		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 _
+1 💚	mvninstall	41m 12s		trunk passed
+1 💚	compile	0m 45s		trunk passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1
+1 💚	compile	0m 40s		trunk passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09
+1 💚	checkstyle	0m 35s		trunk passed
+1 💚	mvnsite	0m 45s		trunk passed
+1 💚	javadoc	0m 52s		trunk passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1
+1 💚	javadoc	0m 59s		trunk passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09
+1 💚	spotbugs	1m 35s		trunk passed
+1 💚	shadedclient	20m 34s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+1 💚	mvninstall	0m 34s		the patch passed
+1 💚	compile	0m 36s		the patch passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1
+1 💚	javac	0m 36s		the patch passed
+1 💚	compile	0m 32s		the patch passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09
+1 💚	javac	0m 32s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
+1 💚	checkstyle	0m 18s		the patch passed
+1 💚	mvnsite	0m 35s		the patch passed
+1 💚	javadoc	0m 33s		the patch passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1
+1 💚	javadoc	0m 52s		the patch passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09
+1 💚	spotbugs	1m 21s		the patch passed
+1 💚	shadedclient	20m 30s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
-1 ❌	unit	20m 44s	/patch-unit-hadoop-hdfs-project_hadoop-hdfs-rbf.txt	hadoop-hdfs-rbf in the patch passed.
+1 💚	asflicense	0m 38s		The patch does not generate ASF License warnings.
		117m 40s

Reason	Tests
Failed junit tests	hadoop.hdfs.server.federation.router.TestRouterRPCMultipleDestinationMountTableResolver

Subsystem	Report/Notes
Docker	ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5487/1/artifact/out/Dockerfile
GITHUB PR	#5487
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets
uname	Linux ac3115c14446 4.15.0-206-generic #217-Ubuntu SMP Fri Feb 3 19:10:13 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/bin/hadoop.sh
git revision	trunk / `0bc696c`
Default Java	Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09
Multi-JDK versions	/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09
Test Results	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5487/1/testReport/
Max. process+thread count	2544 (vs. ulimit of 5500)
modules	C: hadoop-hdfs-project/hadoop-hdfs-rbf U: hadoop-hdfs-project/hadoop-hdfs-rbf
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5487/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 · 2023-03-17T00:45:46Z

All tests in TestRouterRPCMultipleDestinationMountTableResolver are passing locally

ayushtkn

I think this was raised somewhere in past as well but not done considering incompatible

hadoop-yetus · 2023-03-17T01:48:49Z

🎊 +1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 47s		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	44m 5s		trunk passed
+1 💚	compile	0m 48s		trunk passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1
+1 💚	compile	0m 36s		trunk passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09
+1 💚	checkstyle	0m 31s		trunk passed
+1 💚	mvnsite	0m 42s		trunk passed
+1 💚	javadoc	0m 47s		trunk passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1
+1 💚	javadoc	0m 56s		trunk passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09
+1 💚	spotbugs	1m 33s		trunk passed
+1 💚	shadedclient	24m 21s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+1 💚	mvninstall	0m 32s		the patch passed
+1 💚	compile	0m 36s		the patch passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1
+1 💚	javac	0m 36s		the patch passed
+1 💚	compile	0m 30s		the patch passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09
+1 💚	javac	0m 30s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
+1 💚	checkstyle	0m 16s		the patch passed
+1 💚	mvnsite	0m 33s		the patch passed
+1 💚	javadoc	0m 32s		the patch passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1
+1 💚	javadoc	0m 49s		the patch passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09
+1 💚	spotbugs	1m 21s		the patch passed
+1 💚	shadedclient	24m 9s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
+1 💚	unit	20m 46s		hadoop-hdfs-rbf in the patch passed.
+1 💚	asflicense	0m 34s		The patch does not generate ASF License warnings.
		126m 59s

Subsystem	Report/Notes
Docker	ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5487/2/artifact/out/Dockerfile
GITHUB PR	#5487
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets
uname	Linux c30fbbcd3c76 4.15.0-206-generic #217-Ubuntu SMP Fri Feb 3 19:10:13 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/bin/hadoop.sh
git revision	trunk / `b5b3c23`
Default Java	Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09
Multi-JDK versions	/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09
Test Results	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5487/2/testReport/
Max. process+thread count	2070 (vs. ulimit of 5500)
modules	C: hadoop-hdfs-project/hadoop-hdfs-rbf U: hadoop-hdfs-project/hadoop-hdfs-rbf
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5487/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 · 2023-03-17T02:42:04Z

I think this was raised somewhere in past as well but not done considering incompatible

I thought so too, that's why wanted to check on the Jira :)

virajjasani · 2023-03-17T05:14:08Z

@ayushtkn @goiri
I just looked at the function where the changes are made in the PR and realized that other options -setQuota, -setStorageTypeQuota, -clrQuota and -clrStorageTypeQuota also have the same problem and they are also not consistent. I have not tested these options yet but with a quick glance, I feel they also are likely having the same issue.

Would you recommend fixing them all once and for all on trunk (3.4.0) only with this PR? So that at least with 3.4.0 onwards, we have consistent behavior?

ayushtkn · 2023-03-17T05:31:05Z

yep, that is something known to me, it is there with almost all commands, and we had a discussion around it long back and it was kind of: 'Changing exit code is incompatible', remember discussing internally long back as well and the conclusion was people who have internal test scripts and code they heavily rely on these exit code(we were one of those then), so, it got dropped.

Would you recommend fixing them all once and for all on trunk (3.4.0)

I won't say yes, have always been part of the gang or say I come from that era which was strong believer and follower of compat guidelines. Not sure how people are dealing nowadays though :(

see this ex., just a minor CLI change which in all ways is good very useful got reverted just because of compat from all branches. that is some fun to follow
https://issues.apache.org/jira/browse/HDFS-13732

virajjasani · 2023-03-17T05:38:34Z

Ah yes, not having compatibility is a pain for sure :(
The only problem though is, on one hand we have compatibility to maintain, and on the other, inconsistent behavior among different options of dfsrouteradmin.

Any decision is fine though :)

goiri · 2023-03-17T18:57:24Z

Do we have this exit code output being 0 in 3.3.X?
If it hasn't been released it is fine but I suspect it has.

goiri · 2023-03-17T18:58:51Z

I have to say that I'm even willing to break our own "compatibility" given that we are breaking compatibility with the rest of the world that assumes that a failed command should be -1.

ayushtkn · 2023-03-17T19:57:06Z

Do we have this exit code output being 0 in 3.3.X?
If it hasn't been released it is fine but I suspect it has.

It is there since very starting, even in branch-2.9
https://github.com/apache/hadoop/blob/branch-2.9/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/tools/federation/RouterAdmin.java#L182-L185

There ain't any release line which has not released this behaviour :)

virajjasani · 2023-03-17T20:32:37Z

Do we have this exit code output being 0 in 3.3.X?
If it hasn't been released it is fine but I suspect it has.

It is there since very starting, even in branch-2.9 https://github.com/apache/hadoop/blob/branch-2.9/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/tools/federation/RouterAdmin.java#L182-L185

There ain't any release line which has not released this behaviour :)

On the above reference of branch-2.9, though even -add and -update are of same behavior. That means all commands are exiting with status code 0 even for failures, so while not intended behavior, at least they all are consistent in that sense.

Whereas right now on 3.3, -add and -update have different behavior (which is correct with outside world) than -rm etc, which is inconsistent already. It's like some of router admin CLI options have correct behavior from outside world viewpoint but others don't.

virajjasani · 2023-03-17T20:35:28Z

Looks like this is the commit that has changed behavior for -add and -update 9315db5
(HDFS-13815)

ayushtkn · 2023-03-17T20:47:44Z

Hmm, that changed something wrapped with the bug. There failure was claimed as successful or something like that….

But this behaviour including that also is there since branch-3.1, should be in almost all 3.2.x and 3.3.x

no strong opinions here, slightly more inclined to stay with compat, for one reason 3.3.x has this behaviour and changing this is just peace to eyes but would break some test scripts out of which I too was part long back

virajjasani added 2 commits March 16, 2023 15:40

HDFS-16957. RBF: Exit status of dfsrouteradmin -rm should be non-zero…

0bc696c

… for unsuccessful attempt

addendum

b5b3c23

ayushtkn reviewed Mar 17, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HDFS-16957. RBF: Exit status of dfsrouteradmin -rm should be non-zero for unsuccessful attempt #5487

HDFS-16957. RBF: Exit status of dfsrouteradmin -rm should be non-zero for unsuccessful attempt #5487

virajjasani commented Mar 16, 2023

hadoop-yetus commented Mar 17, 2023

virajjasani commented Mar 17, 2023

ayushtkn left a comment

hadoop-yetus commented Mar 17, 2023

virajjasani commented Mar 17, 2023

virajjasani commented Mar 17, 2023 •

edited

Loading

ayushtkn commented Mar 17, 2023

virajjasani commented Mar 17, 2023

goiri commented Mar 17, 2023

goiri commented Mar 17, 2023

ayushtkn commented Mar 17, 2023

virajjasani commented Mar 17, 2023

virajjasani commented Mar 17, 2023 •

edited

Loading

ayushtkn commented Mar 17, 2023 •

edited

Loading

HDFS-16957. RBF: Exit status of dfsrouteradmin -rm should be non-zero for unsuccessful attempt #5487

Are you sure you want to change the base?

HDFS-16957. RBF: Exit status of dfsrouteradmin -rm should be non-zero for unsuccessful attempt #5487

Conversation

virajjasani commented Mar 16, 2023

hadoop-yetus commented Mar 17, 2023

virajjasani commented Mar 17, 2023

ayushtkn left a comment

Choose a reason for hiding this comment

hadoop-yetus commented Mar 17, 2023

virajjasani commented Mar 17, 2023

virajjasani commented Mar 17, 2023 • edited Loading

ayushtkn commented Mar 17, 2023

virajjasani commented Mar 17, 2023

goiri commented Mar 17, 2023

goiri commented Mar 17, 2023

ayushtkn commented Mar 17, 2023

virajjasani commented Mar 17, 2023

virajjasani commented Mar 17, 2023 • edited Loading

ayushtkn commented Mar 17, 2023 • edited Loading

virajjasani commented Mar 17, 2023 •

edited

Loading

virajjasani commented Mar 17, 2023 •

edited

Loading

ayushtkn commented Mar 17, 2023 •

edited

Loading