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


##########
hugegraph-server/hugegraph-dist/src/assembly/static/bin/hugegraph-server.sh:
##########
@@ -121,13 +121,41 @@ if [ "$JAVA_OPTIONS" = "" ]; then
         echo "Failed to start HugeGraphServer, requires at least ${MIN_MEM}MB 
free memory" >> "${OUTPUT}"
         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.
+# A restarted container often reuses the JVM's PID, and HotSpot will not 
overwrite
+# an existing crash log or heap dump, so the file names carry the launch time 
plus
+# a counter that skips names already used in $LOGS. The script execs java 
below,
+# so $$ is the JVM's PID.
+crash_files_exist() {
+    local file
+    for file in "${LOGS}"/java_pid*_"$1".hprof "${LOGS}"/hs_err_pid*_"$1".log; 
do
+        [[ -e ${file} ]] && return 0
+    done
+    return 1
+}
+LAUNCH_STAMP=$(date +%Y%m%d-%H%M%S)
+LAUNCH_ID="${LAUNCH_STAMP}"
+LAUNCH_SUFFIX=0
+while crash_files_exist "${LAUNCH_ID}"; do
+    LAUNCH_SUFFIX=$((LAUNCH_SUFFIX + 1))
+    LAUNCH_ID="${LAUNCH_STAMP}-${LAUNCH_SUFFIX}"
+done
+CRASH_OPTIONS="-XX:+HeapDumpOnOutOfMemoryError"
+CRASH_OPTIONS="${CRASH_OPTIONS} 
\"-XX:HeapDumpPath=${LOGS}/java_pid$$_${LAUNCH_ID}.hprof\""

Review Comment:
   Documented in c29edae1. `docker/README.md` now says each launch writes its 
heap dump to a new file, so a Server that keeps hitting OOM adds a heap-sized 
dump per restart until the volume is full, and that nothing removes old dumps 
on an `emptyDir` or a PVC. It says to size the volume for the number of dumps 
to keep plus logs, move or delete old `java_pid*.hprof` files after each 
incident, or turn dumps off with `-XX:-HeapDumpOnOutOfMemoryError`. I left out 
a built-in retention limit, since that would have the launcher delete files on 
the operator's volume. Happy to add one as a follow-up if you prefer.



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