Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions scripts/rolling-update.env.example
Original file line number Diff line number Diff line change
Expand Up @@ -51,3 +51,10 @@ SSH_STRICT_HOST_KEY_CHECKING="accept-new"
RAFTADMIN_REMOTE_BIN="/tmp/elastickv-raftadmin"
RAFTADMIN_RPC_TIMEOUT_SECONDS="5"
RAFTADMIN_ALLOW_INSECURE="true"

# OOM defenses applied on 2026-04-24 after kernel OOM-SIGKILL cascades.
# GOMEMLIMIT makes Go GC before the container hits --memory; --memory keeps
# any kill scoped to the container, not host processes. Set either to "" to
# opt out. User EXTRA_ENV keys override matching keys in DEFAULT_EXTRA_ENV.
DEFAULT_EXTRA_ENV="GOMEMLIMIT=1800MiB"
CONTAINER_MEMORY_LIMIT="2500m"
76 changes: 74 additions & 2 deletions scripts/rolling-update.sh
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,18 @@ Optional environment:
Each pair must be KEY=VALUE with a non-empty KEY; pairs themselves must not
contain whitespace.

DEFAULT_EXTRA_ENV defaults to "GOMEMLIMIT=1800MiB" (Go runtime soft memory
ceiling; GC works harder before approaching the hard --memory limit so the
kernel OOM killer is not triggered). Merged with EXTRA_ENV before forwarding;
if a user-supplied EXTRA_ENV entry sets the same KEY, the user value wins.
Set DEFAULT_EXTRA_ENV="" to disable the default.

CONTAINER_MEMORY_LIMIT
docker run --memory value (default: 2500m). Hard container-scoped memory
ceiling; any OOM kill is contained to the elastickv container rather than
cascading to host processes (e.g. qemu-guest-agent, systemd). Paired with
GOMEMLIMIT=1800MiB so Go GC preempts the kill. Set to "" to disable.

Notes:
- If RAFT_TO_REDIS_MAP is unset, it is derived automatically from NODES,
RAFT_PORT, and REDIS_PORT.
Expand Down Expand Up @@ -113,6 +125,9 @@ SSH_TARGETS="${SSH_TARGETS:-}"
ROLLING_ORDER="${ROLLING_ORDER:-}"
RAFT_TO_REDIS_MAP="${RAFT_TO_REDIS_MAP:-}"
RAFT_TO_S3_MAP="${RAFT_TO_S3_MAP:-}"
# Container OOM defenses. See usage() for rationale. Empty string disables.
DEFAULT_EXTRA_ENV="${DEFAULT_EXTRA_ENV-GOMEMLIMIT=1800MiB}"
CONTAINER_MEMORY_LIMIT="${CONTAINER_MEMORY_LIMIT-2500m}"

if [[ -z "$NODES" ]]; then
echo "NODES is required" >&2
Expand Down Expand Up @@ -427,6 +442,7 @@ update_one_node() {
RAFT_TO_REDIS_MAP="$RAFT_TO_REDIS_MAP_Q" \
RAFT_TO_S3_MAP="$RAFT_TO_S3_MAP_Q" \
EXTRA_ENV="$EXTRA_ENV_Q" \
CONTAINER_MEMORY_LIMIT="$CONTAINER_MEMORY_LIMIT_Q" \
bash -s <<'REMOTE'
set -euo pipefail

Expand Down Expand Up @@ -707,10 +723,20 @@ run_container() {
done
fi

# Optional hard container-scoped memory limit. Keeps any OOM kill contained
# to the elastickv container rather than cascading to host processes
# (e.g. qemu-guest-agent, systemd). Pair with GOMEMLIMIT via EXTRA_ENV so
# the Go runtime GCs before the kernel kills the container.
local memory_flags=()
if [[ -n "${CONTAINER_MEMORY_LIMIT:-}" ]]; then
memory_flags=(--memory "$CONTAINER_MEMORY_LIMIT")
fi

docker run -d \
--name "$CONTAINER_NAME" \
--restart unless-stopped \
--network host \
"${memory_flags[@]}" \
-v "$DATA_DIR:$DATA_DIR" \
"${s3_creds_volume[@]}" \
"${extra_env_flags[@]}" \
Expand Down Expand Up @@ -868,9 +894,55 @@ ensure_remote_raftadmin_binaries
# CR handling additionally covers deploy.env files edited on Windows.
# `${EXTRA_ENV:-}` is required because `set -u` is active and EXTRA_ENV
# may be unset (the variable is optional in deploy.env).
EXTRA_ENV_NORMALISED="${EXTRA_ENV:-}"
EXTRA_ENV_NORMALISED="${EXTRA_ENV_NORMALISED//[$'\t\r\n']/ }"
# Merge DEFAULT_EXTRA_ENV (operator-safety defaults like GOMEMLIMIT) with any
# user-supplied EXTRA_ENV. User-supplied KEYs win over defaults for the same
# KEY; the remote parser forwards pairs via `-e KEY=VALUE` so docker evaluates
# the last occurrence, which means pre-pending defaults is correct: later user
# entries override earlier defaults. We still de-duplicate here so the printed
# command line stays clean.
EXTRA_ENV_USER_NORMALISED="${EXTRA_ENV:-}"
EXTRA_ENV_USER_NORMALISED="${EXTRA_ENV_USER_NORMALISED//[$'\t\r\n']/ }"
EXTRA_ENV_DEFAULT_NORMALISED="${DEFAULT_EXTRA_ENV:-}"
EXTRA_ENV_DEFAULT_NORMALISED="${EXTRA_ENV_DEFAULT_NORMALISED//[$'\t\r\n']/ }"

merge_extra_env() {
local defaults="$1"
local user="$2"
# Portable across Bash 3.2 (macOS default) which lacks associative
# arrays: concatenate user KEYs into a space-padded string and match
# with " KEY " to test set membership. The EXTRA_ENV list is typically
# a handful of entries, so the linear check is negligible.
local -a user_pairs=()
local -a default_pairs=()
local pair key seen=" " merged=""

read -r -a user_pairs <<< "$user"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Using read -a with a here-string (<<<) on an empty variable will return a non-zero exit status in some Bash versions (like 3.2 on macOS) when set -e is active, causing the script to terminate prematurely if EXTRA_ENV is unset. Since set -euo pipefail is active at the top of the script, this is a high-risk operation. It is safer to check if the variable is non-empty before attempting to read it into an array.

Suggested change
read -r -a user_pairs <<< "$user"
if [[ -n "$user" ]]; then
read -r -a user_pairs <<< "$user"
fi

for pair in "${user_pairs[@]}"; do
[[ -n "$pair" ]] || continue
[[ "$pair" == *=* ]] || continue
key="${pair%%=*}"
seen+="${key} "
done

read -r -a default_pairs <<< "$defaults"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Similar to the user variable, reading from defaults via a here-string can trigger set -e if the variable is empty. While DEFAULT_EXTRA_ENV has a non-empty default in this PR, a user could explicitly set it to an empty string to opt out, which would then crash the script here.

Suggested change
read -r -a default_pairs <<< "$defaults"
if [[ -n "$defaults" ]]; then
read -r -a default_pairs <<< "$defaults"
fi

for pair in "${default_pairs[@]}"; do
[[ -n "$pair" ]] || continue
[[ "$pair" == *=* ]] || continue
Comment on lines +959 to +960
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Validate malformed DEFAULT_EXTRA_ENV entries

merge_extra_env silently drops default tokens that don’t contain = and only forwards the remaining pairs for downstream validation, so a typo in DEFAULT_EXTRA_ENV (for example GOMEMLIMIT1800MiB) makes the rollout continue without the intended safeguard and without any error. This differs from EXTRA_ENV, where malformed entries fail fast, and can hide configuration mistakes in production rollouts.

Useful? React with 👍 / 👎.

key="${pair%%=*}"
if [[ "$seen" != *" ${key} "* ]]; then
merged+="${merged:+ }$pair"
fi
done
for pair in "${user_pairs[@]}"; do
[[ -n "$pair" ]] || continue
merged+="${merged:+ }$pair"
done
printf '%s' "$merged"
}

EXTRA_ENV_NORMALISED="$(merge_extra_env "$EXTRA_ENV_DEFAULT_NORMALISED" "$EXTRA_ENV_USER_NORMALISED")"
EXTRA_ENV_Q="$(printf '%q' "$EXTRA_ENV_NORMALISED")"
CONTAINER_MEMORY_LIMIT_Q="$(printf '%q' "${CONTAINER_MEMORY_LIMIT:-}")"
S3_CREDENTIALS_FILE_Q="$(printf '%q' "${S3_CREDENTIALS_FILE:-}")"
IMAGE_Q="$(printf '%q' "$IMAGE")"
DATA_DIR_Q="$(printf '%q' "$DATA_DIR")"
Expand Down
Loading