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