Repository navigation
bug(infra): graceful shutdown returns before drain completes — duplicate inserts + dropped in-flight requests on every restart under traffic #383
Description
Summary
On SIGTERM/SIGINT, run() (cmd/wavehouse/main.go) does not wait for the shutdown/drain to finish, so the process tears down while requests and the ingest worker are still mid-flight.
Detail
srv.ListenAndServe() (main.go:472) returns http.ErrServerClosed the moment srv.Shutdown is called from the signal goroutine (main.go:449) — per the net/http docs, "Serve/ListenAndServe immediately return ErrServerClosed when Shutdown is called; make sure the program doesn't exit and waits instead for Shutdown to return." run() has no done-channel, so it returns 0 immediately and main calls os.Exit(run()) (main.go:47).
Consequences:
- The deferred
embeddedMQ.Close()(main.go:242) shuts NATS down underneath (a) HTTP handlers still being drained bysrv.Shutdownin the signal goroutine, and (b) the ingest worker mid-flush — itsDoubleAcks fail, so rows already written to ClickHouse are redelivered and re-inserted on next boot. os.Exitkills the signal goroutine beforeingestCleanup(shutCtx)(main.go:457) finishes, so the drain path indispatchLoop/drainAndExit(internal/ingest/worker.go) is effectively dead code in production.
Impact
Every SIGTERM deploy with in-flight batches can produce duplicate rows and/or 5xx'd in-flight requests.
Fix direction
Have the signal goroutine own the lifecycle and signal a done-channel that run() blocks on before returning; run the deferred Close()s only after Shutdown + ingestCleanup return.
Found in a repo-wide audit; verified by code trace. Note: #46 (Graceful Shutdown & Resource Cleanup) is marked completed but this ordering defect remains.
Activity
coderabbitai commentedon Jul 8, 2026
🔗 Related PRs
#122 - fix(ingest): break infinite-Nak loop on permanent delete errors [merged]
#125 - fix(boot): non-fatal schema discovery, /health 503 with diagnostic [merged]
#164 - refactor(ingest)!: insert-only pipeline; mutations via /v1/query [closed]
#182 - refactor: full api --> ingest --> clickhouse --> dlq refactor [closed]
#366 - fix(observability): bound OTel provider shutdown to keep exit fast [merged]
🧪 Issue enrichment is currently in open beta.
You can configure auto-planning by selecting labels in the issue_enrichment configuration.
To disable automatic issue enrichment, add the following to your
.coderabbit.yaml:💬 Have feedback or questions? Drop into our discord!
EricAndrechek commentedon Jul 28, 2026
Noted finding that could be helpful when closing this issue:
insert_deduplication_tokenusing the NATS event ID or something could help us have clickhouse prevent the duplication without relying only on replacingmergetree or the likes that may be schema dependent.EricAndrechek commentedon Oct 8, 2026
Fixed by #585.
run()incmd/wavehouse/main.gonow blocks inApp.Run(internal/app/app.go). That call returns only after the HTTP server'sShutdownhas returned and the ingest worker has drained withinserver.shutdown_timeout. The stores, embedded NATS included, are released byApp.Closeonly after that. The remaining related cases are tracked separately: #665 (a durable's final ack landing after the broker closes), #698 (a slow shard-claim tick outrunning the shutdown budget), #588 (exit status after a failed drain) and #660 (aninsert_deduplication_tokenper INSERT).