byteayan commented on code in PR #3258:
URL: https://github.com/apache/hugegraph/pull/3258#discussion_r4238627599
##########
hugegraph-server/hugegraph-dist/src/assembly/static/bin/hugegraph-server.sh:
##########
@@ -252,13 +261,90 @@ if [ "${OPEN_TELEMETRY}" == "true" ]; then
export OTEL_RESOURCE_ATTRIBUTES=service.name=server
fi
-if [[ "${STDOUT_MODE:-false}" != "true" ]]; then
- # Daemon stderr only reaches hugegraph-server-stdout.log, so a bootstrap
- # rejection of the effective DNS policy (a broken operator override
- # included) would be invisible in the log start-hugegraph.sh points
- # operators at. Let the bootstrap mirror its fatal errors there.
- JVM_OPTIONS="${JVM_OPTIONS} -Dhugegraph.bootstrap.error.log=${OUTPUT}"
-fi
+# Let the bootstrap mirror its fatal errors (a rejected DNS policy, a broken
+# operator override included) into hugegraph-server.log. In daemon mode its
+# stderr only reaches hugegraph-server-stdout.log; in STDOUT_MODE it reaches
the
+# container log, which keeps a single earlier generation, so the file copy is
+# what survives a crash loop on a mounted logs volume.
+JVM_OPTIONS="${JVM_OPTIONS} -Dhugegraph.bootstrap.error.log=${OUTPUT}"
+
+# Keep heap dumps and JVM crash logs in $LOGS whatever JAVA_OPTIONS holds. The
+# defaults come first in JAVA_TOOL_OPTIONS, ahead of the operator's own value.
The
+# JVM applies JAVA_TOOL_OPTIONS before JDK_JAVA_OPTIONS, the command line
+# (JAVA_OPTIONS and -j) and _JAVA_OPTIONS, and the last occurrence of a flag
wins,
+# so any of those overrides a default. The JVM does the parsing itself,
including
+# quoted options, and @argfiles in JDK_JAVA_OPTIONS or on the command line.
+# Child JVMs the Server starts (computer jobs, for example) inherit the
+# environment but not the command line, so they get these defaults too, and
every
+# path must stay unique per JVM. ErrorFile expands %p to each JVM's PID.
+# HeapDumpPath expands %p only from JDK 25, but on every version, when it
names an
+# existing directory, each JVM writes java_pid<its pid>.hprof inside it, so it
+# points at one directory per launch. HotSpot picks that name, so a child JVM
that
+# gets a PID reused within the same launch cannot write over an earlier child's
+# dump; the launcher cannot rename HotSpot's dump file.
+# A restarted container often reuses the PID; HotSpot truncates an existing
crash
+# log (JDK 17+) and will not write a heap dump over an existing file. So the
names
+# carry the host name (the pod name on Kubernetes, so pods sharing one log
volume
+# do not collide), the launch time, and a counter. A plain mkdir claims each
name
+# atomically, so concurrent launches never share one. This runs after every
+# preflight check, so a launch that fails before Java starts leaves nothing.
+# Dump directories are never removed automatically: one stays empty until a JVM
+# using it runs out of memory, and that JVM may be a computer job that outlives
+# the Server, so the launcher cannot tell when a directory is safe to delete.
+crash_name_taken() {
+ local name="$1" file restore_failglob taken=1
+ [[ -e "${LOGS}/heapdump_${name}" || -L "${LOGS}/heapdump_${name}" ]] &&
return 0
+ restore_failglob=$(shopt -p failglob)
+ shopt -u failglob
+ for file in "${LOGS}"/hs_err_pid*_"${name}".log; do
+ if [[ -e ${file} || -L ${file} ]]; then
+ taken=0
+ break
+ fi
+ done
+ eval "${restore_failglob}"
+ return ${taken}
+}
+case "${LOGS}" in
+ *\"*|*%*)
+ # The defaults below cannot carry this path: they quote it with double
quotes
+ # inside JAVA_TOOL_OPTIONS, and ErrorFile expands %. Keep the base
behaviour
+ # instead: heap dumps into $LOGS on the Server's command line, ahead of
+ # JAVA_OPTIONS so an operator value there still wins, and no ErrorFile
default.
+ report_error "WARN: ${LOGS} contains a double quote or %, so heap
dumps go to ${LOGS}\
+ without a per-launch directory and no crash log default is set; set
-XX:ErrorFile yourself"
+ JAVA_OPTIONS="-XX:+HeapDumpOnOutOfMemoryError -XX:HeapDumpPath=${LOGS}
${JAVA_OPTIONS}"
Review Comment:
Fixed in d3db8e73: the fallback now puts the heap dump flags at the front of
`JAVA_TOOL_OPTIONS` instead of the command line, quoting the path with
whichever quote character it does not contain (HotSpot accepts single quotes
there; I checked `"` and `%` paths on JDK 11 and 27). So
`JAVA_TOOL_OPTIONS=-XX:-HeapDumpOnOutOfMemoryError`, and every later source,
overrides it again. A path containing both quote characters gets only a
warning. The test checks the resolved values on a real JVM for `%`, `"`, `'`
with `%`, and both-quote paths, with and without the opt-out. Restoring the
command-line form fails it.
##########
hugegraph-server/hugegraph-dist/src/assembly/static/bin/hugegraph-server.sh:
##########
@@ -252,13 +261,90 @@ if [ "${OPEN_TELEMETRY}" == "true" ]; then
export OTEL_RESOURCE_ATTRIBUTES=service.name=server
fi
-if [[ "${STDOUT_MODE:-false}" != "true" ]]; then
- # Daemon stderr only reaches hugegraph-server-stdout.log, so a bootstrap
- # rejection of the effective DNS policy (a broken operator override
- # included) would be invisible in the log start-hugegraph.sh points
- # operators at. Let the bootstrap mirror its fatal errors there.
- JVM_OPTIONS="${JVM_OPTIONS} -Dhugegraph.bootstrap.error.log=${OUTPUT}"
-fi
+# Let the bootstrap mirror its fatal errors (a rejected DNS policy, a broken
+# operator override included) into hugegraph-server.log. In daemon mode its
+# stderr only reaches hugegraph-server-stdout.log; in STDOUT_MODE it reaches
the
+# container log, which keeps a single earlier generation, so the file copy is
+# what survives a crash loop on a mounted logs volume.
+JVM_OPTIONS="${JVM_OPTIONS} -Dhugegraph.bootstrap.error.log=${OUTPUT}"
+
+# Keep heap dumps and JVM crash logs in $LOGS whatever JAVA_OPTIONS holds. The
+# defaults come first in JAVA_TOOL_OPTIONS, ahead of the operator's own value.
The
+# JVM applies JAVA_TOOL_OPTIONS before JDK_JAVA_OPTIONS, the command line
+# (JAVA_OPTIONS and -j) and _JAVA_OPTIONS, and the last occurrence of a flag
wins,
+# so any of those overrides a default. The JVM does the parsing itself,
including
+# quoted options, and @argfiles in JDK_JAVA_OPTIONS or on the command line.
+# Child JVMs the Server starts (computer jobs, for example) inherit the
+# environment but not the command line, so they get these defaults too, and
every
+# path must stay unique per JVM. ErrorFile expands %p to each JVM's PID.
+# HeapDumpPath expands %p only from JDK 25, but on every version, when it
names an
+# existing directory, each JVM writes java_pid<its pid>.hprof inside it, so it
+# points at one directory per launch. HotSpot picks that name, so a child JVM
that
+# gets a PID reused within the same launch cannot write over an earlier child's
+# dump; the launcher cannot rename HotSpot's dump file.
+# A restarted container often reuses the PID; HotSpot truncates an existing
crash
+# log (JDK 17+) and will not write a heap dump over an existing file. So the
names
+# carry the host name (the pod name on Kubernetes, so pods sharing one log
volume
+# do not collide), the launch time, and a counter. A plain mkdir claims each
name
+# atomically, so concurrent launches never share one. This runs after every
+# preflight check, so a launch that fails before Java starts leaves nothing.
+# Dump directories are never removed automatically: one stays empty until a JVM
+# using it runs out of memory, and that JVM may be a computer job that outlives
+# the Server, so the launcher cannot tell when a directory is safe to delete.
+crash_name_taken() {
+ local name="$1" file restore_failglob taken=1
+ [[ -e "${LOGS}/heapdump_${name}" || -L "${LOGS}/heapdump_${name}" ]] &&
return 0
+ restore_failglob=$(shopt -p failglob)
+ shopt -u failglob
+ for file in "${LOGS}"/hs_err_pid*_"${name}".log; do
+ if [[ -e ${file} || -L ${file} ]]; then
+ taken=0
+ break
+ fi
+ done
+ eval "${restore_failglob}"
+ return ${taken}
+}
+case "${LOGS}" in
+ *\"*|*%*)
+ # The defaults below cannot carry this path: they quote it with double
quotes
+ # inside JAVA_TOOL_OPTIONS, and ErrorFile expands %. Keep the base
behaviour
+ # instead: heap dumps into $LOGS on the Server's command line, ahead of
+ # JAVA_OPTIONS so an operator value there still wins, and no ErrorFile
default.
+ report_error "WARN: ${LOGS} contains a double quote or %, so heap
dumps go to ${LOGS}\
+ without a per-launch directory and no crash log default is set; set
-XX:ErrorFile yourself"
+ JAVA_OPTIONS="-XX:+HeapDumpOnOutOfMemoryError -XX:HeapDumpPath=${LOGS}
${JAVA_OPTIONS}"
+ ;;
+ *)
+ LAUNCH_HOST=$(printf '%s' "${HOSTNAME:-localhost}" | LC_ALL=C tr -c
'A-Za-z0-9._-' '_')
+ LAUNCH_HOST="${LAUNCH_HOST:0:64}"
+ LAUNCH_STAMP="${LAUNCH_HOST}_$(date +%Y%m%d-%H%M%S)"
+ LAUNCH_ID="${LAUNCH_STAMP}"
+ LAUNCH_SUFFIX=0
+ HEAP_DUMP_PATH=""
+ while [[ -z ${HEAP_DUMP_PATH} ]]; do
+ if ! crash_name_taken "${LAUNCH_ID}" &&
+ mkdir "${LOGS}/heapdump_${LAUNCH_ID}" 2>/dev/null; then
+ HEAP_DUMP_PATH="${LOGS}/heapdump_${LAUNCH_ID}"
+ elif [[ ${LAUNCH_SUFFIX} -ge 1000 ]]; then
+ report_error "WARN: heap dump names for ${LAUNCH_STAMP} are
all taken; heap dumps go to ${LOGS}"
+ HEAP_DUMP_PATH="${LOGS}"
+ elif ! crash_name_taken "${LAUNCH_ID}"; then
+ # Not a name clash (a full disk, for example): keep starting,
and dump
+ # into $LOGS itself rather than make diagnostics a startup
requirement.
+ report_error "WARN: cannot create
${LOGS}/heapdump_${LAUNCH_ID}; heap dumps go to ${LOGS}"
+ HEAP_DUMP_PATH="${LOGS}"
+ else
+ LAUNCH_SUFFIX=$((LAUNCH_SUFFIX + 1))
+ LAUNCH_ID="${LAUNCH_STAMP}-${LAUNCH_SUFFIX}"
+ fi
+ done
+ CRASH_OPTIONS="-XX:+HeapDumpOnOutOfMemoryError"
+ CRASH_OPTIONS="${CRASH_OPTIONS} \"-XX:HeapDumpPath=${HEAP_DUMP_PATH}\""
+ CRASH_OPTIONS="${CRASH_OPTIONS}
\"-XX:ErrorFile=${LOGS}/hs_err_pid%p_${LAUNCH_ID}.log\""
Review Comment:
Documented in d3db8e73. HotSpot's `ErrorFile` only expands `%p`, so the
launcher cannot give inherited child JVMs a name beyond the PID. The launcher
comment, `docker/README.md` and the doc#509 quickstart (EN/CN) now say that a
JVM reusing an earlier PID within the same start replaces that JVM's crash log,
next to the matching heap dump caveat.
--
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]