ctubbsii commented on code in PR #2061:
URL: https://github.com/apache/zookeeper/pull/2061#discussion_r2277814199
##########
bin/zkEnv.sh:
##########
@@ -136,8 +136,16 @@ export CLASSPATH
# default heap for zookeeper server
ZK_SERVER_HEAP="${ZK_SERVER_HEAP:-1000}"
-export SERVER_JVMFLAGS="-Xmx${ZK_SERVER_HEAP}m $SERVER_JVMFLAGS"
+if [[ ${ZK_SERVER_HEAP} =~ ^[0-9]+.*[0-9]+$ ]]; then
Review Comment:
Curly braces aren't needed in these square brackets. Also, I'm not sure you
have the regex correct. Yours would match on any of these clearly incorrect
values:
* `00000 abc 0`
* `00` but not `0`
I think what you want is "a single non-zero digit, followed by any number of
additional digits", which is `^[1-9][0-9]*$`
```suggestion
if [[ $ZK_SERVER_HEAP =~ ^[1-9][0-9]*$ ]]; then
```
##########
bin/zkEnv.sh:
##########
@@ -136,8 +136,16 @@ export CLASSPATH
# default heap for zookeeper server
ZK_SERVER_HEAP="${ZK_SERVER_HEAP:-1000}"
-export SERVER_JVMFLAGS="-Xmx${ZK_SERVER_HEAP}m $SERVER_JVMFLAGS"
+if [[ ${ZK_SERVER_HEAP} =~ ^[0-9]+.*[0-9]+$ ]]; then
+ export SERVER_JVMFLAGS="-Xmx${ZK_SERVER_HEAP}m $SERVER_JVMFLAGS"
+else
+ export SERVER_JVMFLAGS="${ZK_SERVER_HEAP} $SERVER_JVMFLAGS"
Review Comment:
Curly braces aren't needed here:
```suggestion
export SERVER_JVMFLAGS="$ZK_SERVER_HEAP $SERVER_JVMFLAGS"
```
##########
bin/zkEnv.sh:
##########
@@ -136,8 +136,16 @@ export CLASSPATH
# default heap for zookeeper server
ZK_SERVER_HEAP="${ZK_SERVER_HEAP:-1000}"
-export SERVER_JVMFLAGS="-Xmx${ZK_SERVER_HEAP}m $SERVER_JVMFLAGS"
+if [[ ${ZK_SERVER_HEAP} =~ ^[0-9]+.*[0-9]+$ ]]; then
+ export SERVER_JVMFLAGS="-Xmx${ZK_SERVER_HEAP}m $SERVER_JVMFLAGS"
+else
+ export SERVER_JVMFLAGS="${ZK_SERVER_HEAP} $SERVER_JVMFLAGS"
+fi
# default heap for zookeeper client
ZK_CLIENT_HEAP="${ZK_CLIENT_HEAP:-256}"
-export CLIENT_JVMFLAGS="-Xmx${ZK_CLIENT_HEAP}m $CLIENT_JVMFLAGS"
+if [[ ${ZK_CLIENT_HEAP} =~ ^[0-9]+.*[0-9]+$ ]]; then
+ export CLIENT_JVMFLAGS="-Xmx${ZK_CLIENT_HEAP}m $CLIENT_JVMFLAGS"
+else
+ export CLIENT_JVMFLAGS="${ZK_CLIENT_HEAP} $CLIENT_JVMFLAGS"
+fi
Review Comment:
An alternative approach that is much simpler:
```suggestion
# set memory options only if the user didn't customize the CLIENT_JVMFLAGS
if [[ -z $CLIENT_JVMFLAGS ]]; then
export CLIENT_JVMFLAGS="-Xmx${ZK_CLIENT_HEAP}m"
fi
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
