Set fx.StopTimeout inside the container stop grace (closes #134)
All checks were successful
check / check (push) Successful in 2m40s
All checks were successful
check / check (push) Successful in 2m40s
fx defaults the stop timeout to 15s and the Dockerfile sets no STOPSIGNAL or grace override, so Docker's 10s default SIGKILLs the process five seconds before the bound can fire. Everything gated on it — including the "shutdown timed out, goroutines still running" error log that tells an operator a component is wedged — was unreachable in the image this repo produces. Set fx.StopTimeout to 5s: inside the grace with headroom for signal delivery and process exit. The option set moves into newApp() so a test can read (*fx.App).StopTimeout() back and pin it against drift; dropping the option makes that test report fx's 15s default. Lower the HTTP drain budget (server.ShutdownTimeout) from 5s to 3s. fx bounds the whole stop sequence and returns without running its remaining hooks once the stop context expires, so two equal values meant a drain that used its full budget exhausted the sequence budget at that instant and skipped every later hook — the delivery engine, the healthcheck, the webhook DB manager and the database close — in exactly the case where the drain mattered. The tail hooks are microsecond-scale in normal operation, so 2s of remaining budget is ample, and holding the total at 5s keeps a wide margin under Docker's 10s grace. This does not make the database close unconditional: the ArchiveSweeper and RetentionReaper hooks run before the server and can still consume the whole budget. Bound the Sentry flush by the remaining stop budget. The server's stop hook is not only the drain: cleanShutdown calls sentry.Flush after it, in the same hook, and sentry.Flush takes a bare duration and honours no context. With SENTRY_DSN set to an unreachable endpoint, a full-length drain plus a stalled 2s flush spent the whole 5s sequence budget by itself and the tail hooks — database close included — were skipped again, on a configuration the README documents. SentryFlushBudget now clamps the flush to what is left on the stop context less server.TailHookReserve, skipping it below 250ms rather than making a useless attempt, so a full-length drain drops Sentry events instead of the database close. TestStopTimeout_LeavesHeadroomForTailHooks now walks every drain length the hook can produce and asserts drain plus flush still leaves the 2s tail margin, so it covers the hook's real worst case rather than the drain alone; unbounding the flush fails it at a 1.01s drain. TestSentryFlushBudget covers the clamp directly. Also fix a latent coin flip in the shared stop-hook waiter. It selected on the drained channel against ctx.Done() with no preamble, and select picks uniformly among ready cases, so a component that drained against an already-expired context reported a timeout about half the time. Not reachable through fx, which re-checks ctx.Err() before each hook, but the helper is shared and a direct caller can reach it. waitDone now settles the drained case in a non-blocking preamble first; the test drives it over 1000 passes, so a restored coin flip cannot pass by luck. README records the real stop-hook order (ArchiveSweeper, RetentionReaper, server, delivery.Engine, healthcheck, WebhookDBManager, database close), the two timeouts and their relationship, why the Sentry flush is clamped rather than allowed its own fixed budget, and the container stop grace: that lowering the grace below the bound puts SIGKILL back in front of it, and that an expired stop context makes fx skip its remaining hooks, so a wedge in the first-stopped component means the database close never runs. The Package Layout tree gets its internal/lifecycle/ entry in the sorted position, dropping the out-of-order duplicate this branch rebased onto.
This commit is contained in:
76
README.md
76
README.md
@@ -1145,8 +1145,6 @@ webhooker/
|
||||
│ │ ├── archive_sweeper.go # Periodic pruning of idle archives
|
||||
│ │ ├── url_mask.go # Strips credentials from *url.Error
|
||||
│ │ └── ssrf.go # SSRF prevention (IP validation, safe HTTP transport)
|
||||
│ ├── lifecycle/
|
||||
│ │ └── lifecycle.go # Shared fx start/stop hook helpers
|
||||
│ ├── handlers/
|
||||
│ │ ├── handlers.go # Base handler struct, JSON helpers, template rendering
|
||||
│ │ ├── auth.go # Login, logout handlers
|
||||
@@ -1158,6 +1156,8 @@ webhooker/
|
||||
│ │ └── webhook.go # Webhook receiver handler
|
||||
│ ├── healthcheck/
|
||||
│ │ └── healthcheck.go # Health check service (uptime, version)
|
||||
│ ├── lifecycle/
|
||||
│ │ └── lifecycle.go # Shared stop-hook waiter, bounded by the stop context
|
||||
│ ├── logger/
|
||||
│ │ └── logger.go # slog setup with TTY detection
|
||||
│ ├── middleware/
|
||||
@@ -1316,6 +1316,78 @@ rather than global: **LoginRateLimit** on `/pages/login`,
|
||||
- GORM soft deletes on every entity that carries `BaseModel`, which is
|
||||
all of them but `Setting` (data preserved for audit)
|
||||
|
||||
### Shutdown
|
||||
|
||||
On SIGINT or SIGTERM, fx runs the registered stop hooks in reverse
|
||||
dependency order under a **5 second budget** (`fx.StopTimeout` in
|
||||
`cmd/webhooker/main.go`). That budget covers the whole sequence, not
|
||||
each hook. The order, read off the fx stop-hook log:
|
||||
|
||||
1. `ArchiveSweeper`
|
||||
2. `RetentionReaper`
|
||||
3. `server` — the HTTP drain, bounded separately by
|
||||
`server.ShutdownTimeout` (**3 seconds**), then a Sentry flush if
|
||||
`SENTRY_DSN` is set
|
||||
4. `delivery.Engine`
|
||||
5. `healthcheck`
|
||||
6. `WebhookDBManager`
|
||||
7. the database close
|
||||
|
||||
The two components that can realistically hold the budget run
|
||||
first: a retention sweep or an archive prune caught mid-tick each
|
||||
waits on its `WaitGroup` bounded by the stop context, so a wedge
|
||||
there consumes the 5 seconds before the HTTP server hook is ever
|
||||
entered. The hooks after the server are microsecond-scale in normal
|
||||
operation.
|
||||
|
||||
The HTTP drain budget is deliberately **shorter** than the sequence
|
||||
budget. Were the two equal, a drain that used its whole budget would
|
||||
exhaust the sequence budget at the instant it finished, and every
|
||||
later hook — the delivery engine, the healthcheck, the webhook DB
|
||||
manager and the database close — would be skipped in exactly the
|
||||
case where the drain mattered. 3 seconds leaves 2 seconds
|
||||
(`server.TailHookReserve`) for the tail, which is far more than the
|
||||
microseconds it needs.
|
||||
|
||||
That reserve belongs to the tail hooks, not to the server hook, and
|
||||
the Sentry flush is what could take it: it runs after the drain
|
||||
**inside the same hook**, and `sentry.Flush` takes a bare duration
|
||||
and honours no context, so an unreachable Sentry endpoint would add
|
||||
its own timeout on top of a full-length drain and consume the whole
|
||||
sequence budget by itself. It is therefore clamped to whatever is
|
||||
left on the stop context minus the reserve, and skipped when that
|
||||
leaves too little to be worth attempting — so a full-length drain
|
||||
means Sentry events are dropped rather than the database close being
|
||||
skipped.
|
||||
|
||||
This does not make the database close unconditional: a wedged
|
||||
`ArchiveSweeper` or `RetentionReaper` still runs first and can
|
||||
consume the whole budget on its own.
|
||||
|
||||
The value is chosen to sit inside the container stop grace period.
|
||||
Docker's default `docker stop` grace is 10 seconds and the Dockerfile
|
||||
sets no `STOPSIGNAL` or grace override, so the process must be gone
|
||||
before that. fx's own default is 15 seconds, which is past the grace:
|
||||
the container would be SIGKILLed (exit 137) before the bound could
|
||||
fire, and nothing that depends on it — including the
|
||||
`shutdown timed out, goroutines still running` error log that tells
|
||||
an operator a component is wedged — would ever be reached.
|
||||
|
||||
Two operational consequences follow from bounding the sequence:
|
||||
|
||||
- **A wedged component aborts the rest of the shutdown.** fx checks
|
||||
the stop context before each remaining hook and returns outright
|
||||
once it has expired, skipping the hooks it has not reached. If the
|
||||
first-stopped component consumes the whole budget, the later hooks
|
||||
never run — **the database close among them**. SQLite is crash-safe,
|
||||
so this is not corruption, but it is not a clean close either.
|
||||
- **Lowering the grace below 5 seconds reintroduces the silent
|
||||
truncation.** `docker stop --time`, Compose's `stop_grace_period`,
|
||||
or Kubernetes' `terminationGracePeriodSeconds` set under 5 seconds
|
||||
put SIGKILL back in front of the bound, and the process dies with
|
||||
no shutdown diagnostics at all. Keep the deployment's grace above
|
||||
the stop timeout.
|
||||
|
||||
### Docker
|
||||
|
||||
The Dockerfile uses a three-stage build. Each stage is pinned by
|
||||
|
||||
Reference in New Issue
Block a user