Skip to content

fix(deployment): increase time before LAPIS readiness probe fails to 1min from 30s #7298

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
corneliusroemer-agent wants to merge 3 commits into
base: main
Choose a base branch
from
+2 −2
Open

fix(deployment): increase time before LAPIS readiness probe fails to 1min from 30s #7298

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2f8573c
fix(deployment): let the LAPIS readiness probe ride out a brief SILO …
corneliusroemer-agent Sep 10, 2026
7a58311
Apply suggestion from @corneliusroemer
corneliusroemer Sep 10, 2026
714e462
Merge branch 'main' into fix/lapis-readiness-probe-tolerate-silo-blip
corneliusroemer Sep 18, 2026
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
4 changes: 2 additions & 2 deletions kubernetes/loculus/templates/lapis-deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ spec:
httpGet:
path: /sample/info
port: 8080
periodSeconds: 10
failureThreshold: 3
periodSeconds: 5
failureThreshold: 12
Comment on lines +59 to +60

@claude claude Bot Sep 10, 2026

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.

Halving the period doubles how often each pod runs a real /sample/info → SILO round trip: ~0.8 → ~1.6 probes/s across eight organisms in CI. That is small in absolute terms, but it is extra work on exactly the code path whose latency is the problem, on pods that request 100m CPU. The 10s / failureThreshold: 6 variant mentioned in the description buys the identical 60s tolerance without adding load; the only thing it gives up is halved recovery time after an eviction. Worth being explicit about which of the two you're optimising for, since the added load pushes slightly in the direction of the failure being mitigated.

Also a nit: the neighbouring failureThreshold: 36 # 3 minutes to start carries its arithmetic inline. With the long comment dropped, 5 × 12 has no in-file record of meaning 60s — a trailing # 60s would keep the two probes readable side by side.

timeoutSeconds: 5
livenessProbe:

@claude claude Bot Sep 10, 2026

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.

The liveness probe below is left at 10s × 3 = 30s, so it is now stricter than the readiness probe it is meant to outlive. Two ways that bites:

  1. If /actuator/health also consults SILO (the 100 ms budget in SiloClient.kt was reportedly introduced precisely so the health endpoint wouldn't block), then the same bursts that trip readiness can trip liveness — and liveness crossing its threshold restarts the container after 30s, well before readiness reaches 60s. The mitigation would be replaced by a worse failure mode: a restart plus a fresh startupProbe cycle instead of a ~10s eviction.
  2. Even if /actuator/health is purely local, the PR's own diagnosis is that the stall is LAPIS-side under contention (resources.lapis.requests.cpu is 100m, 30m on the preview server, no CPU limit). A JVM starved enough to blow a 100 ms budget can also blow the liveness probe's timeoutSeconds: 5.

Raising failureThreshold to 6 on the liveness probe (60s, same tolerance) would make it impossible for liveness to fire first.

I couldn't verify from this job whether LAPIS's actuator health contributes a SILO indicator (no network access here) — if you can confirm it doesn't, point 2 alone is still a reason to widen it. Fix this →

httpGet:
Expand Down
Loading