gortiz opened a new pull request, #19372: URL: https://github.com/apache/pinot/pull/19372
## What Adds continuous [Java Flight Recorder](https://docs.oracle.com/en/java/javase/21/jfapi/) recording to the Helm chart, per role, and deprecates the `pinot.jfr.*` cluster configs that did the same job from inside the JVM. The point is post-mortem profiling: when a server falls over, the profile of the minutes leading up to it should already be on disk rather than something you go and enable afterwards. ```yaml server: jfr: enabled: true jfr: # shared by every role that enables it configuration: default # ~1% overhead maxSize: 2Gi persistence: enabled: false # true keeps recordings across pod rescheduling ``` That renders into the role's `JAVA_OPTS`: ``` -XX:FlightRecorderOptions=repository=/var/pinot/jfr,preserve-repository=false,maxchunksize=12582912 -XX:StartFlightRecording=name=pinot,settings=default,disk=true,maxsize=2147483648,dumponexit=false ``` ## Why JVM arguments rather than the existing `pinot.jfr.*` cluster configs `ContinuousJfrStarter` starts the recording after the component has connected to Helix. Starting it from the command line is better in three ways: - **Coverage.** The listener cannot run before cluster config has been read, so class loading, plugin init, segment preload and the ZooKeeper connect itself are never captured. - **Availability.** It needs ZooKeeper reachable, which is not a safe assumption during the incidents a profile would help with. - **Data loss.** `JFR.stop` without a filename deletes the whole repository, so changing any `pinot.jfr.*` key — including one that only affects cleanup — silently discards every chunk recorded so far. Nothing is removed. The configs are `@Deprecated(forRemoval = true)`, still work, and warn once naming the JVM arguments to use instead. ## Why there is an init container JFR's `maxsize` makes the recording roll like a log file, so nothing needs to rotate files by hand. What JFR does *not* do is reclaim the repository of a JVM that has already exited — and `preserve-repository=true`, which is what keeps recordings across a restart in the first place, means those directories survive on a PersistentVolume forever. Measured: a second run against the same repository root leaves the first run's directory untouched, so each restart leaks up to a full `maxSize`. `jfr-janitor` reclaims them. Running it as an init container is what makes it safe — init containers finish before the Pinot container starts, so every repository it sees belongs to a run that is already over. It also refuses to delete anything written to within `jfr.janitor.minIdleMinutes`, and always exits 0: a failed cleanup must never keep a role from starting. ## Design notes - **Its own volume.** Never a subdirectory of the role's data volume, so a runaway recording cannot eat the space Pinot needs for segments. - **`jfr.persistence.enabled` defaults to false.** An `emptyDir` applies with a plain rolling restart; a PersistentVolume adds a `volumeClaimTemplates` entry, which Kubernetes forbids changing in place. `UPGRADING.md` has the one-time `--cascade=orphan` procedure. - **The stateless minion always uses an `emptyDir`.** It is a Deployment, so replicas cannot each have their own volume, and sharing one would let a starting pod's cleanup delete a running pod's live recording during a rolling update. - **One unit convention.** Every size is a Kubernetes quantity; the chart converts to the byte counts JFR accepts. JFR's own unit table never reaches `values.yaml`, and the cleanup script parses no units at all. - **Values validated at render time.** A bad JFR option is not a warning — the JVM refuses to start — so the chart fails `helm install` with a clear message instead of leaving a CrashLoopBackOff. That includes checking the volume can hold the janitor's budget plus the runs that follow it. ## Getting a recording out ```bash # snapshot a running JVM, without interrupting the recording kubectl exec <pod> -- jcmd 1 JFR.dump name=pinot filename=/tmp/snap.jfr # after a crash: rebuild from the repository left behind, including the chunk # that was still open when the JVM died kubectl exec <pod> -- jfr assemble /var/pinot/jfr/<repository-dir> /tmp/crash.jfr ``` Each `*.jfr` chunk in the repository is a valid recording on its own and is named with its start timestamp, so you can pull only the window you care about instead of the whole volume. ## Testing - `helm lint --strict`, `helm template` and `kubeconform -strict` over six configurations: defaults, all five roles on with `emptyDir`, all five with a PVC, janitor off, a user-supplied `initContainers` entry alongside the janitor, and a large `profile` setup. - With JFR disabled the rendered manifests are identical to master except that an empty `initContainers: []` key is no longer emitted. - Negative cases assert `helm template` *fails*: `maxSize: 500m` (a lowercase `m` is *milli* in Kubernetes), `maxSize: 2GB`, `maxAge: P7D`, a janitor with neither bound set, and an over-committed volume. - `helm/pinot/scripts/jfr-janitor-test.sh` — 13 fixture checks over the cleanup script: both passes, non-repository entries left alone, a recently written repository never deleted, a malformed budget skipping the pass rather than deleting everything, and exit 0 on a read-only or missing directory. `shellcheck -s sh` clean. - `ContinuousJfrStarterTest` — 29 cases, including a data provider over the JVM-argument detection (both flags, both `=`/`:` forms, and `-Dsomething=-XX:StartFlightRecording` which must *not* match). - End to end on JDK 25: ran a JVM with the flags the chart renders, confirmed the repository layout, recovered it with `jfr assemble`, and ran the janitor extracted from the rendered manifest against the result. ## Notes for reviewers - The chart has no CI job today (`helm lint`/`helm template` appear in none of the workflows). Happy to add one in a follow-up if that is wanted — it would have caught two bugs found during review. - `@Deprecated(forRemoval = true)` names no removal release. Suggestions welcome on which one to target. -- 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]
