bitflicker64 commented on code in PR #3258:
URL: https://github.com/apache/hugegraph/pull/3258#discussion_r4227376355
##########
hugegraph-server/hugegraph-dist/src/assembly/static/bin/hugegraph-server.sh:
##########
@@ -248,13 +257,84 @@ 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.
+# 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 %.
+ report_error "WARN: ${LOGS} contains a double quote or %, so the heap
dump and crash log\
+ defaults are not set; set -XX:HeapDumpPath and -XX:ErrorFile yourself"
+ ;;
+ *)
+ 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}\""
Review Comment:
Important: Every JVM in one launch shares this HeapDumpPath directory, and
inside it HotSpot names the file only java_pid<pid>.hprof. A long-running
Server can start many computer-job JVMs (AbstractComputer uses ProcessBuilder
and the child inherits JAVA_TOOL_OPTIONS). If one child dumps on OOM and a
later child gets the same PID, the later dump is not written, because, as the
comment above says, HotSpot will not write over an existing file. The comment
at lines 273-274 says every path must stay unique per JVM, and that does not
hold for PID reuse within one launch. This is not a regression, since before
this PR child JVMs had no dump flags at all. Please either qualify the per-JVM
uniqueness claim here and in docker/README.md ("the Server and the JVMs it
starts write separate dumps"), or give children their own name, for example by
having AbstractComputer pass a per-job -XX:HeapDumpPath.
##########
hugegraph-server/hugegraph-dist/src/assembly/travis/test-java-security-properties.sh:
##########
@@ -559,6 +599,344 @@ assert_argument "true" "$ENABLED_CAPTURE"
assert_argument "-Doperator.marker=preserved" "$ENABLED_CAPTURE"
assert_no_argument '^-D(networkaddress\.cache\.ttl|sun\.net\.inetaddr\.ttl)=' \
"$ENABLED_CAPTURE"
+# Heap dump and crash log defaults go first in JAVA_TOOL_OPTIONS, so the JVM's
+# own precedence lets every operator source override them. Check the values a
+# real JVM settles on when started the way a captured launcher run would start
+# it: the JAVA_TOOL_OPTIONS, JDK_JAVA_OPTIONS and _JAVA_OPTIONS the launcher
+# passed on, and the crash-related -XX: arguments of its command line.
+effective_flag() {
+ local capture="$1"
+ local flag="$2"
+ local args=()
+ local line
+ local flags
+ while IFS= read -r line; do
+ if [[ "$line" =~
^-XX:([+-]HeapDumpOnOutOfMemoryError|HeapDumpPath=|ErrorFile=) ]]; then
+ args+=("$line")
+ fi
+ done < "$capture"
+ if ! flags=$(JAVA_TOOL_OPTIONS="$(cat "${capture}.tool-options")" \
+ JDK_JAVA_OPTIONS="$(cat "${capture}.jdk-java-options")" \
+ _JAVA_OPTIONS="$(cat "${capture}.underscore-java-options")" \
+ "$JAVA_BIN" ${args[@]+"${args[@]}"} -XX:+PrintFlagsFinal
-version \
+ 2>"${capture}.flags.err"); then
+ fail "JVM rejected the options captured in ${capture}: $(cat
"${capture}.flags.err")"
+ fi
+ # The value sits between "= " and the trailing "{origin}" columns and may
+ # contain spaces.
+ # A here-string, not a pipe: awk exits early, which would kill printf with
+ # SIGPIPE under pipefail.
+ awk -v flag="$flag" '$2 == flag {
+ sub(/^[^=]*= /, ""); sub(/ *(\{[^}]*\} *)+$/, ""); print; exit }' <<<
"$flags"
+}
+
+assert_effective_flag() {
+ local capture="$1"
+ local flag="$2"
+ local pattern="$3"
+ local value
+ value=$(effective_flag "$capture" "$flag")
+ [[ "$value" =~ $pattern ]] ||
+ fail "effective ${flag} is '${value}', expected to match ${pattern}"
+}
+
+LOGS_PATTERN=$(printf '%s' "${SERVER_ROOT}/logs" | sed
's/[][\.*^$+?(){}|]/\\&/g')
Review Comment:
Minor: LOGS_PATTERN is built from SERVER_ROOT, which line 22 resolves with a
plain `cd ... && pwd` and so keeps symlinks. hugegraph-server.sh builds LOGS
from abs_path, which uses `cd -P` and gives the physical path. When the
distribution is passed through a symlinked path, the captured -XX:HeapDumpPath
and -XX:ErrorFile hold the physical path and these assertions fail on a correct
launcher. On macOS /tmp is a symlink to /private/tmp, so a local run against a
distribution under /tmp hits this. Please resolve SERVER_ROOT with `pwd -P` at
line 22, or at least when building LOGS_PATTERN.
##########
docker/README.md:
##########
@@ -230,6 +230,36 @@ Standalone stores RocksDB data at
`/hugegraph-server/rocksdb-data`. The HStore t
`docker compose down` keeps named-volume data. `docker compose down -v`
intentionally deletes it.
+### Server logs and crash files
+
+This section describes images built from current master. Images from 1.7.0 and
earlier do not set `STDOUT_MODE`: `docker logs` shows only the entrypoint's
output, and the service logs stay in `logs/` inside the container.
+
+Both Server images (`hugegraph/hugegraph` and `hugegraph/server`) write the
HugeGraph log to container stdout. WARN and above from Hadoop, ZooKeeper, SOFA,
Netty and Commons also reaches stdout. INFO from Hadoop, Netty and Commons, the
audit log and the slow-query log stay in files only; ZooKeeper and SOFA log
nothing below WARN. Launcher errors that stop the Server before Java starts,
such as an unsupported JDK or too little free memory, and fatal bootstrap
errors go to stderr as well as `hugegraph-server.log`, so when a Server exits
during startup, `docker logs` or `kubectl logs --previous` shows why. Every
start also prints `Picked up JAVA_TOOL_OPTIONS: ...` with the crash-file
defaults below.
+
+`/hugegraph-server/logs` holds `hugegraph-server.log`, JVM crash logs
(`hs_err_pid<pid>_<host>_<launch time>.log`) and out-of-memory heap dumps
(`heapdump_<host>_<launch time>/java_pid<pid>.hprof`), where `<launch time>` is
the container's local time as `YYYYMMDD-HHMMSS`. Each start gets its own
`heapdump_*` directory, so the Server and the JVMs it starts, such as computer
jobs, write separate dumps. The host name (on Kubernetes, the pod name, or
`spec.hostname` when that is set) and a `-1`, `-2`, ... counter keep names from
clashing; the counter also separates pods that share a host name. If the
directory cannot be created, for example on a full volume, the Server still
starts and dumps go to `logs/` itself as `java_pid<pid>.hprof`. That is best
effort: a full volume may have no room for the dump either, and a restarted
container that reuses a PID cannot write over an earlier dump with the same
name.
+
+Both images declare `VOLUME /hugegraph-server`, so `docker restart` and a
Compose recreate keep the files in that anonymous volume. `docker compose down`
leaves the volume behind but the next `up` does not attach it, so the files
become hard to reach and keep using disk until `docker compose down -v` deletes
them; mount a named volume or a bind mount at `/hugegraph-server/logs` to keep
them reachable. Kubernetes starts a new container on every restart and does not
turn the image's `VOLUME` into a pod volume, so mount an `emptyDir` or a
PersistentVolumeClaim at `/hugegraph-server/logs`. Give each pod its own
volume, or a per-pod directory on a shared PVC: crash files get distinct names,
but `hugegraph-server.log`, the audit log and the slow-query log have fixed
names, and pods sharing them would roll them over for each other. For a per-pod
directory, pass the pod name in through the Downward API and use it in
`subPathExpr` (the Helm chart sets neither):
+
+```yaml
+env:
+ - name: POD_NAME
+ valueFrom:
+ fieldRef:
+ fieldPath: metadata.name
+volumeMounts:
+ - name: server-logs
+ mountPath: /hugegraph-server/logs
+ subPathExpr: $(POD_NAME)
+```
+
+The Helm chart in [`helm/`](../helm/hugegraph) does not mount a logs volume
yet, so with it these files are lost when a container restarts.
+
+A heap dump can be as large as the JVM heap. Every start uses a new directory
and the JVMs a Server starts can dump too, so a Server that keeps running out
of memory fills the volume with heap-sized files. Size the volume for the dumps
you want to keep plus the logs, and keep it within any `ephemeral-storage`
limit or `emptyDir` `sizeLimit`: eviction deletes an `emptyDir` together with
the dump in it. Do not use an `emptyDir` with `medium: Memory`, which counts
against the memory limit. A liveness probe that restarts the pod while a large
dump is being written leaves a truncated file.
+
+The launcher never deletes dumps or `heapdump_*` directories. Each start
leaves a directory behind, empty unless something ran out of memory; the
launcher keeps it because a computer-job JVM can go on using it after its
Server exits. Move or delete old directories once no HugeGraph JVM from that
launch is running, but never the newest one of a running Server: without its
directory, the next dump is written as a file with that name and later dumps
from that launch fail.
+
+To turn heap dumps off for every JVM, set
`JAVA_TOOL_OPTIONS=-XX:-HeapDumpOnOutOfMemoryError`, which also keeps the
image's default `JAVA_OPTS`. Set it in the container's environment, not in the
host shell: `docker run -e JAVA_TOOL_OPTIONS=...`, an `environment:` entry on
the Server service in Compose (the shipped Compose files do not pass it
through), or `env` on the container in Kubernetes. Putting the flag in
`JAVA_OPTS` turns dumps off for the Server JVM only, because the JVMs it starts
do not see its command line, and setting `JAVA_OPTS` replaces the image default
(`-XX:+UseContainerSupport -XX:MaxRAMPercentage=50 ...`), so repeat those
flags. Either way, each start still creates its empty `heapdump_*` directory.
Review Comment:
Minor: "Either way, each start still creates its empty heapdump_* directory"
is not true on the fallback path described at line 237 and implemented in
hugegraph-server.sh lines 315-326. When mkdir of heapdump_* fails, HeapDumpPath
is set to logs/ and no directory is created. It is also not true when LOGS
contains a double quote or %, because the defaults are then skipped entirely.
Please qualify the sentence, for example: "each start that can create it still
creates its empty heapdump_* directory". The paired hugegraph-doc#509 text
needs the same change.
--
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]
