kezhuw commented on code in PR #2120:
URL: https://github.com/apache/zookeeper/pull/2120#discussion_r1729655144
##########
bin/zkEnv.sh:
##########
@@ -145,9 +145,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"
+if [[ -n $ZK_SERVER_HEAP ]]; then
+ echo 'WARNING: ZK_SERVER_HEAP will not be recognized in a future version.
Set -Xmx directly using SERVER_JVMFLAGS instead.'
+ SERVER_JVMFLAGS="-Xmx${ZK_SERVER_HEAP}m $SERVER_JVMFLAGS"
+fi
+export SERVER_JVMFLAGS=${SERVER_JVMFLAGS:--Xmx1000m}
Review Comment:
This breaks ZOOKEEPER-1670, if we custom `SERVER_JVMFLAGS` for propose other
than max heap size.
I think our goals are:
1. Backward compatibility.
2. Use `SERVER_JVMFLAGS` to custom all jvm flags.
3. Set max heap size if no related flags (`-Xmx`, `-XX:MaxRAM` and
`-XX:MaxRAMPercentage`) configured.
I think we chould **prepend** `-Xmx1000m` only if `SERVER_JVMFLAGS` does not
contain `-XX:MaxRAM` to achieve above.
See also for
[comments](https://issues.apache.org/jira/browse/ZOOKEEPER-1670?focusedCommentId=13606860&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-13606860)
from ZOOKEEPER-1670:
> However another problem exists where SERVER_JVMFLAGS is also used to set
the jaas configuration file. I feel we need to introduce another variable
called ZK_SERVER_HEAP which defaults to 1G and we set the
SERVER_JVMFLAGS="$ZK_SERVER_HEAP $SERVER_JVMFLAGS"
>
> That way any existing settings user do for heap size still works but will
default to 1G if nothing is found.
> >Also many folks are already specifying the heap size using the command
line "SERVER_JVMFLAGS" parameter. This change will now cause potential problems
as the new defaults may override the SERVER_JVMFLAGS settings already in place.
>
> I dont think this is true as SERVER_JVMFLAGS is at the end. So if you were
specifying the heap size using SERVER_JVMFLAGS it should still be applied as
the last heap size setting is what will be the picked up.
--
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: notifications-unsubscr...@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
