epugh commented on code in PR #2792:
URL: https://github.com/apache/solr/pull/2792#discussion_r1818879760
##########
solr/bin/solr:
##########
@@ -1054,31 +1054,13 @@ fi
# Establish default GC logging opts if no env var set (otherwise init to
sensible default)
if [ -z "${GC_LOG_OPTS}" ]; then
- if [[ "$JAVA_VER_NUM" -lt "9" ]] ; then
- GC_LOG_OPTS=('-verbose:gc' '-XX:+PrintHeapAtGC' '-XX:+PrintGCDetails' \
- '-XX:+PrintGCDateStamps' '-XX:+PrintGCTimeStamps'
'-XX:+PrintTenuringDistribution' \
- '-XX:+PrintGCApplicationStoppedTime')
- else
- GC_LOG_OPTS=('-Xlog:gc*')
- fi
-else
- # TODO: Should probably not overload GC_LOG_OPTS as both string and array,
but leaving it be for now
- # shellcheck disable=SC2128
- GC_LOG_OPTS=($GC_LOG_OPTS)
+ GC_LOG_OPTS=('-Xlog:gc*')
Review Comment:
I googled to confirm the `-Xlog:gc*` syntax... This is some nice cleanup!
##########
solr/bin/solr:
##########
@@ -1054,31 +1054,13 @@ fi
# Establish default GC logging opts if no env var set (otherwise init to
sensible default)
if [ -z "${GC_LOG_OPTS}" ]; then
- if [[ "$JAVA_VER_NUM" -lt "9" ]] ; then
- GC_LOG_OPTS=('-verbose:gc' '-XX:+PrintHeapAtGC' '-XX:+PrintGCDetails' \
- '-XX:+PrintGCDateStamps' '-XX:+PrintGCTimeStamps'
'-XX:+PrintTenuringDistribution' \
- '-XX:+PrintGCApplicationStoppedTime')
- else
- GC_LOG_OPTS=('-Xlog:gc*')
- fi
-else
- # TODO: Should probably not overload GC_LOG_OPTS as both string and array,
but leaving it be for now
- # shellcheck disable=SC2128
- GC_LOG_OPTS=($GC_LOG_OPTS)
+ GC_LOG_OPTS=('-Xlog:gc*')
fi
# if verbose gc logging enabled, setup the location of the log file and
rotation
if [ "${#GC_LOG_OPTS[@]}" -gt 0 ]; then
- if [[ "$JAVA_VER_NUM" -lt "9" ]] || [ "$JAVA_VENDOR" == "OpenJ9" ]; then
- gc_log_flag="-Xloggc"
- if [ "$JAVA_VENDOR" == "OpenJ9" ]; then
- gc_log_flag="-Xverbosegclog"
- fi
- if [ -z ${JAVA8_GC_LOG_FILE_OPTS+x} ]; then
Review Comment:
That makes sense. I do sort of wish we had a bit more ways of testing
these settings in our `.bats` tests, especially as with Java major versions
coming out every six months, we may find we are using more of these
configuration options. Your cleanup makes sense to me!
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
