SteNicholas commented on code in PR #3855: URL: https://github.com/apache/celeborn/pull/3855#discussion_r4134396519
########## charts/celeborn/files/worker-drain.sh: ########## @@ -0,0 +1,119 @@ +#!/bin/sh +# +# 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. +# +# preStop hook for a Celeborn worker. Asks the worker to decommission when a scale-in is +# removing it, and to shut down gracefully otherwise. See the chart's README.md. +# +# Reads POD_NAME, POD_IP, STS_NAME, WORKER_HTTP_PORT and ALWAYS_DECOMMISSION from the +# environment. + +set -u + +SA=/var/run/secrets/kubernetes.io/serviceaccount + +# preStop output is not collected, so log through the main process. +log() { + _msg="celeborn preStop: $*" + if [ -w /proc/1/fd/1 ]; then + echo "$_msg" >/proc/1/fd/1 + else + echo "$_msg" + fi +} + +ORDINAL="" +DESIRED="" +EXIT_TYPE=GRACEFUL + +if [ "${ALWAYS_DECOMMISSION:-false}" = "true" ]; then + # Graceful shutdown persists state to recover from on restart. Where the worker's storage + # does not outlive the pod there is nothing to recover, so a rollout would drop in-flight + # shuffles - drain every time instead, whatever is removing this pod. + EXIT_TYPE=DECOMMISSION + log "alwaysDecommission is set - draining regardless of why this pod is going away" +else + # A scale-in must decommission, waiting for the shuffle data to expire. A rolling update or + # a node drain must not, or every pod replacement would block for hours. The statefulset + # controller lowers spec.replicas before deleting pods on a scale-in, so an ordinal at or + # above the desired count is being removed for good. + ORDINAL=${POD_NAME:-} + ORDINAL=${ORDINAL##*-} + + if [ -r "$SA/token" ]; then + URL="https://kubernetes.default.svc/apis/apps/v1/namespaces/$(cat "$SA/namespace")/statefulsets/${STS_NAME:-}/scale" + AUTH="Authorization: Bearer $(cat "$SA/token")" + if command -v curl >/dev/null 2>&1; then + SCALE=$(curl -sS --cacert "$SA/ca.crt" -H "$AUTH" "$URL" 2>/dev/null) Review Comment: Bound the HTTP requests before waiting for the drain. Could the scale lookup and exit POSTs have explicit connection and total request deadlines (including the wget path)? If the API server accepts the connection but stops responding, this curl call waits indefinitely, so the hook never reaches either the exit request or its graceful fallback. Kubernetes counts preStop against terminationGracePeriodSeconds, so the pod can spend the entire six-hour grace period here and then be killed without starting decommissioning. A stalled first exit endpoint likewise prevents trying loopback. Only the final wait after a successful exit request should be allowed to consume the drain budget. -- 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]
