bitflicker64 commented on code in PR #3253:
URL: https://github.com/apache/hugegraph/pull/3253#discussion_r4252226754
##########
hugegraph-store/hg-store-dist/src/assembly/static/bin/restart-hugegraph-store.sh:
##########
@@ -28,5 +28,46 @@ function abs_path() {
}
BIN=$(abs_path)
-. "$BIN"/stop-hugegraph-store.sh
-. "$BIN"/start-hugegraph-store.sh
+
+DAEMON_ARG=""
+CONF_ARG=""
+GC_ARG=""
+PID_ARG=""
+USER_ARG=""
+LOGS_ARG=""
+PLUGINS_ARG=""
+TELEMETRY_ARG=""
+
+while getopts "d:c:g:i:j:l:o:y:" arg; do
+ case ${arg} in
+ d) DAEMON_ARG="$OPTARG" ;;
+ c) CONF_ARG="$OPTARG" ;;
+ g) GC_ARG="$OPTARG" ;;
+ i) PID_ARG="$OPTARG" ;;
+ j) USER_ARG="$OPTARG" ;;
+ l) LOGS_ARG="$OPTARG" ;;
+ o) PLUGINS_ARG="$OPTARG" ;;
+ y) TELEMETRY_ARG="$OPTARG" ;;
+ ?) echo "USAGE: $0 [-d true|false] [-c conf_dir] [-g g1] [-i pid_file]
[-j opts] [-l logs_dir] [-o plugins_dir] [-y true|false]" && exit 1 ;;
+ esac
+done
+
+# stop-hugegraph-store.sh only understands -i (the pid file).Forward the -i
flag to stop script
+STOP_ARGS=()
+[ -n "$PID_ARG" ] && STOP_ARGS+=(-i "$PID_ARG")
+
+# Forward every override to start so the restarted instance also keeps the
overriden
+# conf/logs/pid/plugins/daemon/gc/telemetry settings
+START_ARGS=()
+[ -n "$DAEMON_ARG" ] && START_ARGS+=(-d "$DAEMON_ARG")
+[ -n "$CONF_ARG" ] && START_ARGS+=(-c "$CONF_ARG")
+[ -n "$GC_ARG" ] && START_ARGS+=(-g "$GC_ARG")
+[ -n "$PID_ARG" ] && START_ARGS+=(-i "$PID_ARG")
+[ -n "$USER_ARG" ] && START_ARGS+=(-j "$USER_ARG")
+[ -n "$LOGS_ARG" ] && START_ARGS+=(-l "$LOGS_ARG")
+[ -n "$PLUGINS_ARG" ] && START_ARGS+=(-o "$PLUGINS_ARG")
+[ -n "$TELEMETRY_ARG" ] && START_ARGS+=(-y "$TELEMETRY_ARG")
+
+# Run stop and start as separate processes so that:
+bash "$BIN"/stop-hugegraph-store.sh "${STOP_ARGS[@]}"
Review Comment:
Important: The stop result is ignored here, so a failed stop now ends in a
restart that reports success. On master this script sourced
stop-hugegraph-store.sh, and its `exit 1` (line 54 at this head, when
`kill_process_and_wait` times out after 30 seconds) ended the restart with
status 1. Now stop runs as a child, its status is dropped, and
start-hugegraph-store.sh runs anyway. The old JVM is still alive, so the check
at start-hugegraph-store.sh line 253 prints `HugeGraphStoreServer is already
running...` and exits 0. I ran this script at 6ecc85e and the master version
with a stub stop that exits 1 and a stub start that exits 0: master returns 1
without calling start, this head calls start and returns 0. A caller that
checks the exit code then believes Store was restarted with the new settings
while the old process keeps running. The comment on line 71 also stops after
`so that:`. Please stop when stop fails:
```suggestion
bash "$BIN"/stop-hugegraph-store.sh "${STOP_ARGS[@]}" || exit 1
```
##########
hugegraph-store/hg-store-dist/src/assembly/static/bin/start-hugegraph-store.sh:
##########
@@ -116,19 +111,36 @@ if [ -z "$DAEMON" ]; then
DAEMON="true"
fi
-while getopts "d:g:j:y:" arg; do
+while getopts "d:c:g:i:j:l:o:y:" arg; do
case ${arg} in
+ c) CONF_OVERRIDE="$OPTARG" ;;
g) GC_OPTION="$OPTARG" ;;
+ i) PID_FILE_OVERRIDE="$OPTARG" ;;
j) USER_OPTION="$OPTARG" ;;
+ l) LOGS_OVERRIDE="$OPTARG" ;;
+ o) PLUGINS_OVERRIDE="$OPTARG" ;;
# Telemetry is used to collect metrics, traces and logs
y) OPEN_TELEMETRY="$OPTARG" ;;
d) DAEMON="$OPTARG" ;;
- ?) echo "USAGE: $0 [-d true|false] [-g g1|ZGC] [-j xxx] [-y
true|false]" && exit 1 ;;
+ ?) echo "USAGE: $0 [-d true|false] [-c conf_dir] [-g g1|ZGC] [-i
pid_file] [-j xxx] [-l logs_dir] [-o plugins_dir] [-y true|false]" && exit 1 ;;
esac
done
+# Canonicalize relative path overrides to absolute paths.
+CONF_OVERRIDE="$(canonicalize_dir "$CONF_OVERRIDE")" || exit 1
+LOGS_OVERRIDE="$(canonicalize_dir "$LOGS_OVERRIDE")" || exit 1
+PLUGINS_OVERRIDE="$(canonicalize_dir "$PLUGINS_OVERRIDE")" || exit 1
+PID_FILE_OVERRIDE="$(canonicalize_file "$PID_FILE_OVERRIDE")" || exit 1
+
+CONF="${CONF_OVERRIDE:-$TOP/conf}"
+LOGS="${LOGS_OVERRIDE:-$TOP/logs}"
+PLUGINS="${PLUGINS_OVERRIDE:-$TOP/plugins}"
+OUTPUT=${LOGS}/hugegraph-store-server.log
+PID_FILE="${PID_FILE_OVERRIDE:-$BIN/pid}"
Review Comment:
Important: Store does not export the four overrides, so the restart that
Store triggers on itself loses them. `PdConfigureListener.restart()` runs `sh
-c <user.dir>/bin/restart-hugegraph-store.sh` with no arguments when PD
publishes a new `S:Timestamp`, and the child only inherits the JVM environment.
Here the overrides stay plain shell variables (start-hugegraph.sh on Server
does export them). For a Store started with `-c /etc/hugegraph-store -i
/run/hugegraph-store/pid`, that restart calls stop without `-i`, which finds no
`$BIN/pid` and exits 0 without stopping anything. It then calls start with no
flags, which resolves `$TOP/conf`. The `ps` check at line 253 looks for
`$TOP/conf` and does not match the running JVM, so a second Store JVM is
launched from the stock conf while the original keeps running. Please export
the overrides after they are resolved, for example `export CONF_OVERRIDE
LOGS_OVERRIDE PLUGINS_OVERRIDE PID_FILE_OVERRIDE` below this block, so
restart-hugegraph-st
ore.sh and stop-hugegraph-store.sh pick them up when called without flags.
Separately, the listener reads and writes `<user.dir>/conf/application-pd.yml`,
so the pushed config does not follow `-c` either; that Java code is not part of
this PR and may be better as a follow-up.
--
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]
