feat(chart): make s3proxy probes configurable (tolerant liveness)#95
Merged
Merged
Conversation
c4dcab7 to
000a9cb
Compare
000a9cb to
c498991
Compare
…a busy loop)
The s3proxy container's startup/liveness/readiness probes were hardcoded in the
deployment template, so a deployment couldn't loosen them. The proxy runs a
single uvicorn worker (one asyncio event loop); under heavy upload load a probe
can take seconds to be served, and the tight 5s liveness timeout then restarts a
live pod -> in-flight uploads drop -> clients retry -> load spiral -> cluster-wide
crashloop (observed in production under backup load).
Expose all three probes via .Values.{startupProbe,livenessProbe,readinessProbe}
with the current values as defaults (toYaml; maps merge, so a deployment can set
just e.g. livenessProbe.timeoutSeconds without restating the probe). Lets
argocd raise the liveness timeout to absorb a busy loop instead of killing it.
(Drops the earlier crypto-offload approach: empirically, cryptography's AES-GCM
holds the GIL, so asyncio.to_thread did not free the event loop -- to_thread and
inline gave the same ~9-10 loop ticks during a 110ms encrypt. The real lever is
probe tolerance + capacity, not offload.)
c498991 to
f7a6fad
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Under backup upload load the s3proxy pods entered a self-sustaining
CrashLoopBackOffcascade:Container … failed liveness probe, will be restarted(pods serving fine then SIGTERM'd, exit 0) → in-flight uploads dropped → barman/scylla retried → backlog grew → next pod hit harder → cascade. It persisted after removing the scylla workload and was not fixed by raising the memory governor.Root mechanism: the proxy runs a single uvicorn worker (one asyncio event loop), so under load
/healthzcan't be served within the 5 s liveness timeout, and kubelet kills a live pod. The probes were hardcoded in the deployment template — no way for a deployment to loosen them.What
Expose the s3proxy container's
startupProbe/livenessProbe/readinessProbevia.Values(defaults = current values, rendered withtoYaml). Helm merges maps, so a deployment can set justlivenessProbe.timeoutSeconds: 60without restating the whole probe. This lets argocd raise the liveness timeout to absorb a busy loop instead of restarting it.helm lintclean;helm template --set livenessProbe.timeoutSeconds=60renders the merged probe correctly.Note — the crypto-offload approach was dropped
The earlier version of this PR offloaded AES-GCM to
asyncio.to_thread. Empirically that does not help:cryptography's AES-GCM holds the GIL, so the worker thread blocks the loop thread anyway. Measured loop ticks during a 110 ms encrypt: baseline idle104, inline9,to_thread10— i.e. to_thread ≈ inline. So the real levers are probe tolerance (this PR) + capacity (workers/CPU, deploy-side, needs a matching pod-memory bump to avoid OOM), not offload.