fix(bitcoind): make liveness probe optional and shutdown grace configurable for IBD - #9261
fix(bitcoind): make liveness probe optional and shutdown grace configurable for IBD#9261openoms wants to merge 2 commits into
Conversation
…urable for IBD During initial block download bitcoind holds the RPC lock through long validation stretches, so the liveness exec probe times out and kubelet repeatedly kills a healthy, progressing node. Each kill also SIGKILLed the shutdown UTXO flush after the default 30s grace, forcing a 20-40 min chainstate replay on restart (measured on a zrh1 signet node: 5 kills in 90 min, net sync progress near zero). Add livenessProbe.enabled (default true) and terminationGracePeriodSeconds (default 600). Syncing deployments set enabled: false and re-enable once at the tip.
blink-claw-bot
left a comment
blink-claw-bot
left a comment
There was a problem hiding this comment.
Review: fix(bitcoind): make liveness probe optional and shutdown grace configurable for IBD
Overall: Looks good. Clean, focused fix for a real operational problem — IBD nodes getting killed by liveness probes and then losing UTXO cache on ungraceful shutdown. The PR description clearly explains the problem and the measured impact (5 kills in 90 min, near-zero sync progress on signet).
File-by-file
charts/bitcoind/values.yaml
- ✅
terminationGracePeriodSeconds: 600— sensible default. 10 minutes is generous for UTXO flush; matches common practice for bitcoind operators. The comment explaining why is helpful. - ✅
livenessProbe.enabled: true— safe default (existing behavior preserved), with clear documentation on when and why to disable it during IBD. - The comments in
values.yamlare well-written and explain the operational context. Good.
charts/bitcoind/templates/statefulset.yaml
- ✅
terminationGracePeriodSeconds: {{ .Values.terminationGracePeriodSeconds }}— correctly placed at the pod spec level. - ✅
{{- if .Values.livenessProbe.enabled }}/{{- end }}— correctly wraps only the livenessProbe block, leaving readinessProbe and startupProbe untouched. The{{-trimming is consistent with the rest of the template.
charts/bitcoind/Chart.yaml
- ✅ Version bump
0.6.1-dev→0.6.2-dev— appropriate for a values/template change.
Minor observations (non-blocking)
- nit: The
terminationGracePeriodSecondsvalue could also benefit from being documented in a README or chart docs if those exist, since operators doing IBD will need to know about both this andlivenessProbe.enabled: falsetogether. - The readinessProbe is left always-on, which makes sense — readiness doesn't trigger restarts, it just removes the pod from service endpoints. Good call leaving it as-is.
Verdict
No blocking issues found. The defaults preserve existing behavior, the new knobs solve a well-documented operational pain point, and the implementation is minimal and correct.
Approving.
🤖 Review by Claude Opus 4.6 via Blink-Claw-Bot
The exporter polls bitcoind over RPC and exits when RPC answers slowly, which crash-loops the sidecar during initial block download (observed 3 restarts during a signet IBD) while adding RPC load bitcoind can least afford. Add exporter.enabled (default true); syncing deployments disable it and re-enable once at the tip. The metrics Service port and prometheus.io scrape annotations follow the toggle so disabled deployments leave no dead scrape target.
blink-claw-bot
left a comment
blink-claw-bot
left a comment
There was a problem hiding this comment.
Review Summary
Clean, well-motivated change. The PR description clearly explains the problem (liveness probe killing healthy bitcoind during IBD, SIGKILL during UTXO flush causing chainstate replay) and the solution is appropriately minimal — toggle flags with backward-compatible defaults.
LOW
1. PR scope slightly wider than title suggests
The title says "make liveness probe optional and shutdown grace configurable" but the PR also introduces exporter.enabled to gate the prometheus sidecar, metrics service port, and scrape annotations. The exporter toggle is related (exporter also polls RPC during IBD), and the description mentions it indirectly, but consider updating the title/description to call it out explicitly — e.g. fix(bitcoind): make liveness probe and exporter optional, shutdown grace configurable for IBD.
2. Environment-specific values files don't demonstrate the new knobs
signet-values.yml is a natural candidate for showcasing livenessProbe.enabled: false and exporter.enabled: false since signet deployments often start from scratch. Not blocking — the defaults are correct for tip-synced nodes, and operators can override — but an example in the signet/testnet values would be useful documentation.
What looks good
- Backward compatible: all new values default to the existing behavior (
enabled: true,terminationGracePeriodSeconds: 600is more generous than the k8s default of 30s but harmless for synced nodes). - Comments are accurate and useful: they explain the why (RPC lock during validation, UTXO flush on shutdown), not just the what.
- Exporter gating is thorough: service port, pod annotations, and sidecar container are all consistently wrapped.
- Chart version bumped appropriately.
readinessProbeintentionally left always-on — correct, since readiness doesn't kill the pod.
LGTM — no blocking issues.
🤖 Review by Claude Opus 4.6 via Blink-Claw-Bot
3 participants
During initial block download bitcoind holds the RPC lock through long validation stretches, so the liveness exec probe times out and kubelet repeatedly kills a healthy, progressing node. Each kill also SIGKILLed the shutdown UTXO flush after the default 30s grace, forcing a 20-40 min chainstate replay on restart (measured on a signet node: 5 kills in 90 min, net sync progress near zero).
Add livenessProbe.enabled (default true) and terminationGracePeriodSeconds (default 600). Syncing deployments set enabled: false and re-enable once at the tip.