imbajin commented on code in PR #3253:
URL: https://github.com/apache/hugegraph/pull/3253#discussion_r4172354571
##########
hugegraph-server/hugegraph-dist/src/assembly/static/bin/util.sh:
##########
@@ -655,6 +682,6 @@ function kill_process_and_wait() {
}
function exit_with_usage_help(){
- echo "USAGE: $0 [-d true|false] [-g g1] [-m true|false] [-p true|false]
[-s true|false] [-j java_options] [-t timeout] [-y true|false]"
+ echo "USAGE: $0 [-c conf_dir] [-d true|false] [-g g1] [-i pid_file] [-l
logs_dir] [-m true|false] [-p true|false] [-s true|false] [-j java_options] [-t
timeout] [-y true|false]"
Review Comment:
🧹 start-hugegraph.sh now accepts -o for PLUGINS_OVERRIDE, but this usage
text omits it. Please add [-o plugins_dir] so the help lists every accepted
option.
##########
hugegraph-server/hugegraph-dist/src/assembly/static/bin/start-hugegraph.sh:
##########
@@ -39,30 +39,44 @@ function abs_path() {
BIN=$(abs_path)
TOP="$(cd "$BIN"/../ && pwd)"
-CONF="$TOP/conf"
-LOGS="$TOP/logs"
SCRIPTS="$TOP/scripts"
-PID_FILE="$BIN/pid"
. "$BIN"/util.sh
# Note: keep ':' in the end of the string to indicate the option needs a value
-while getopts "d:g:m:p:s:j:t:y:" arg; do
- case ${arg} in
- d) DAEMON="$OPTARG" ;;
- g) GC_OPTION="$OPTARG" ;;
- m) OPEN_MONITOR="$OPTARG" ;;
- p) PRELOAD="$OPTARG" ;;
- s) OPEN_SECURITY_CHECK="$OPTARG" ;;
- j) USER_OPTION="$OPTARG" ;;
- t) SERVER_STARTUP_TIMEOUT_S="$OPTARG" ;;
- # Telemetry is used to collect metrics, traces and logs
- y) OPEN_TELEMETRY="$OPTARG" ;;
- # Note: update usage info when the params changed
- ?) exit_with_usage_help ;;
- esac
+while getopts "c:d:g:i:j:l:m:o:p:s:t:y:" arg; do
+ case ${arg} in
+ c) CONF_OVERRIDE="$OPTARG" ;;
+ d) DAEMON="$OPTARG" ;;
+ g) GC_OPTION="$OPTARG" ;;
+ i) PID_FILE_OVERRIDE="$OPTARG" ;;
+ j) USER_OPTION="$OPTARG" ;;
+ l) LOGS_OVERRIDE="$OPTARG" ;;
+ m) OPEN_MONITOR="$OPTARG" ;;
+ o) PLUGINS_OVERRIDE="$OPTARG" ;;
+ p) PRELOAD="$OPTARG" ;;
+ s) OPEN_SECURITY_CHECK="$OPTARG" ;;
+ t) SERVER_STARTUP_TIMEOUT_S="$OPTARG" ;;
+ # Telemetry is used to collect metrics, traces and logs
+ y) OPEN_TELEMETRY="$OPTARG" ;;
+ # Note: update usage info when the params changed
+ ?) exit_with_usage_help ;;
+ esac
done
+# Canonicalize relative path overrides to absolute paths
+CONF_OVERRIDE="$(canonicalize_dir "$CONF_OVERRIDE")"
Review Comment:
⚠️ If a requested override cannot be created or resolved,
canonicalize_dir/canonicalize_file exits only inside this command-substitution
subshell. This assignment's failure is ignored and the following
${...:-$TOP/...} assignments silently select packaged defaults, so the service
can start with a different configuration or PID path than requested. Please
stop the parent script when any normalization fails. Evidence: these four
assignments at lines 68-71 are followed by defaulting at lines 73-76, and the
script does not enable errexit.
##########
hugegraph-server/hugegraph-dist/src/assembly/static/bin/start-monitor.sh:
##########
@@ -37,6 +37,21 @@ fi
# Monitor HugeGraphServer every minute, if the server crashes then restart it.
# Modify the frequency according to actual needs carefully.
-CRONTAB_JOB="*/1 * * * * export JAVA_HOME=$JAVA_HOME &&
$TOP/bin/monitor-hugegraph.sh"
+
+# Persist any path overrides (-c/-l/-i/-o) that the caller
(start-hugegraph.sh) exported
+CRONTAB_JOB="*/1 * * * * export JAVA_HOME=$JAVA_HOME &&"
+if [ -n "$CONF_OVERRIDE" ]; then
+ CRONTAB_JOB="$CRONTAB_JOB export CONF_OVERRIDE='$CONF_OVERRIDE' &&"
Review Comment:
⚠️ An override containing an apostrophe is inserted inside single quotes
without escaping, producing malformed shell syntax in the saved cron command.
The monitor then cannot restore the path settings or reliably restart the
server. Please shell-escape each value before building CRONTAB_JOB, or reject
unsupported characters with a clear error.
##########
hugegraph-server/hugegraph-dist/src/assembly/static/bin/stop-hugegraph.sh:
##########
@@ -44,7 +45,7 @@ TOP="$(cd $BIN/../ && pwd)"
. "$BIN"/util.sh
-PID_FILE=$BIN/pid
+PID_FILE="${PID_FILE_OVERRIDE:-$BIN/pid}"
Review Comment:
⚠️ The new -i path is preserved by canonicalize_file, but the stop scripts
later expand PID_FILE unquoted in their file checks/reads (here, cat $PID_FILE;
Store and PD also use [ -f ${PID_FILE} ]). A path such as '/var/lib/Huge
Graph/server.pid' can be written by start but is split when stop reads it,
leaving the service running. Please quote PID_FILE at every test/read in all
three stop scripts.
--
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]
