SIGTERM is not handled: every deploy kills the process without draining or failing the running row (interim fix; the full drain is recorded as a limitation)
#50
Milestone v0.31.1. Reduced from #47 item 3 after the phase-1 investigation; verified directly in the code.
Problem (worse than #47 said).cmd/tapir/main.go:44 uses signal.NotifyContext(context.Background(), os.Interrupt): SIGINT only. Kubernetes sends SIGTERM, which is not handled, so on every deploy the process is killed (exit 143, which kubectl get pods shows as Error on the old pod) and none of the shutdown code runs: not Process's cancellation handling, not the HTTP server's Shutdown, not the worker's stop(). A comment at cmd/tapir/scheduler.go:131 claims "pod SIGTERM exits the loop cleanly": it does not. The running job is failed only later, by the NEXT pod's start-up sweep (FailInterruptedUploads), and during a rolling update that sweep fails the OLD pod's job within seconds (the surge pod starts first). Under SIGINT, ListenAndServe returns ErrServerClosed immediately, so cmdServe returns before Shutdown drains.
Numbers (investigation). A live 64-minute job took 2m53s (a 90-minute file about 4.5 min). Tapir CD redeploys on every push to main (4 ReplicaSets in 4.4 hours today). A deploy lands during a running job perhaps one time in five or less at current volume. The cost is a re-upload with an honest "server restarted" message.
Decision (recommended, see the sprint plan): record the full drain as a limitation; do the cheap interim fix now. A real drain needs a SIGTERM handler, a Worker.Drain(ctx) that stops accepting and finishes the current job within a bound, terminationGracePeriodSeconds of about 330 s or more, and strategy: Recreate or a heartbeat-aware sweep (otherwise the surge pod's sweep kills the draining job anyway); Recreate means downtime on every deploy and queued jobs are still lost because the audio is not retained. About 4-6 h plus a cross-repo infra commit: not worth it at current volume.
Interim fix, red tests first
TestMain_SIGTERMCancelsTheContextLikeSIGINT (handle syscall.SIGTERM as well).
TestWorker_ARunningJobIsFailedAsInterruptedWhenTheContextIsCancelled (today a shutdown mid-job is deliberately left for the sweep; with SIGTERM the process may exit before the sweep ever runs on a surviving pod, so record UploadErrInterrupted immediately, bounded by a short context, and keep the sweep as the backstop).
Fix the false comment in scheduler.go.
Add a cross-reference: ADR-033 limitation text and tapir#47.
Acceptance: the tests red first; task check and golangci-lint run ./... exit 0; after deploy, kubectl get pods shows the old pod as Completed, not Error, and a job killed by a deploy shows the interrupted message at once. Review: low-medium; include in the v0.31.1 gatekeeper round.
**Milestone v0.31.1. Reduced from #47 item 3 after the phase-1 investigation; verified directly in the code.**
**Problem (worse than #47 said).** `cmd/tapir/main.go:44` uses `signal.NotifyContext(context.Background(), os.Interrupt)`: SIGINT only. Kubernetes sends **SIGTERM**, which is not handled, so on every deploy the process is killed (exit 143, which `kubectl get pods` shows as `Error` on the old pod) and none of the shutdown code runs: not `Process`'s cancellation handling, not the HTTP server's `Shutdown`, not the worker's `stop()`. A comment at `cmd/tapir/scheduler.go:131` claims "pod SIGTERM exits the loop cleanly": it does not. The running job is failed only later, by the NEXT pod's start-up sweep (`FailInterruptedUploads`), and during a rolling update that sweep fails the OLD pod's job within seconds (the surge pod starts first). Under SIGINT, `ListenAndServe` returns `ErrServerClosed` immediately, so `cmdServe` returns before `Shutdown` drains.
**Numbers (investigation).** A live 64-minute job took 2m53s (a 90-minute file about 4.5 min). Tapir CD redeploys on every push to main (4 ReplicaSets in 4.4 hours today). A deploy lands during a running job perhaps one time in five or less at current volume. The cost is a re-upload with an honest "server restarted" message.
**Decision (recommended, see the sprint plan): record the full drain as a limitation; do the cheap interim fix now.** A real drain needs a SIGTERM handler, a `Worker.Drain(ctx)` that stops accepting and finishes the current job within a bound, `terminationGracePeriodSeconds` of about 330 s or more, and `strategy: Recreate` or a heartbeat-aware sweep (otherwise the surge pod's sweep kills the draining job anyway); Recreate means downtime on every deploy and queued jobs are still lost because the audio is not retained. About 4-6 h plus a cross-repo infra commit: not worth it at current volume.
**Interim fix, red tests first**
- `TestMain_SIGTERMCancelsTheContextLikeSIGINT` (handle `syscall.SIGTERM` as well).
- `TestWorker_ARunningJobIsFailedAsInterruptedWhenTheContextIsCancelled` (today a shutdown mid-job is deliberately left for the sweep; with SIGTERM the process may exit before the sweep ever runs on a surviving pod, so record `UploadErrInterrupted` immediately, bounded by a short context, and keep the sweep as the backstop).
- Fix the false comment in `scheduler.go`.
- Add a cross-reference: ADR-033 limitation text and tapir#47.
**Acceptance:** the tests red first; `task check` and `golangci-lint run ./...` exit 0; after deploy, `kubectl get pods` shows the old pod as `Completed`, not `Error`, and a job killed by a deploy shows the `interrupted` message at once. **Review:** low-medium; include in the v0.31.1 gatekeeper round.
---
_Created via git-mcp on behalf of @mathias_
mathias
added this to the v0.31.1 - security and robustness milestone 2026-10-03 18:48:14 +00:00
Shipped in v0.31.1 (e037c6b, image live): SIGTERM handled; a shutdown records the running job interrupted at once on its own 3 s context; serveHTTP waits (10 s each) for in-flight requests and the worker; review found and fixed that FailUpload could overwrite a done row on a lost commit ack (now only fails queued/transcribing). Left open on purpose: the live acceptance is not observed yet. The rollout to e037c6b replaced a pod that ran the OLD image (it shows Error, exit 143, as before). The next rollout, which runs the new code, must leave the old pod Completed; I will check it at the v0.32.0 deploy and close then. Full drain remains a recorded limitation (ADR-033). Follow-ups: #57.
Shipped in v0.31.1 (e037c6b, image live): SIGTERM handled; a shutdown records the running job `interrupted` at once on its own 3 s context; `serveHTTP` waits (10 s each) for in-flight requests and the worker; review found and fixed that `FailUpload` could overwrite a `done` row on a lost commit ack (now only fails queued/transcribing). **Left open on purpose:** the live acceptance is not observed yet. The rollout to e037c6b replaced a pod that ran the OLD image (it shows `Error`, exit 143, as before). The next rollout, which runs the new code, must leave the old pod `Completed`; I will check it at the v0.32.0 deploy and close then. Full drain remains a recorded limitation (ADR-033). Follow-ups: #57.
---
_Created via git-mcp on behalf of @mathias_
Verified live, closing. At the rollout from image 85f4cc3 to 7b7da65 on 2026-10-04 (both run the SIGTERM handling from v0.31.1), a watch on the namespace recorded the old pod tapir-597dbfccc-ng78q as 1/1 Running → 1/1 Terminating → 0/1 Completed, with 0 restarts. Before the fix the old pod ended Error. The new pod tapir-579b48444f-tssld came up 1/1, 0 restarts, and GET /welcome returns 200.
What this does and does not show: the process exits cleanly on SIGTERM (exit 0) instead of dying. No job was in flight at either rollout, so a job being drained is still unobserved; ADR-033 records that a deploy still loses a job in flight (recorded limitation, interim fix only, per D2).
Source for the fix: v0.31.1 (PR #55, cmd/tapir/shutdown.go).
**Verified live, closing.** At the rollout from image 85f4cc3 to 7b7da65 on 2026-10-04 (both run the SIGTERM handling from v0.31.1), a watch on the namespace recorded the old pod tapir-597dbfccc-ng78q as `1/1 Running` → `1/1 Terminating` → `0/1 Completed`, with 0 restarts. Before the fix the old pod ended `Error`. The new pod tapir-579b48444f-tssld came up 1/1, 0 restarts, and `GET /welcome` returns 200.
What this does and does not show: the process exits cleanly on SIGTERM (exit 0) instead of dying. No job was in flight at either rollout, so a job being drained is still unobserved; ADR-033 records that a deploy still loses a job in flight (recorded limitation, interim fix only, per D2).
Source for the fix: v0.31.1 (PR #55, `cmd/tapir/shutdown.go`).
---
_Created via git-mcp on behalf of @mathias_
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Milestone v0.31.1. Reduced from #47 item 3 after the phase-1 investigation; verified directly in the code.
Problem (worse than #47 said).
cmd/tapir/main.go:44usessignal.NotifyContext(context.Background(), os.Interrupt): SIGINT only. Kubernetes sends SIGTERM, which is not handled, so on every deploy the process is killed (exit 143, whichkubectl get podsshows asErroron the old pod) and none of the shutdown code runs: notProcess's cancellation handling, not the HTTP server'sShutdown, not the worker'sstop(). A comment atcmd/tapir/scheduler.go:131claims "pod SIGTERM exits the loop cleanly": it does not. The running job is failed only later, by the NEXT pod's start-up sweep (FailInterruptedUploads), and during a rolling update that sweep fails the OLD pod's job within seconds (the surge pod starts first). Under SIGINT,ListenAndServereturnsErrServerClosedimmediately, socmdServereturns beforeShutdowndrains.Numbers (investigation). A live 64-minute job took 2m53s (a 90-minute file about 4.5 min). Tapir CD redeploys on every push to main (4 ReplicaSets in 4.4 hours today). A deploy lands during a running job perhaps one time in five or less at current volume. The cost is a re-upload with an honest "server restarted" message.
Decision (recommended, see the sprint plan): record the full drain as a limitation; do the cheap interim fix now. A real drain needs a SIGTERM handler, a
Worker.Drain(ctx)that stops accepting and finishes the current job within a bound,terminationGracePeriodSecondsof about 330 s or more, andstrategy: Recreateor a heartbeat-aware sweep (otherwise the surge pod's sweep kills the draining job anyway); Recreate means downtime on every deploy and queued jobs are still lost because the audio is not retained. About 4-6 h plus a cross-repo infra commit: not worth it at current volume.Interim fix, red tests first
TestMain_SIGTERMCancelsTheContextLikeSIGINT(handlesyscall.SIGTERMas well).TestWorker_ARunningJobIsFailedAsInterruptedWhenTheContextIsCancelled(today a shutdown mid-job is deliberately left for the sweep; with SIGTERM the process may exit before the sweep ever runs on a surviving pod, so recordUploadErrInterruptedimmediately, bounded by a short context, and keep the sweep as the backstop).scheduler.go.Acceptance: the tests red first;
task checkandgolangci-lint run ./...exit 0; after deploy,kubectl get podsshows the old pod asCompleted, notError, and a job killed by a deploy shows theinterruptedmessage at once. Review: low-medium; include in the v0.31.1 gatekeeper round.Created via git-mcp on behalf of @mathias
Shipped in v0.31.1 (
e037c6b, image live): SIGTERM handled; a shutdown records the running jobinterruptedat once on its own 3 s context;serveHTTPwaits (10 s each) for in-flight requests and the worker; review found and fixed thatFailUploadcould overwrite adonerow on a lost commit ack (now only fails queued/transcribing). Left open on purpose: the live acceptance is not observed yet. The rollout toe037c6breplaced a pod that ran the OLD image (it showsError, exit 143, as before). The next rollout, which runs the new code, must leave the old podCompleted; I will check it at the v0.32.0 deploy and close then. Full drain remains a recorded limitation (ADR-033). Follow-ups: #57.Created via git-mcp on behalf of @mathias
Verified live, closing. At the rollout from image
85f4cc3to7b7da65on 2026-10-04 (both run the SIGTERM handling from v0.31.1), a watch on the namespace recorded the old pod tapir-597dbfccc-ng78q as1/1 Running→1/1 Terminating→0/1 Completed, with 0 restarts. Before the fix the old pod endedError. The new pod tapir-579b48444f-tssld came up 1/1, 0 restarts, andGET /welcomereturns 200.What this does and does not show: the process exits cleanly on SIGTERM (exit 0) instead of dying. No job was in flight at either rollout, so a job being drained is still unobserved; ADR-033 records that a deploy still loses a job in flight (recorded limitation, interim fix only, per D2).
Source for the fix: v0.31.1 (PR #55,
cmd/tapir/shutdown.go).Created via git-mcp on behalf of @mathias