ctubbsii commented on code in PR #2120:
URL: https://github.com/apache/zookeeper/pull/2120#discussion_r1471365691
##########
bin/zkEnv.sh:
##########
@@ -144,9 +144,15 @@ fi
#echo "CLASSPATH=$CLASSPATH"
-# default heap for zookeeper server
-ZK_SERVER_HEAP="${ZK_SERVER_HEAP:-1000}"
-export SERVER_JVMFLAGS="-Xmx${ZK_SERVER_HEAP}m $SERVER_JVMFLAGS"
+# ZK_SERVER_MAXRAMPERCENTAGE has higher precedence over ZK_SERVER_HEAP
+if [ "x$ZK_SERVER_MAXRAMPERCENTAGE" = "x" ]
+then
+ # default heap for zookeeper server
+ ZK_SERVER_HEAP="${ZK_SERVER_HEAP:-1000}"
+ export SERVER_JVMFLAGS="-Xmx${ZK_SERVER_HEAP}m $SERVER_JVMFLAGS"
+else
+ export SERVER_JVMFLAGS="-XX:MaxRAMPercentage=$ZK_SERVER_MAXRAMPERCENTAGE
$SERVER_JVMFLAGS"
+fi
Review Comment:
This seems like it's a bit overly complex. This file is already a config
file. It doesn't need additional configuration options to configure the config
file.
Users can just edit this file if they need to tweak it. So, it really should
just be as simple as possible, and only as complex as necessary for users to be
able to easily set the jvm flags.
I suggest something like this:
```suggestion
export SERVER_JVMFLAGS=${SERVER_JVMFLAGS:--Xmx1000m}
```
This gets rid of the unneeded ZK_SERVER_HEAP environment variable and bakes
the default into the `SERVER_JVMFLAGS variable`, which you can still override,
either by editing this file, or by setting `export
SERVER_JVMFLAGS='-XX:MaxRAMPercentage=25'` (or whatever value you want) in your
environment. I would suggest a similar thing be done for the `CLIENT_JVMFLAGS`.
--
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]
