[ 
https://issues.apache.org/jira/browse/CASSANDRA-20745?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17993041#comment-17993041
 ] 

Stefan Miklosovic edited comment on CASSANDRA-20745 at 7/3/25 10:36 AM:
------------------------------------------------------------------------

I have added one more commit which removes the dependency of nodetool calling 
cassandra-env.sh. It was there purely for getting JMX_PORT only which is not 
necessary anymore. We can get that port by parsing that script itself.

https://github.com/apache/cassandra/pull/4226/commits/9931372cc184d65143bd026f7efed08ac5f5ff42

I do not think it was ever intentional to run cassandra-env.sh and to invoke 
other logic. It is visible from the current code where we are setting old JVM 
options which would be updated by cassandra-env.sh, just to get it back to what 
it was after we picked up the port from there. 


was (Author: smiklosovic):
I have added one more commit which removes the dependency of nodetool calling 
cassandra-env.sh. It was there purely for getting JMX_PORT only which is not 
necessary anymore. We can get that port by parsing that script itself.

https://github.com/apache/cassandra/pull/4226/commits/9931372cc184d65143bd026f7efed08ac5f5ff42

> nodetool not started when MAX_HEAP_SIZE is set only in cassandra-env.sh
> -----------------------------------------------------------------------
>
>                 Key: CASSANDRA-20745
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-20745
>             Project: Apache Cassandra
>          Issue Type: Bug
>          Components: Legacy/Tools
>            Reporter: Stefan Miklosovic
>            Priority: Normal
>          Time Spent: 10m
>  Remaining Estimate: 0h
>
> cassandra-env.sh says in its documentation this (1)
> {noformat}
> # Override these to set the amount of memory to allocate to the JVM at
> # start-up. For production use you may wish to adjust this for your
> # environment. MAX_HEAP_SIZE is the total amount of memory dedicated
> # to the Java heap. HEAP_NEWSIZE refers to the size of the young
> # generation when CMS is used.
> #
> # When using G1 only MAX_HEAP_SIZE may be set, and HEAP_NEWSIZE must be
> # left unset.
> #
> # When using CMS both MAX_HEAP_SIZE and HEAP_NEWSIZE should be either set
> # or not (if you set one, set the other).
> #MAX_HEAP_SIZE="20G"
> #HEAP_NEWSIZE="10G"
> #MAX_DIRECT_MEMORY_SIZE="10G"
> {noformat}
> Okay, so when I am going to use G1 then I need to set MAX_HEAP_SIZE only and 
> I have to leave HEAP_NEWSIZE unset.
> So I do that, by uncommenting only that line:
> {code}
> MAX_HEAP_SIZE="20G"
> #HEAP_NEWSIZE="10G"
> #MAX_DIRECT_MEMORY_SIZE="10G"
> {code}
> Then Cassandra starts and I am going to invoke nodetool:
> {code}
> $ nodetool status
> please set or unset MAX_HEAP_SIZE and HEAP_NEWSIZE in pairs when using CMS GC 
> (see cassandra-env.sh)
> {code}
> The reason this is happening is that nodetool adds all parameters from 
> jvm(11|17)-clients.options which do not contain any information about GC:
> {code}
> -Djdk.attach.allowAttachSelf=true --add-exports 
> java.base/jdk.internal.misc=ALL-UNNAMED --add-exports 
> java.base/jdk.internal.ref=ALL-UNNAMED --add-exports 
> java.base/sun.nio.ch=ALL-UNNAMED --add-exports 
> java.management.rmi/com.sun.jmx.remote.internal.rmi=ALL-UNNAMED --add-exports 
> java.rmi/sun.rmi.registry=ALL-UNNAMED --add-exports 
> java.rmi/sun.rmi.server=ALL-UNNAMED --add-exports 
> java.sql/java.sql=ALL-UNNAMED --add-opens 
> java.base/java.lang.module=ALL-UNNAMED --add-opens 
> java.base/jdk.internal.loader=ALL-UNNAMED --add-opens 
> java.base/jdk.internal.ref=ALL-UNNAMED --add-opens 
> java.base/jdk.internal.reflect=ALL-UNNAMED --add-opens 
> java.base/jdk.internal.math=ALL-UNNAMED --add-opens 
> java.base/jdk.internal.module=ALL-UNNAMED --add-opens 
> java.base/jdk.internal.util.jar=ALL-UNNAMED --add-opens 
> jdk.management/com.sun.management.internal=ALL-UNNAMED --add-opens 
> java.base/java.lang.reflect=ALL-UNNAMED
> {code}
> However, nodetool eventually sources cassandra-env.sh (2) which will trigger 
> that logic.
> More to it, this code in particular seems to be not entirely correct, there 
> is a small logical error in that (3).
> {code}
> if [ "x$MAX_HEAP_SIZE" = "x" ] && [ "x$HEAP_NEWSIZE" = "x" -o $USING_G1 -eq 0 
> ]; then
>     MAX_HEAP_SIZE="$CALCULATED_MAX_HEAP_SIZE"
>     HEAP_NEWSIZE="$CALCULATED_CMS_HEAP_NEWSIZE"
> elif [ "x$MAX_HEAP_SIZE" = "x" ] ||  [ "x$HEAP_NEWSIZE" = "x" -a $USING_G1 
> -ne 0 ]; then
>     echo "please set or unset MAX_HEAP_SIZE and HEAP_NEWSIZE in pairs when 
> using CMS GC (see cassandra-env.sh)"
>     exit 1
> fi
> {code}
> When I set MAX_HEAP_SIZE only, as advised on using G1 GC, then "elif" brach 
> will go to evaluate 
> {code}
> [ "x$HEAP_NEWSIZE" = "x" -a $USING_G1 -ne 0 ]
> {code}
> "xHEAP_NEWSIZE" = "x" will evaluate to true and USING_G1 -ne 0 will evaluate 
> to true as well! Why? Because nodetool options do not contain "+UseG1GC"
> so when nodetool calls this
> {code}
> echo $JVM_OPTS | grep -q +UseG1GC
> USING_G1=$?
> {code}
> then USING_G1 will be set to 1, because $? will be 1 as grep returns 1 when 
> not found. That evaluates USING_G1 -ne 0 to "true", which makes "[ 
> "x$HEAP_NEWSIZE" = "x" -a $USING_G1 -ne 0 ]" to be true, which will print 
> that error message and nodetool will not start.
> (1) 
> https://github.com/apache/cassandra/blob/trunk/conf/cassandra-env.sh#L99-L137
> (2) https://github.com/apache/cassandra/blob/trunk/bin/nodetool#L46-L53
> (3) 
> https://github.com/apache/cassandra/blob/trunk/conf/cassandra-env.sh#L131-L137



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org

Reply via email to