HADOOP-18327.Fix eval expression in hadoop-functions.sh

[ashutoshcipher]

Description of PR

Need to fix the eval expression.

1. Prefix exec by eval in Hadoop bin scripts Prior to this change, if HADOOP_OPTS contains any arguments that include a space, the command is not parsed correctly. For example, if HADOOP_OPTS="... -XX:OnOutOfMemoryError="kill -9 %p" ...", the bin/hadoop script will fail with the error "Unrecognized option: -9". No amount of clever escaping of the quotes or spaces in the "kill -9 %p" command will fix this. The only alternative appears to be to use 'eval'. Switching to use 'eval' instead of 'exec' also works, but it results in an intermediate bash process being left alive throughout the entire lifetime of the Java proces being started. Using 'exec' prefixed by 'eval' as has been done in this commit gets the best of both worlds, in that options with spaces are parsed correctly, and you don't end up with an intermediate bash process as the parent of the Java process.

2. We can replace single quote with escape-char and a double quote.

JIRA - HADOOP-18327

For code changes:

• [X] 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]

:broken_heart: -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 :ok:	reexec	0m 44s		Docker mode activated.
			_ Prechecks _
+1 :green_heart:	dupname	0m 0s		No case conflicting files found.
+0 :ok:	codespell	0m 0s		codespell was not available.
+0 :ok:	detsecrets	0m 0s		detect-secrets was not available.
+0 :ok:	shelldocs	0m 0s		Shelldocs was not available.
+1 :green_heart:	@author	0m 0s		The patch does not contain any @author tags.
-1 :x:	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 :green_heart:	mvninstall	37m 41s		trunk passed
+1 :green_heart:	mvnsite	1m 44s		trunk passed
+1 :green_heart:	shadedclient	19m 17s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+1 :green_heart:	mvninstall	1m 7s		the patch passed
+1 :green_heart:	blanks	0m 0s		The patch has no blanks issues.
+1 :green_heart:	mvnsite	1m 25s		the patch passed
+1 :green_heart:	shellcheck	0m 3s		No new issues.
+1 :green_heart:	shadedclient	19m 3s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
+1 :green_heart:	unit	2m 1s		hadoop-common in the patch passed.
+1 :green_heart:	asflicense	0m 52s		The patch does not generate ASF License warnings.
		86m 14s

Subsystem	Report/Notes
Docker	ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4536/1/artifact/out/Dockerfile
GITHUB PR	https://github.com/apache/hadoop/pull/4536
Optional Tests	dupname asflicense mvnsite unit codespell detsecrets shellcheck shelldocs
uname	Linux 5cbaa84870a5 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 / 15d9590aa3b41cdafea4e36852fbe6071d4c3c8d
Test Results	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4536/1/testReport/
Max. process+thread count	581 (vs. ulimit of 5500)
modules	C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4536/1/console
versions	git=2.25.1 maven=3.6.3 shellcheck=0.7.0
Powered by	Apache Yetus 0.14.0 https://yetus.apache.org

This message was automatically generated.

[iwasakims]

Can you share the way to reproduce the issue? It looks like we can not get expected outcome regardless of the patch due to another reason.

$ export HADOOP_OPTS="-XX:OnOutOfMemoryError='kill -9 %p'"
$ echo $HADOOP_OPTS
-XX:OnOutOfMemoryError='kill -9 %p'

$ bin/hadoop version
/home/iwasakims/dist/hadoop-3.3.3/bin/../libexec/hadoop-functions.sh: line 1143: -9: command not found
/home/iwasakims/dist/hadoop-3.3.3/bin/../libexec/hadoop-functions.sh: line 1143: -9: command not found
/home/iwasakims/dist/hadoop-3.3.3/bin/../libexec/hadoop-functions.sh: line 1143: -9: command not found
/home/iwasakims/dist/hadoop-3.3.3/bin/../libexec/hadoop-functions.sh: line 1143: -9: command not found
/home/iwasakims/dist/hadoop-3.3.3/bin/../libexec/hadoop-functions.sh: line 1143: -9: command not found
/home/iwasakims/dist/hadoop-3.3.3/bin/../libexec/hadoop-functions.sh: line 1143: -9: command not found
/home/iwasakims/dist/hadoop-3.3.3/bin/../libexec/hadoop-functions.sh: line 1143: -9: command not found
/home/iwasakims/dist/hadoop-3.3.3/bin/../libexec/hadoop-functions.sh: line 1143: -9: command not found
/home/iwasakims/dist/hadoop-3.3.3/bin/../libexec/hadoop-functions.sh: line 1143: -9: command not found
/home/iwasakims/dist/hadoop-3.3.3/bin/../libexec/hadoop-functions.sh: line 1143: -9: command not found
/home/iwasakims/dist/hadoop-3.3.3/bin/../libexec/hadoop-functions.sh: line 1143: -9: command not found
/home/iwasakims/dist/hadoop-3.3.3/bin/../libexec/hadoop-functions.sh: line 1143: -9: command not found
Unrecognized option: -9
Error: Could not create the Java Virtual Machine.
Error: A fatal exception has occurred. Program will exit.

$ bash -x -c 'export SHELLOPTS && bin/hadoop version' 2>&1 | grep OnOutOf
++ export 'HADOOP_OPTS=-XX:OnOutOfMemoryError='\''kill -9 %p'\'''
++ HADOOP_OPTS='-XX:OnOutOfMemoryError='\''kill -9 %p'\'''
++ hadoop_debug 'Initial HADOOP_OPTS=-XX:OnOutOfMemoryError='\''kill -9 %p'\'''
+ HADOOP_OPTS='-XX:OnOutOfMemoryError='\''kill -9 %p'\'' '
+ [[ ! -XX:OnOutOfMemoryError='kill -9 %p'  =~ yarn.log.dir ]]
+ eval 'HADOOP_OPTS='\''-XX:OnOutOfMemoryError='\''kill -9 %p'\''  -Dyarn.log.dir=/home/iwasakims/dist/hadoop-3.3.3/logs'\'''
++ HADOOP_OPTS=-XX:OnOutOfMemoryError=kill
+ [[ ! -XX:OnOutOfMemoryError='kill -9 %p'  =~ yarn.log.file ]]
+ eval 'HADOOP_OPTS='\''-XX:OnOutOfMemoryError='\''kill -9 %p'\''  -Dyarn.log.file=hadoop.log'\'''
++ HADOOP_OPTS=-XX:OnOutOfMemoryError=kill
+ [[ ! -XX:OnOutOfMemoryError='kill -9 %p'  =~ yarn.home.dir ]]
+ eval 'HADOOP_OPTS='\''-XX:OnOutOfMemoryError='\''kill -9 %p'\''  -Dyarn.home.dir=/home/iwasakims/dist/hadoop-3.3.3'\'''
++ HADOOP_OPTS=-XX:OnOutOfMemoryError=kill
+ [[ ! -XX:OnOutOfMemoryError='kill -9 %p'  =~ yarn.root.logger ]]
+ eval 'HADOOP_OPTS='\''-XX:OnOutOfMemoryError='\''kill -9 %p'\''  -Dyarn.root.logger=INFO,console'\'''
++ HADOOP_OPTS=-XX:OnOutOfMemoryError=kill
+ [[ ! -XX:OnOutOfMemoryError='kill -9 %p'  =~ java.library.path ]]
+ eval 'HADOOP_OPTS='\''-XX:OnOutOfMemoryError='\''kill -9 %p'\''  -Djava.library.path=/home/iwasakims/dist/hadoop-3.3.3/lib/native'\'''
++ HADOOP_OPTS=-XX:OnOutOfMemoryError=kill
+ [[ ! -XX:OnOutOfMemoryError='kill -9 %p'  =~ hadoop.log.dir ]]
+ eval 'HADOOP_OPTS='\''-XX:OnOutOfMemoryError='\''kill -9 %p'\''  -Dhadoop.log.dir=/home/iwasakims/dist/hadoop-3.3.3/logs'\'''
++ HADOOP_OPTS=-XX:OnOutOfMemoryError=kill
+ [[ ! -XX:OnOutOfMemoryError='kill -9 %p'  =~ hadoop.log.file ]]
+ eval 'HADOOP_OPTS='\''-XX:OnOutOfMemoryError='\''kill -9 %p'\''  -Dhadoop.log.file=hadoop.log'\'''
++ HADOOP_OPTS=-XX:OnOutOfMemoryError=kill
+ [[ ! -XX:OnOutOfMemoryError='kill -9 %p'  =~ hadoop.home.dir ]]
+ eval 'HADOOP_OPTS='\''-XX:OnOutOfMemoryError='\''kill -9 %p'\''  -Dhadoop.home.dir=/home/iwasakims/dist/hadoop-3.3.3'\'''
++ HADOOP_OPTS=-XX:OnOutOfMemoryError=kill
+ [[ ! -XX:OnOutOfMemoryError='kill -9 %p'  =~ hadoop.id.str ]]
+ eval 'HADOOP_OPTS='\''-XX:OnOutOfMemoryError='\''kill -9 %p'\''  -Dhadoop.id.str=iwasakims'\'''
++ HADOOP_OPTS=-XX:OnOutOfMemoryError=kill
+ [[ ! -XX:OnOutOfMemoryError='kill -9 %p'  =~ hadoop.root.logger ]]
+ eval 'HADOOP_OPTS='\''-XX:OnOutOfMemoryError='\''kill -9 %p'\''  -Dhadoop.root.logger=INFO,console'\'''
++ HADOOP_OPTS=-XX:OnOutOfMemoryError=kill
+ [[ ! -XX:OnOutOfMemoryError='kill -9 %p'  =~ hadoop.policy.file ]]
+ eval 'HADOOP_OPTS='\''-XX:OnOutOfMemoryError='\''kill -9 %p'\''  -Dhadoop.policy.file=hadoop-policy.xml'\'''
++ HADOOP_OPTS=-XX:OnOutOfMemoryError=kill
+ [[ ! -XX:OnOutOfMemoryError='kill -9 %p'  =~ hadoop.security.logger ]]
+ eval 'HADOOP_OPTS='\''-XX:OnOutOfMemoryError='\''kill -9 %p'\''  -Dhadoop.security.logger=INFO,NullAppender'\'''
++ HADOOP_OPTS=-XX:OnOutOfMemoryError=kill
+ hadoop_debug 'Final HADOOP_OPTS: -XX:OnOutOfMemoryError='\''kill -9 %p'\'' '
+ exec /usr/lib/jvm/java-8-openjdk-amd64/bin/java -Dproc_version '-XX:OnOutOfMemoryError='\''kill' -9 '%p'\''' org.apache.hadoop.util.VersionInfo

[ashutoshcipher]

Thanks @iwasakims for your comments. I missed one change which I have added in latest commit. After that change, it will work fine.

Before the change, it is failing.

88665a380593:hadoop-3.3.3 ashugpt$ export HADOOP_OPTS="-XX:OnOutOfMemoryError='kill -9 %p'"
88665a380593:hadoop-3.3.3 ashugpt$ echo $HADOOP_OPTS
-XX:OnOutOfMemoryError='kill -9 %p'
88665a380593:hadoop-3.3.3 ashugpt$ bin/hadoop version
Picked up JAVA_TOOL_OPTIONS: -Dlog4j2.formatMsgNoLookups=true
Hadoop 3.3.3
Source code repository https://github.com/apache/hadoop.git -r d37586cbda38c338d9fe481addda5a05fb516f71
Compiled by stevel on 2022-05-09T16:36Z
Compiled with protoc 3.7.1
From source with checksum eb96dd4a797b6989ae0cdb9db6efc6
This command was run using /Users/ashugpt/Downloads/hadoop-3.3.3/share/hadoop/common/hadoop-common-3.3.3.jar

[hadoop-yetus]

:broken_heart: -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 :ok:	reexec	0m 40s		Docker mode activated.
			_ Prechecks _
+1 :green_heart:	dupname	0m 0s		No case conflicting files found.
+0 :ok:	codespell	0m 0s		codespell was not available.
+0 :ok:	detsecrets	0m 0s		detect-secrets was not available.
+0 :ok:	shelldocs	0m 0s		Shelldocs was not available.
+1 :green_heart:	@author	0m 0s		The patch does not contain any @author tags.
-1 :x:	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 :green_heart:	mvninstall	37m 12s		trunk passed
+1 :green_heart:	mvnsite	1m 44s		trunk passed
+1 :green_heart:	shadedclient	21m 1s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+1 :green_heart:	mvninstall	1m 8s		the patch passed
+1 :green_heart:	blanks	0m 0s		The patch has no blanks issues.
+1 :green_heart:	mvnsite	1m 27s		the patch passed
+1 :green_heart:	shellcheck	0m 4s		No new issues.
+1 :green_heart:	shadedclient	19m 59s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
+1 :green_heart:	unit	2m 0s		hadoop-common in the patch passed.
+1 :green_heart:	asflicense	0m 53s		The patch does not generate ASF License warnings.
		88m 20s

Subsystem	Report/Notes
Docker	ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4536/2/artifact/out/Dockerfile
GITHUB PR	https://github.com/apache/hadoop/pull/4536
Optional Tests	dupname asflicense mvnsite unit codespell detsecrets shellcheck shelldocs
uname	Linux e284b9bf46e9 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 / 7854b910b454279c84f623e46d2df99b5ca65c47
Test Results	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4536/2/testReport/
Max. process+thread count	556 (vs. ulimit of 5500)
modules	C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4536/2/console
versions	git=2.25.1 maven=3.6.3 shellcheck=0.7.0
Powered by	Apache Yetus 0.14.0 https://yetus.apache.org

This message was automatically generated.

[ashutoshcipher]

@iwasakims - Can you please review the latest changes, Thanks.

[PrabhuJoseph]

Thanks @ashutoshcipher for the patch. WIthout this patch the below fails.

export HADOOP_OPTS="-XX:OnOutOfMemoryError='kill -9 %p'"
[root@ip-17-3-4-1 usr]# hadoop fs -ls /
Unrecognized option: -9

@iwasakims Do you have any review comments.

[iwasakims]

Hmm, hadoop_add_param does not work if the value contains ". (HADOOP_OPTS='-XX:OnOutOfMemoryError="kill -9 %p"' instead of previous HADOOP_OPTS="-XX:OnOutOfMemoryError='kill -9 %p'") @ashutoshcipher

$ export HADOOP_OPTS='-XX:OnOutOfMemoryError="kill -9 %p"'
$ echo $HADOOP_OPTS
-XX:OnOutOfMemoryError="kill -9 %p"

$ bash -x -c 'export SHELLOPTS && bin/hadoop version' 2>&1 | less
...
+ hadoop_add_param HADOOP_OPTS yarn.log.dir -Dyarn.log.dir=/home/rocky/dist/hadoop-3.4.0-SNAPSHOT/logs
+ [[ ! -XX:OnOutOfMemoryError="kill -9 %p"  =~ yarn.log.dir ]]
+ eval 'HADOOP_OPTS="-XX:OnOutOfMemoryError="kill -9 %p"  -Dyarn.log.dir=/home/rocky/dist/hadoop-3.4.0-SNAPSHOT/logs"'
++ HADOOP_OPTS=-XX:OnOutOfMemoryError=kill
++ -9 '%p  -Dyarn.log.dir=/home/rocky/dist/hadoop-3.4.0-SNAPSHOT/logs'
/home/rocky/dist/hadoop-3.4.0-SNAPSHOT/bin/../libexec/hadoop-functions.sh: line 1143: -9: command not found
...

[iwasakims]

It would be nice if you can add test cases to files under hadoop-common-project/hadoop-common/src/test/scripts in order to see if no regression.

[iwasakims]

You can run the tests by mvn test -Pshelltest -Dtest=x on hadoop-common-project/hadoop-common. (You need to install bats for this.)

[ashutoshcipher]

Hmm, hadoop_add_param does not work if the value contains ". (HADOOP_OPTS='-XX:OnOutOfMemoryError="kill -9 %p"' instead of previous HADOOP_OPTS="-XX:OnOutOfMemoryError='kill -9 %p'") @ashutoshcipher

$ export HADOOP_OPTS='-XX:OnOutOfMemoryError="kill -9 %p"'
$ echo $HADOOP_OPTS
-XX:OnOutOfMemoryError="kill -9 %p"

$ bash -x -c 'export SHELLOPTS && bin/hadoop version' 2>&1 | less
...
+ hadoop_add_param HADOOP_OPTS yarn.log.dir -Dyarn.log.dir=/home/rocky/dist/hadoop-3.4.0-SNAPSHOT/logs
+ [[ ! -XX:OnOutOfMemoryError="kill -9 %p"  =~ yarn.log.dir ]]
+ eval 'HADOOP_OPTS="-XX:OnOutOfMemoryError="kill -9 %p"  -Dyarn.log.dir=/home/rocky/dist/hadoop-3.4.0-SNAPSHOT/logs"'
++ HADOOP_OPTS=-XX:OnOutOfMemoryError=kill
++ -9 '%p  -Dyarn.log.dir=/home/rocky/dist/hadoop-3.4.0-SNAPSHOT/logs'
/home/rocky/dist/hadoop-3.4.0-SNAPSHOT/bin/../libexec/hadoop-functions.sh: line 1143: -9: command not found
...

@iwasakims

Prefix exec by eval in Hadoop bin scripts Prior to this change, if HADOOP_OPTS contains any arguments that include a space, the command is not parsed correctly. For example, if HADOOP_OPTS="... -XX:OnOutOfMemoryError="kill -9 %p" ...", the bin/hadoop script will fail with the error "Unrecognized option: -9". No amount of clever escaping of the quotes or spaces in the "kill -9 %p" command will fix this. The only alternative appears to be to use 'eval'. Switching to use 'eval' instead of 'exec' also works, but it results in an intermediate bash process being left alive throughout the entire lifetime of the Java proces being started. Using 'exec' prefixed by 'eval' as has been done in this commit gets the best of both worlds, in that options with spaces are parsed correctly, and you don't end up with an intermediate bash process as the parent of the Java process.

[iwasakims]

@ashutoshcipher If your suggestion is to add support spaces enclosed in single quote (') like -XX:OnOutOfMemoryError='kill -9 %p', you should add the statement to documentation and tests showing the spec.

[iwasakims]

add support spaces enclosed in single quote (')

and not to support enclosing vars with double quote (")