bitflicker64 commented on code in PR #2945: URL: https://github.com/apache/hugegraph/pull/2945#discussion_r4298031827
########## hugegraph-server/hugegraph-dist/src/assembly/static/bin/arthas.sh: ########## @@ -0,0 +1,115 @@ +#!/bin/bash +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +# Offline attachment to a running Server or Store using the bundled agent. +set -euo pipefail + +fail() { echo "Error: $*" >&2; exit 1; } +usage() { + echo "Usage: $0 [--pid PID] [--telnet-port PORT] [--command COMMAND]" + echo "Run as the service OS user; defaults to bin/pid and loopback port 3658." + echo "New agents disable HTTP/MCP; use the existing telnet port for an active agent." + echo "Exiting this helper stops Arthas, including after --command or quit; the JVM keeps running." +} + +BIN=$(cd -P "$(dirname "${BASH_SOURCE[0]}")" && pwd) +TOP=$(cd -P "$BIN/.." && pwd) +pid="" +port=3658 +boot_args=() +while [ "$#" -gt 0 ]; do + case "$1" in + --pid|--telnet-port|--command) + [ "$#" -ge 2 ] || fail "Missing value for $1" + case "$1" in + --pid) pid=$2 ;; + --telnet-port) port=$2 ;; + --command) boot_args=(--command "$2") ;; + esac + shift 2 ;; + -h|--help) usage; exit 0 ;; + *) usage >&2; fail "Unknown option: $1" ;; + esac +done +if [ -z "$pid" ]; then + [ -r "$BIN/pid" ] || fail "No bin/pid; start the service or supply --pid PID" + pid=$(cat "$BIN/pid") +fi +[[ "$pid" =~ ^[1-9][0-9]{0,9}$ ]] || fail "PID must be a positive integer" +[[ "$port" =~ ^[1-9][0-9]{0,4}$ ]] && [ "$port" -le 65535 ] || fail "Invalid telnet port" +JAVA=${JAVA_HOME:+$JAVA_HOME/bin/}java +ARTHAS="$TOP/lib/arthas" +[ -f "$ARTHAS/arthas-boot.jar" ] || fail "Bundled Arthas files are missing from lib/arthas" +boot_args+=(--arthas-home "$ARTHAS" --target-ip 127.0.0.1 --telnet-port "$port" --http-port -1 "$pid") +# Keep the listener within this diagnosis session. Verify its PID before stopping +# it, since an occupied port must never stop a different application's agent. +state_dir=$(mktemp -d) +client_pid="" +attaching=true +# The native client's -t option does not bound its final response wait. +bounded_client() { + "$JAVA" -jar "$ARTHAS/arthas-client.jar" -t 5000 -c "$1" 127.0.0.1 "$port" & + local command_pid=$! + (sleep 10; kill -KILL "$command_pid" 2>/dev/null) >/dev/null 2>&1 & + local timer_pid=$! + local status=0 + wait "$command_pid" || status=$? + kill "$timer_pid" 2>/dev/null || true + wait "$timer_pid" 2>/dev/null || true + return "$status" +} +stop_agent() { + local session + if ! session=$(bounded_client session 2>&1); then + if [[ "$session" != *"Connection refused"* ]]; then + echo "Warning: could not verify Arthas cleanup; check port $port and stop it manually" >&2 + fi + return 0 + fi + if [[ "$session" =~ JAVA_PID[[:space:]]+$pid([[:space:]]|$) ]]; then + bounded_client stop >/dev/null 2>&1 || + echo "Warning: could not stop Arthas; run the helper with --command stop" >&2 + fi +} +cleanup() { + trap - EXIT INT TERM HUP Review Comment: Important: under an interactive bash or zsh, a terminal hangup (an SSH drop) does not finish cleanup, and the agent keeps listening. The helper gets SIGHUP twice within a millisecond. On a pty with `bash -i` (with and without job control) and `zsh -i`, closing the pty left the port open every time. An xtrace shows the second HUP either re-entering the trap before `cleanup` starts, or killing `cleanup` after `trap -` here restored the default. Ctrl-C 0.5 s after `quit` does the same. Please ignore further signals once exit starts: `trap 'trap "" INT TERM HUP; exit 129' HUP` (likewise INT and TERM), and `trap '' INT TERM HUP; trap - EXIT` here. Line 71 then needs `kill -KILL "$timer_pid"`, since the timer subshell inherits the ignored TERM and every exit would wait 20 s. With both changes all these cases close the port and test-arthas-cli.sh still passes. ########## hugegraph-store/Dockerfile: ########## @@ -35,7 +35,8 @@ RUN --mount=type=cache,id=hugegraph-maven-${SOURCE_REVISION},target=/root/.m2,sh # 2nd stage: runtime env # Note: ZGC (The Z Garbage Collector) is only supported on ARM-Mac with java > 13 -FROM eclipse-temurin:17-jre-jammy +# Local Arthas attach requires the JDK attach and compiler modules. Review Comment: Important: this comment line makes `docker / docker-build` fail at this head. The step "Verify shared Maven stages stay identical" takes each Dockerfile from `FROM ... AS build` up to the next `FROM` and diffs it against hugegraph-pd/Dockerfile. This line sits before the runtime `FROM`, so it lands in the extracted build stage. The job log shows exactly that diff: `+# Local Arthas attach requires the JDK attach and compiler modules.` The same awk reproduces it locally for all three files. The image build and Compose smoke steps later in that job pass on the JDK base, so only this check is red. The two Server Dockerfiles have the same line 38. Please move the comment below the `FROM eclipse-temurin:17-jdk-jammy` line in all three files, or drop it. ########## hugegraph-server/hugegraph-dist/src/assembly/static/bin/arthas.sh: ########## @@ -0,0 +1,115 @@ +#!/bin/bash +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +# Offline attachment to a running Server or Store using the bundled agent. +set -euo pipefail + +fail() { echo "Error: $*" >&2; exit 1; } +usage() { + echo "Usage: $0 [--pid PID] [--telnet-port PORT] [--command COMMAND]" + echo "Run as the service OS user; defaults to bin/pid and loopback port 3658." + echo "New agents disable HTTP/MCP; use the existing telnet port for an active agent." + echo "Exiting this helper stops Arthas, including after --command or quit; the JVM keeps running." +} + +BIN=$(cd -P "$(dirname "${BASH_SOURCE[0]}")" && pwd) +TOP=$(cd -P "$BIN/.." && pwd) +pid="" +port=3658 +boot_args=() +while [ "$#" -gt 0 ]; do + case "$1" in + --pid|--telnet-port|--command) + [ "$#" -ge 2 ] || fail "Missing value for $1" + case "$1" in + --pid) pid=$2 ;; + --telnet-port) port=$2 ;; + --command) boot_args=(--command "$2") ;; + esac + shift 2 ;; + -h|--help) usage; exit 0 ;; + *) usage >&2; fail "Unknown option: $1" ;; + esac +done +if [ -z "$pid" ]; then + [ -r "$BIN/pid" ] || fail "No bin/pid; start the service or supply --pid PID" + pid=$(cat "$BIN/pid") +fi +[[ "$pid" =~ ^[1-9][0-9]{0,9}$ ]] || fail "PID must be a positive integer" +[[ "$port" =~ ^[1-9][0-9]{0,4}$ ]] && [ "$port" -le 65535 ] || fail "Invalid telnet port" +JAVA=${JAVA_HOME:+$JAVA_HOME/bin/}java +ARTHAS="$TOP/lib/arthas" +[ -f "$ARTHAS/arthas-boot.jar" ] || fail "Bundled Arthas files are missing from lib/arthas" +boot_args+=(--arthas-home "$ARTHAS" --target-ip 127.0.0.1 --telnet-port "$port" --http-port -1 "$pid") +# Keep the listener within this diagnosis session. Verify its PID before stopping +# it, since an occupied port must never stop a different application's agent. +state_dir=$(mktemp -d) +client_pid="" +attaching=true +# The native client's -t option does not bound its final response wait. +bounded_client() { + "$JAVA" -jar "$ARTHAS/arthas-client.jar" -t 5000 -c "$1" 127.0.0.1 "$port" & + local command_pid=$! + (sleep 10; kill -KILL "$command_pid" 2>/dev/null) >/dev/null 2>&1 & + local timer_pid=$! + local status=0 + wait "$command_pid" || status=$? + kill "$timer_pid" 2>/dev/null || true + wait "$timer_pid" 2>/dev/null || true + return "$status" +} +stop_agent() { + local session + if ! session=$(bounded_client session 2>&1); then + if [[ "$session" != *"Connection refused"* ]]; then + echo "Warning: could not verify Arthas cleanup; check port $port and stop it manually" >&2 + fi + return 0 + fi + if [[ "$session" =~ JAVA_PID[[:space:]]+$pid([[:space:]]|$) ]]; then + bounded_client stop >/dev/null 2>&1 || + echo "Warning: could not stop Arthas; run the helper with --command stop" >&2 + fi +} +cleanup() { + trap - EXIT INT TERM HUP + if [ -n "$client_pid" ]; then + # Let an in-flight loadAgent finish before checking/closing its listener. + if [ "$attaching" = false ]; then + kill "$client_pid" 2>/dev/null || true + fi + wait "$client_pid" 2>/dev/null || true + fi + if [ -f "$state_dir/validated" ]; then + stop_agent + fi + rm -f "$state_dir/validated" + rmdir "$state_dir" +} +trap cleanup EXIT +trap 'exit 130' INT +trap 'exit 143' TERM +trap 'exit 129' HUP +"$JAVA" --add-modules jdk.attach --class-path "$ARTHAS/arthas-core.jar" \ + "$BIN/ArthasAttach.java" "$pid" "$ARTHAS" "$port" "$TOP" "$state_dir/validated" <&0 & +client_pid=$! +wait "$client_pid" +attaching=false +"$JAVA" -jar "$ARTHAS/arthas-boot.jar" "${boot_args[@]}" <&0 & Review Comment: Minor: when stdin is not a terminal and reaches EOF, the client does not exit, so the helper and the listener stay up until Arthas evicts the idle session. `bin/arthas.sh < /dev/null` and `printf 'version\n' | bin/arthas.sh` both kept the port open for the 90 s I watched. With `arthas.sessionTimeout=15` the helper exited after about 64 s and cleanup closed the port, so with the shipped 1800 s this lasts about 30 minutes. Likely triggers are piped commands instead of `--command`, or `docker exec` / `ssh host bin/arthas.sh` without a terminal. Consider refusing interactive mode without a terminal, for example `[ -t 0 ] || [ -n "$command" ] || fail "Interactive use needs a terminal; use --command"`, with `command` set by `--command`. -- 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]
