VedarthConfluent commented on code in PR #15048:
URL: https://github.com/apache/kafka/pull/15048#discussion_r1443686810


##########
docker/jvm/launch:
##########
@@ -28,25 +27,25 @@ fi
 # the default is to pick the first IP (or network).
 export KAFKA_JMX_HOSTNAME=${KAFKA_JMX_HOSTNAME:-$(hostname -i | cut -d" " -f1)}
 
-if [ "$KAFKA_JMX_PORT" ]; then
+if [ "${KAFKA_JMX_PORT-}" ]; then
   # This ensures that the "if" section for JMX_PORT in kafka launch script 
does not trigger.
     export JMX_PORT=$KAFKA_JMX_PORT
-    export KAFKA_JMX_OPTS="$KAFKA_JMX_OPTS 
-Djava.rmi.server.hostname=$KAFKA_JMX_HOSTNAME 
-Dcom.sun.management.jmxremote.local.only=false 
-Dcom.sun.management.jmxremote.rmi.port=$JMX_PORT 
-Dcom.sun.management.jmxremote.port=$JMX_PORT"
+    export KAFKA_JMX_OPTS="${KAFKA_JMX_OPTS-} 
-Djava.rmi.server.hostname=$KAFKA_JMX_HOSTNAME 
-Dcom.sun.management.jmxremote.local.only=false 
-Dcom.sun.management.jmxremote.rmi.port=$JMX_PORT 
-Dcom.sun.management.jmxremote.port=$JMX_PORT"
 fi
 
 # Make a temp env variable to store user provided performance otps
-if [ -z "$KAFKA_JVM_PERFORMANCE_OPTS" ]; then
+if [ -z "${KAFKA_JVM_PERFORMANCE_OPTS-}" ]; then
     export TEMP_KAFKA_JVM_PERFORMANCE_OPTS=""
 else
     export TEMP_KAFKA_JVM_PERFORMANCE_OPTS="$KAFKA_JVM_PERFORMANCE_OPTS"
 fi
 
 # We will first use CDS for storage to format storage
-export KAFKA_JVM_PERFORMANCE_OPTS="$KAFKA_JVM_PERFORMANCE_OPTS 
-XX:SharedArchiveFile=/opt/kafka/storage.jsa"
+export KAFKA_JVM_PERFORMANCE_OPTS="${KAFKA_JVM_PERFORMANCE_OPTS-} 
-XX:SharedArchiveFile=/opt/kafka/storage.jsa"
 
 echo "===> Using provided cluster id $CLUSTER_ID ..."
 # A bit of a hack to not error out if the storage is already formatted. Need 
storage-tool to support this

Review Comment:
   Update on this, I have moved this check back to the bash script, given that 
the documentation suggests that `--ignored-formatted` flag should only be used 
in a very specific scenario only and I couldn't find another way to handle it 
in the wrapper. Maybe it will be feasible once storage tool is rewritten.
   Given that this refactor was mentioned as a minor concern, hopefully it 
should be okay if we keep this small logic in bash script for now.



-- 
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: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to