Add per-role continuous JFR recording to the Helm chart

helm/pinot/UPGRADING.md

@@ -21,6 +21,59 @@
 
 # Upgrading the Pinot Helm Chart
 
+## From 1.0.x to 1.0.1
+
+### Continuous JFR profiling moved from cluster config to JVM arguments
+
+The chart can now run a continuous Java Flight Recorder recording per role, started by
+`-XX:StartFlightRecording` in `JAVA_OPTS`. Enable it with `<role>.jfr.enabled` and tune the shared
+`jfr` block; see the "Continuous profiling with Java Flight Recorder" section of the README.
+
+JFR is off by default, so upgrading changes nothing until you opt in.
+
+#### Enabling it is in-place; keeping recordings on a volume is not
+
+With the defaults, `<role>.jfr.enabled: true` only changes `spec.template`, so it applies with a
+normal rolling restart. Recordings go to an `emptyDir` and are lost when the pod is rescheduled.
+
+Setting `jfr.persistence.enabled: true` is the part that cannot be done in place: it adds an entry
+to that StatefulSet's `volumeClaimTemplates`, and **Kubernetes forbids changing that field**, so
+`helm upgrade` fails with:
+
+```
+updates to statefulset spec for fields other than 'replicas', 'ordinals', 'template',
+'updateStrategy', 'persistentVolumeClaimRetentionPolicy' and 'minReadySeconds' are forbidden
+```
+
+Turning it back off fails the same way. If you need the PersistentVolume, delete the StatefulSet
+without touching the pods, then upgrade:
+
+```bash
+kubectl delete statefulset <release>-pinot-server --cascade=orphan
+helm upgrade <release> ./helm/pinot \
+  --set server.jfr.enabled=true --set jfr.persistence.enabled=true
+```
+
+`--cascade=orphan` leaves the running pods alone; the recreated StatefulSet adopts them, and the new
+`jfr` volume is attached as each pod is rolled. Do this per role. A role's existing `data` PVCs are
+untouched either way.
+
+#### If you use the `pinot.jfr.*` cluster configs
+
+Those are deprecated and will be removed in a future Pinot release. They still work, but they start
+the recording only after the component has connected to Helix — so JVM startup is never captured —
+and changing any `pinot.jfr.*` key restarts the recording, which discards all history recorded so
+far.
+
+**Remove the `pinot.jfr.*` cluster configs before enabling `<role>.jfr.enabled`.** This is a
+prerequisite, not a preference. Running both means the old code path issues
+`JFR.configure repositorypath=...`, which is JVM-global: it relocates the recording the JVM started,
+off the volume the chart provisioned for it.
+
+Pinot 1.6.0 and later detect this and ignore the deprecated configs with a warning. **Earlier images
+do not** — and the chart happily renders the JVM arguments for whatever `image.tag` you have pinned,
+so on a pre-1.6.0 image the collision above is exactly what you get.
+
 ## From 0.x to 1.0.0
 
 Version 1.0.0 replaces the Bitnami ZooKeeper subchart with native Helm

helm/pinot/scripts/jfr-janitor-test.sh

@@ -0,0 +1,145 @@
+#!/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.
+#
+# Fixture tests for jfr-janitor.sh. Run: sh helm/pinot/scripts/jfr-janitor-test.sh
+
+set -u
+
+script_dir=$(CDPATH='' cd -- "$(dirname -- "$0")" && pwd)
+janitor="$script_dir/jfr-janitor.sh"
+work=$(mktemp -d)
+trap 'rm -rf "$work"' EXIT
+
+failures=0
+checks=0
+
+check() {
+  checks=$(( checks + 1 ))
+  if [ "$2" = "$3" ]; then
+    echo "ok   - $1"
+  else
+    echo "FAIL - $1"
+    echo "         expected: $3"
+    echo "         actual:   $2"
+    failures=$(( failures + 1 ))
+  fi
+}
+
+# `date` takes a different flag for relative times on BSD (macOS) and GNU (the container).
+if date -v-1M +%Y >/dev/null 2>&1; then
+  minutes_ago() { date -v"-$1M" +%Y%m%d%H%M; }
+else
+  minutes_ago() { date -d "-$1 minutes" +%Y%m%d%H%M; }
+fi
+
+# make_repo <parent> <name> <kib> [minutes_in_the_past]
+make_repo() {
+  _dir="$1/$2"
+  mkdir -p "$_dir"
+  dd if=/dev/zero of="$_dir/chunk.jfr" bs=1024 count="$3" 2>/dev/null
+  if [ "${4:-0}" -gt 0 ]; then
+    _stamp=$(minutes_ago "$4")
+    touch -t "$_stamp" "$_dir/chunk.jfr" "$_dir"
+  fi
+}
+
+# Names of surviving entries, sorted, space separated. The fixtures use plain names, so `ls` is
+# fine here and is the portable option.
+# shellcheck disable=SC2012
+survivors() {
+  ls -1 "$1" 2>/dev/null | sort | tr '\n' ' ' | sed 's/ $//'
+}
+
+run() {
+  _repo="$1"; shift
+  env PINOT_JFR_REPOSITORY="$_repo" \
+      PINOT_JFR_JANITOR_MAX_AGE_MINUTES="${AGE:-}" \
+      PINOT_JFR_JANITOR_MAX_TOTAL_KIB="${BUDGET:-}" \
+      PINOT_JFR_JANITOR_MIN_IDLE_MINUTES="${IDLE:-15}" \
+      sh "$janitor" "$@" > "$work/out.txt" 2>&1
+  echo "$?" > "$work/rc.txt"
+}
+
+# --- age pass -----------------------------------------------------------------------------------
+r="$work/age"; mkdir -p "$r"
+make_repo "$r" 2026_08_01_10_00_00_1 512 20000
+make_repo "$r" 2026_08_20_10_00_00_1 512 60
+AGE=10080 BUDGET='' IDLE=15 run "$r"
+check "age pass drops only the aged-out repository" "$(survivors "$r")" "2026_08_20_10_00_00_1"
+check "age pass exits 0" "$(cat "$work/rc.txt")" "0"
+
+# --- size pass ----------------------------------------------------------------------------------
+r="$work/size"; mkdir -p "$r"
+make_repo "$r" 2026_08_01_10_00_00_1 1024 20000
+make_repo "$r" 2026_08_02_10_00_00_1 1024 20000
+make_repo "$r" 2026_08_03_10_00_00_1 1024 20000
+AGE='' BUDGET=2048 IDLE=15 run "$r"
+check "size pass trims oldest-first down to the budget" \
+  "$(survivors "$r")" "2026_08_02_10_00_00_1 2026_08_03_10_00_00_1"
+
+# --- non-repository entries are never touched ---------------------------------------------------
+r="$work/foreign"; mkdir -p "$r/lost+found" "$r/operator-scratch"
+: > "$r/lost+found/keep"; : > "$r/notes.txt"
+make_repo "$r" 2026_08_01_10_00_00_1 1024 20000
+AGE=1 BUDGET=1 IDLE=15 run "$r"
+check "foreign files and directories survive" \
+  "$(survivors "$r")" "lost+found notes.txt operator-scratch"
+
+# --- a recently written repository is never deleted ---------------------------------------------
+r="$work/live"; mkdir -p "$r"
+make_repo "$r" 2026_08_01_10_00_00_1 1024 0
+AGE=1 BUDGET=1 IDLE=15 run "$r"
+check "a live repository survives both passes" "$(survivors "$r")" "2026_08_01_10_00_00_1"
+check "and the overage is reported" \
+  "$(grep -c 'still .* against' "$work/out.txt")" "1"
+
+# --- a malformed budget must never mean 'delete everything' -------------------------------------
+r="$work/badbudget"; mkdir -p "$r"
+make_repo "$r" 2026_08_01_10_00_00_1 1024 20000
+make_repo "$r" 2026_08_02_10_00_00_1 1024 20000
+AGE='' BUDGET='4GiB' IDLE=15 run "$r"
+check "unparseable budget skips the size pass instead of deleting" \
+  "$(survivors "$r")" "2026_08_01_10_00_00_1 2026_08_02_10_00_00_1"
+check "unparseable budget still exits 0" "$(cat "$work/rc.txt")" "0"
+check "unparseable budget is reported" "$(grep -c 'unusable size budget' "$work/out.txt")" "1"
+
+# --- empty budget simply skips the pass ---------------------------------------------------------
+r="$work/nobudget"; mkdir -p "$r"
+make_repo "$r" 2026_08_01_10_00_00_1 1024 20000
+AGE='' BUDGET='' IDLE=15 run "$r"
+check "empty budget skips the size pass" "$(survivors "$r")" "2026_08_01_10_00_00_1"
+
+# --- degraded environments must not fail the init container -------------------------------------
+AGE=1 BUDGET=1 IDLE=15 run "$work/does-not-exist"
+check "missing repository directory exits 0" "$(cat "$work/rc.txt")" "0"
+
+r="$work/readonly"; mkdir -p "$r"
+make_repo "$r" 2026_08_01_10_00_00_1 1024 20000
+chmod 500 "$r"
+AGE=1 BUDGET=1 IDLE=15 run "$r"
+rc=$(cat "$work/rc.txt")
+chmod 700 "$r"
+check "an unremovable repository still exits 0" "$rc" "0"
+
+env PINOT_JFR_REPOSITORY= sh "$janitor" > "$work/out.txt" 2>&1
+check "unset repository exits 0" "$?" "0"
+
+echo
+echo "$checks checks, $failures failure(s)"
+[ "$failures" -eq 0 ]

helm/pinot/scripts/jfr-janitor.sh

@@ -0,0 +1,168 @@
+#!/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.
+#
+# Reclaims JFR repositories left behind by previous JVM runs.
+#
+# JFR's own `maxsize` bounds the repository of the JVM that is running. Nothing inside the JVM ever
+# reclaims the repository of a JVM that has already exited, so with `preserve-repository=true` those
+# directories accumulate on the volume until it is full. This script deletes them.
+#
+# It runs as an init container, which is what makes it safe: init containers finish before the Pinot
+# container starts, so the repositories it sees belong to runs that are already over. It still
+# refuses to touch anything written to recently, so that it stays safe if the volume is ever shared.
+#
+# Nothing here is required for Pinot to run. Every failure is tolerated and the script always exits
+# 0: blocking a Pinot role from starting because a cleanup failed would be far worse than leaving a
+# stale recording on disk.
+#
+# All sizes and durations arrive already converted to plain integers by the Helm chart, so this
+# script parses no units. That keeps a single source of truth for the unit table and means a
+# malformed value can never turn the size pass into "delete everything".
+#
+# Inputs (environment):
+#   PINOT_JFR_REPOSITORY                 directory holding the per-run repositories (required)
+#   PINOT_JFR_JANITOR_MAX_AGE_MINUTES    drop repositories older than this many minutes; empty skips
+#   PINOT_JFR_JANITOR_MAX_TOTAL_KIB      trim oldest-first until under this many KiB; empty skips
+#   PINOT_JFR_JANITOR_MIN_IDLE_MINUTES   never touch a repository written to this recently
+
+repo="${PINOT_JFR_REPOSITORY:-}"
+max_age_minutes="${PINOT_JFR_JANITOR_MAX_AGE_MINUTES:-}"
+max_total_kib="${PINOT_JFR_JANITOR_MAX_TOTAL_KIB:-}"
+min_idle="${PINOT_JFR_JANITOR_MIN_IDLE_MINUTES:-15}"
+
+# A JFR repository directory is named `<yyyy_MM_dd_HH_mm_ss>_<pid>`. Matching that pattern keeps the
+# janitor away from anything else sharing the volume (`lost+found`, an operator's scratch file), and
+# makes a lexicographic sort a chronological one.
+pattern='[0-9][0-9][0-9][0-9]_[0-9][0-9]_[0-9][0-9]_*'
+
+log() {
+  echo "jfr-janitor: $*"
+}
+
+# A whole number, and nothing else. Anything the chart failed to convert is treated as "not set"
+# rather than as zero: a zero budget would mean "delete everything".
+is_positive_int() {
+  case "${1:-}" in
+    '' | *[!0-9]*) return 1 ;;
+    *) return 0 ;;
+  esac
+}
+
+# Size of a path in KiB; 0 if it cannot be read.
+kib() {
+  size=$(du -sk "$1" 2>/dev/null | awk 'NR == 1 { print $1 }') || size=""
+  is_positive_int "$size" || size=0
+  echo "$size"
+}
+
+# True when a repository may still belong to a live JVM.
+#
+# JFR flushes at least once a second, so a live repository has a recently modified chunk file. The
+# directory's own mtime is checked too, which covers the window between a JVM creating its
+# repository and writing the first chunk into it.
+in_use() {
+  if [ -n "$(find "$1" -type f -mmin "-$min_idle" 2>/dev/null | head -1)" ]; then
+    return 0
+  fi
+  [ -n "$(find "$1" -maxdepth 0 -mmin "-$min_idle" 2>/dev/null)" ]
+}
+
+reclaim_by_age() {
+  is_positive_int "$max_age_minutes" || return 0
+  log "dropping repositories older than $max_age_minutes minutes"
+  find "$repo" -mindepth 1 -maxdepth 1 -type d -name "$pattern" -mmin "+$max_age_minutes" \
+    2>/dev/null | sort > "$candidates" || return 0
+  while IFS= read -r dir; do
+    [ -d "$dir" ] || continue
+    if in_use "$dir"; then
+      log "WARN skipping $dir: written to within the last $min_idle minutes"
+      continue
+    fi
+    log "removing $dir (aged out)"
+    rm -rf "$dir" || log "WARN could not remove $dir"
+  done < "$candidates"
+}
+
+reclaim_by_size() {
+  if ! is_positive_int "$max_total_kib"; then
+    if [ -n "$max_total_kib" ]; then
+      log "WARN ignoring unusable size budget '$max_total_kib'; skipping the size pass"
+    fi
+    return 0
+  fi
+  used=$(kib "$repo")
+  log "budget is $max_total_kib KiB, $used KiB in use"
+  find "$repo" -mindepth 1 -maxdepth 1 -type d -name "$pattern" 2>/dev/null | sort \
+    > "$candidates" || return 0
+  matched=0
+  skipped=0
+  while IFS= read -r dir; do
+    if [ "$used" -le "$max_total_kib" ]; then
+      break
+    fi
+    [ -d "$dir" ] || continue
+    matched=$(( matched + 1 ))
+    if in_use "$dir"; then
+      log "WARN skipping $dir: written to within the last $min_idle minutes"
+      skipped=$(( skipped + 1 ))
+      continue
+    fi
+    size=$(kib "$dir")
+    log "removing $dir ($size KiB, over budget)"
+    if rm -rf "$dir"; then
+      used=$(( used - size ))
+    else
+      log "WARN could not remove $dir"
+    fi
+  done < "$candidates"
+  if [ "$used" -gt "$max_total_kib" ]; then
+    if [ "$matched" -eq 0 ]; then
+      log "WARN still $used KiB over a $max_total_kib KiB budget and nothing matched '$pattern';" \
+          "the JFR repository naming may have changed, or $repo holds data this script does not own"
+    else
+      log "WARN still $used KiB against a $max_total_kib KiB budget after cleanup" \
+          "($skipped repositories skipped as recently written); the volume may fill"
+    fi
+  fi
+}
+
+main() {
+  if [ -z "$repo" ]; then
+    log "WARN PINOT_JFR_REPOSITORY is not set; nothing to do"
+    return 0
+  fi
+  mkdir -p "$repo" 2>/dev/null || true
+  if [ ! -d "$repo" ]; then
+    log "WARN $repo does not exist and could not be created; nothing to do"
+    return 0
+  fi
+
+  # Sorted candidates go to a file rather than a pipeline so the loops run in this shell and can
+  # keep a running total.
+  candidates="${TMPDIR:-/tmp}/jfr-janitor-candidates.$$"
+  trap 'rm -f "$candidates"' EXIT
+
+  log "$repo holds $(kib "$repo") KiB before cleanup"
+  reclaim_by_age
+  reclaim_by_size
+  log "$repo holds $(kib "$repo") KiB after cleanup"
+}
+
+main || log "WARN cleanup did not complete; continuing so that Pinot can start"
+exit 0