Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion scripts/sbin/start-edge.sh
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,16 @@
# Start IoTDB Edge: ConfigNode + DataNode in one JVM process.

if [ -z "${IOTDB_HOME}" ]; then
export IOTDB_HOME="$(cd "$(dirname "$0")"/.. && pwd)"
IOTDB_HOME="$(dirname "$0")/.."
fi
# Normalise to a physical absolute path. This value is handed to the JVM as
# -DIOTDB_HOME and is what stop-edge.sh matches on, so it has to identify the
# installation on its own, independently of the path used to launch.
IOTDB_HOME_PHYSICAL="$(cd -P -- "${IOTDB_HOME}" 2>/dev/null && pwd -P)"
if [ -n "${IOTDB_HOME_PHYSICAL}" ]; then
IOTDB_HOME="${IOTDB_HOME_PHYSICAL}"
fi
export IOTDB_HOME
if [ -z "${IOTDB_CONF}" ]; then
export IOTDB_CONF=${IOTDB_HOME}/conf
fi
Expand Down
34 changes: 32 additions & 2 deletions scripts/sbin/stop-edge.sh
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,19 @@

# Stop IoTDB Edge (the merged ConfigNode + DataNode process).

IOTDB_HOME="$(cd "$(dirname "$0")"/.. && pwd)"
if [ -z "${IOTDB_HOME}" ]; then
IOTDB_HOME="$(cd "$(dirname "$0")"/.. && pwd)"
fi

# Resolve to a physical absolute path so that the same installation reached
# through a different path still compares equal. "cd -P" is required: a plain
# "cd" collapses ".." logically, which would resolve "<symlink>/../x" against the
# symlink's parent instead of its target.
resolve_home() {
(cd -P -- "$1" 2>/dev/null && pwd -P) || printf '%s' "$1"
}

IOTDB_HOME_RESOLVED="$(resolve_home "${IOTDB_HOME}")"

PID_FILE="${IOTDB_HOME}/edge.pid"

Expand All @@ -31,9 +43,27 @@ is_same_edge_home() {
return 0
;;
*)
return 1
;;
esac
# Fall back to comparing resolved paths, so that a start-edge.sh invoked with
# IOTDB_HOME pointing at a symlink is still recognised here. The value is
# delimited by the next " -D", which start-edge.sh always emits after
# -DIOTDB_HOME. If a hand-built command line ends with -DIOTDB_HOME, the
# extraction keeps the trailing arguments, the resolution below fails and the
# process is simply not matched -- never matched to the wrong installation.
local home="${command_line#*-DIOTDB_HOME=}"
home="${home%% -D*}"
[ -n "$home" ] && [ "$home" != "$command_line" ] || return 1
# Only absolute values can be resolved from here. A relative one such as "."
# is meaningful in the started process's working directory, not in ours, so
# resolving it here could match an unrelated installation. The emptiness check
# above matters for the same reason: "cd" succeeds on an empty argument and
# yields our own working directory.
case "$home" in
/*) ;;
*) return 1 ;;
esac
[ "$(resolve_home "$home")" = "${IOTDB_HOME_RESOLVED}" ]
Comment on lines +54 to +66

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Resolve relative process homes using the process's working directory

start-edge.sh accepts IOTDB_HOME=. and passes it unchanged to the JVM. If an instance was started this way in /srv/edge-a, running /srv/edge-b/sbin/stop-edge.sh from /srv/edge-b with IOTDB_HOME unset resolves the process's . to B. Consequently, find_edge_processes considers A a match and terminates it, even if B has no PID file or running instance. The base script rejects this process; the new fallback accepts it.

Please normalize the home to a physical absolute path at launch, or resolve relative process values against that process's working directory; do not resolve them against the stopping shell's working directory.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right, and thank you for running it. I reproduced it before changing
anything: a synthetic process with cwd in edge-a and -DIOTDB_HOME=. on its
command line, stopped from edge-b -- the base script correctly declines it, my
fallback resolved the . against the stopping shell and terminated it.

I have taken the first of your two suggestions. start-edge.sh now normalises
IOTDB_HOME to a physical absolute path before exporting it, so the value handed
to the JVM identifies the installation on its own:

started through the symlink, before   -DIOTDB_HOME=$ROOT/iotdb
started through the symlink, now      -DIOTDB_HOME=$ROOT/apache-iotdb-2.0.11-SNAPSHOT-edge-bin

I kept an absolute-path guard in the stop-edge.sh fallback as well, rather than
relying only on the launcher. A process started by an earlier start-edge.sh
during an upgrade can still carry a relative value, and for those the fallback now
declines instead of resolving against our own directory -- the same outcome as the
base script. That leaves resolving against the process's working directory
unnecessary, which I was reluctant to do since it needs lsof or /proc and a
wrong answer there puts us back to stopping the wrong process.

Two things about the normalisation that I would rather state than leave to be
found. It changes the path string that appears in ps, in the console message and
in the logs -- on this machine /tmp becomes /private/tmp -- which is the point
of the change, but anything grepping for the old string is affected. And if the
cd -P cannot run, IOTDB_HOME keeps the value it had, which for the unset case
is still relative; that instance then falls to the literal comparison in
stop-edge.sh, which is today's behaviour. A fallback to a logical cd does not
help here: both still have to reach the directory, so it fails on the same inputs.

Both of your cases are in the regression set now, with the version you reviewed as
a control so they are shown to fail without the change.

}

is_edge_process() {
Expand Down