bitflicker64 commented on code in PR #3258:
URL: https://github.com/apache/hugegraph/pull/3258#discussion_r4233439780
##########
hugegraph-server/hugegraph-dist/src/assembly/static/bin/hugegraph-server.sh:
##########
@@ -248,13 +257,86 @@ 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
+ *\"*|*%*)
+ # JAVA_TOOL_OPTIONS has no escape for a double quote, and ErrorFile
expands %.
Review Comment:
Important: this branch drops heap dumps altogether, which is a regression
for installs whose logs path contains `%` or `"`. At the base, the
`JAVA_OPTIONS` default (old line 130) always passed
`-XX:+HeapDumpOnOutOfMemoryError -XX:HeapDumpPath=${LOGS}`; this PR removes
those from line 140 and relies on this block, so here the Server gets no
`HeapDumpOnOutOfMemoryError` at all, only a WARN. I ran lines 290-339 with
`LOGS=<dir>/a%b/logs`: `JAVA_TOOL_OPTIONS` stays empty. On JDK 17 the base
flags with that same path write `java_pid<pid>.hprof` into it, since
`HeapDumpPath` does not expand `%` before JDK 25. The comment's premise is also
not quite right: HotSpot accepts single quotes in `JAVA_TOOL_OPTIONS`, and
`'-XX:HeapDumpPath=<dir>/q"x/logs'` resolves with the double quote intact
(checked with `-XX:+PrintFlagsFinal` on JDK 17). Please keep
`-XX:+HeapDumpOnOutOfMemoryError` and a `HeapDumpPath` in this branch
(single-quoted when the path holds `"`, and at least for JDK < 25 when it
holds `%`), and skip only the `ErrorFile` default when `%` is present; or fall
back to the base behaviour by keeping the two flags in the `JAVA_OPTIONS`
default for this case. A test case in `test-java-security-properties.sh` with a
`%` in the logs path would cover it.
--
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]
