byteayan commented on code in PR #3258:
URL: https://github.com/apache/hugegraph/pull/3258#discussion_r4203372362


##########
hugegraph-server/hugegraph-dist/src/assembly/static/bin/hugegraph-server.sh:
##########
@@ -110,30 +120,73 @@ JAVA_VERSION=$($JAVA -version 2>&1 |
 # Drop any pre-release suffix, e.g. "24-ea" -> "24"
 JAVA_VERSION="${JAVA_VERSION%%[!0-9]*}"
 if [[ -z $JAVA_VERSION || $JAVA_VERSION -lt $MIN_JAVA_VERSION ]]; then
-    echo "Make sure the JDK is installed and the version >= $MIN_JAVA_VERSION, 
current is $JAVA_VERSION" \
-         >> "${OUTPUT}"
+    report_error "Make sure the JDK is installed and the version >= 
$MIN_JAVA_VERSION, current is $JAVA_VERSION"
     exit 1
 fi
 
 if [[ ! -r ${JVM_MODULE_OPTIONS} ]]; then
-    echo "Missing or unreadable JVM module options file: 
${JVM_MODULE_OPTIONS}" >> "${OUTPUT}"
+    report_error "Missing or unreadable JVM module options file: 
${JVM_MODULE_OPTIONS}"
     exit 1
 fi
 
 # Set Java options
 if [ "$JAVA_OPTIONS" = "" ]; then
     XMX=$(calc_xmx $MIN_MEM $MAX_MEM)
     if [ $? -ne 0 ]; then
-        echo "Failed to start HugeGraphServer, requires at least ${MIN_MEM}MB 
free memory" >> "${OUTPUT}"
+        report_error "Failed to start HugeGraphServer, requires at least 
${MIN_MEM}MB free memory"
         exit 1
     fi
-    JAVA_OPTIONS="-Xms${MIN_MEM}m -Xmx${XMX}m -XX:+HeapDumpOnOutOfMemoryError 
-XX:HeapDumpPath=${LOGS} ${USER_OPTION}"
+    JAVA_OPTIONS="-Xms${MIN_MEM}m -Xmx${XMX}m ${USER_OPTION}"
 
     # Rolling out detailed GC logs
     #JAVA_OPTIONS="${JAVA_OPTIONS} -XX:+UseGCLogFileRotation 
-XX:GCLogFileSize=10M -XX:NumberOfGCLogFiles=3 \
     #              -Xloggc:./logs/gc.log -XX:+PrintHeapAtGC 
-XX:+PrintGCDetails -XX:+PrintGCDateStamps"
 fi
 
+# Keep heap dumps and JVM crash logs in $LOGS whatever JAVA_OPTIONS holds. The
+# defaults go first in JAVA_TOOL_OPTIONS, which the JVM reads before the 
operator's
+# own JAVA_TOOL_OPTIONS, JDK_JAVA_OPTIONS, the command line (JAVA_OPTIONS and 
-j)
+# and _JAVA_OPTIONS, so any of those overrides them. The JVM does that parsing
+# itself, including quoted options and @argfiles.
+# Child JVMs the Server starts (computer jobs, for example) inherit these 
options,
+# so every path must stay unique per JVM. ErrorFile expands %p to each JVM's 
PID.
+# HeapDumpPath does not expand %p, but when it names an existing directory 
each JVM
+# writes java_pid<its pid>.hprof inside it, so it points at a directory per 
launch.
+# A restarted container often reuses the PID, and HotSpot will not overwrite an
+# existing crash log or heap dump, 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 that skips names already used in $LOGS.
+crash_files_exist() {
+    [[ -e "${LOGS}/heapdump_$1" ]] && return 0
+    local file
+    for file in "${LOGS}"/hs_err_pid*_"$1".log; do
+        [[ -e ${file} ]] && return 0
+    done
+    return 1
+}
+LAUNCH_HOST=$(printf '%s' "${HOSTNAME:-localhost}" | tr -c 'A-Za-z0-9._-' '_')
+# Earlier launches on this host leave their heap dump directory behind even 
when
+# nothing was dumped. Remove those that are empty; rmdir never removes a dump.
+for HEAP_DUMP_DIR in "${LOGS}"/heapdump_"${LAUNCH_HOST}"_*/; do
+    [[ -d ${HEAP_DUMP_DIR} ]] && rmdir "${HEAP_DUMP_DIR}" 2>/dev/null

Review Comment:
   Fixed in ac68cf32. Each launch now writes `<dir>/.owner` with the JVM's PID 
and its process start time (the script execs java, so both are the JVM's). 
Startup removes a directory only when it holds nothing but `.owner` and no 
process with that PID and start time exists, so a PID reused after a container 
restart does not count as the old owner. Directories without a marker, with a 
dump, or owned by a running process are kept, and nothing is removed if `ps` 
cannot report a start time (both images install procps). The test's EXIT 
cleanup now removes only directories created during the run. Fixtures cover a 
dead owner (removed) plus a live owner, an unowned directory, another host's 
directory and one holding a dump (all kept). I also checked that empty 
directories owned by a separately running process, on this host and another, 
survive a full test run.



-- 
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]

Reply via email to