Jackie-Jiang commented on code in PR #20247: URL: https://github.com/apache/pinot/pull/20247#discussion_r4558715165
########## compatibility-verifier/server-readiness.inc: ########## @@ -0,0 +1,138 @@ +# +# 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. +# + +# Shared by compCheck.sh and the readiness regression tests. Each server start gets its own +# log directory, including both servers in the MSE suite. +function startServerWithLogs() { + local serviceName=$1 + local launcher=$2 + local configFile=$3 + local serverLogDir="${LOG_DIR}/${serviceName}.${logCount}" + mkdir -p "$serverLogDir" || exit 1 + echo "$serverLogDir" > "${PID_DIR}/${serviceName}.logdir" + ( + # The default logging configuration includes both server and Helix logs. Preserve the working + # directory so relative paths in a custom server config still refer to the same data on restart. + export LOG_ROOT="$serverLogDir" + export PINOT_COMPONENT="$serviceName" + if [ -f "$configFile" ]; then + exec "$launcher" StartServer -configFileName "$configFile" + else + exec "$launcher" StartServer + fi + ) > "${LOG_DIR}/${serviceName}.${logCount}.log" 2>&1 & + echo $! > "${PID_DIR}/${serviceName}.pid" +} + +function waitForServerReadiness() { + local serviceName=$1 + local port=$2 + local timeoutSeconds=${SERVER_READY_TIMEOUT_SECONDS:-600} + local pid serverLogDir remaining requestTimeout + case "$timeoutSeconds" in + ''|*[!0-9]*|0) echo "SERVER_READY_TIMEOUT_SECONDS must be a positive integer"; return 1 ;; + esac + pid=$(cat "${PID_DIR}/${serviceName}.pid") || return 1 + serverLogDir=$(cat "${PID_DIR}/${serviceName}.logdir") || return 1 + timeoutSeconds=$((10#$timeoutSeconds)) + if [ "$timeoutSeconds" -le 0 ]; then + echo "SERVER_READY_TIMEOUT_SECONDS must be a positive integer" + return 1 + fi + local deadline=$((SECONDS + timeoutSeconds)) + while [ "$SECONDS" -lt "$deadline" ]; do + if ! kill -0 "$pid" 2>/dev/null; then + echo "$serviceName (pid $pid) exited before becoming ready" + return 1 + fi + remaining=$((deadline - SECONDS)) + if [ "$remaining" -le 0 ]; then + break + fi + requestTimeout=5 + if [ "$remaining" -lt "$requestTimeout" ]; then + requestTimeout=$remaining + fi + echo "Checking port $port for $serviceName readiness" + # curl normally exits successfully for HTTP 503. Require a successful health response, + # and keep each request within the remaining overall startup budget. + if curl --fail --silent --show-error --connect-timeout 2 --max-time "$requestTimeout" \ + --write-out '\nHTTP %{http_code}\n' "http://localhost:${port}/health"; \ + >> "$serverLogDir/readiness.log" 2>&1; then + return 0 + fi + sleep 1 + done + echo "$serviceName did not become ready within ${timeoutSeconds}s" + return 1 +} + +function collectServerDiagnostics() { + local serviceName=$1 + local port=$2 + local pid serverLogDir dumpPid dumpDeadline jstackCommand + serverLogDir=$(cat "${PID_DIR}/${serviceName}.logdir" 2>/dev/null) || return 0 + pid=$(cat "${PID_DIR}/${serviceName}.pid" 2>/dev/null) || return 0 + echo "Collecting diagnostics for $serviceName (pid $pid) in $serverLogDir" + ps -p "$pid" -o pid,ppid,stat,args > "$serverLogDir/process.txt" 2>&1 + curl --silent --show-error --connect-timeout 2 --max-time 5 \ + --write-out '\nHTTP %{http_code}\n' "http://localhost:${port}/health"; \ + > "$serverLogDir/health-on-failure.txt" 2>&1 + if ! kill -0 "$pid" 2>/dev/null; then + echo "Server process has exited; no thread dump available" > "$serverLogDir/threads.txt" + return 0 + fi + jstackCommand=jstack + if [ -n "${JAVA_HOME:-}" ] && [ -x "$JAVA_HOME/bin/jstack" ]; then + jstackCommand="$JAVA_HOME/bin/jstack" + fi + # Attach before stopServices kills the JVM, but do not let a stuck attach block cleanup. + "$jstackCommand" -l "$pid" > "$serverLogDir/threads.txt" 2>&1 & Review Comment: Fixed in 6ae154001151eae63da0c0f896491e7aa696f8d3. Verifier servers now use a synchronous logging configuration with `immediateFlush=true`, preserving the existing SIGKILL behavior for upgrades, rollbacks, and cleanup. Added a child-JVM regression that logs final server and Helix messages, calls `Runtime.halt()`, and verifies both messages survived. The logging configuration also passed abrupt-exit probes on Log4j 2.25.3 and 2.26.1. All 23 Java tests, seven readiness tests, and required style/license checks passed. -- 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]
