Skip to content

feat(app): distinguish a clean stop from one whose drain or release failed #588

Open

Description

@EricAndrechek

A stop that fails halfway exits 0, exactly like a clean one. There is no signal an orchestrator, a systemd unit, or a CI script can branch on.

Where it comes from

internal/app/app.go:165-173 — every component loop runs under one errgroup, and the wrapper discards the loop's error once the caller has asked for a stop:

err := c.run(gctx)
// A stop the caller asked for is clean whatever the loop
// reported on its way out (a consumer creation cut short, say).
if err == nil || ctx.Err() != nil {
    return nil
}

That behaviour is deliberate and correct as far as it goes: on SIGTERM, a loop returning "context canceled" is not a failure, and #585 added the comment explaining why. The gap is that it does not distinguish "returned an error because we asked it to stop" from "returned an error because the drain genuinely failed" — both become nil, Run returns nil, and the process exits 0.

The release phase has the same hole one level up: cmd/wavehouse/main.go's deferred a.Close(closeCtx) logs a failure at Warn and nothing more, so a store that failed to close cleanly is invisible outside the log stream.

Why it matters

Today the observable outcomes are identical for:

  • a clean stop where everything drained and every store closed;
  • a stop where the ingest worker hit its deadline with batches still in flight (waitOrDeadline returns ctx.Err() and abandons the goroutines — safe, because un-acked events are redelivered, but it means the next boot re-does work);
  • a stop where the HTTP drain overran and connections were force-closed;
  • a stop where Pebble or the embedded NATS store failed to close.

An operator doing a rolling restart cannot tell the difference without reading logs, and automation cannot tell at all. systemd's RestartForceExitStatus, a deploy script gating the next pod on the previous one, and CI teardown checks all want this as an exit code.

Proposed shape

  1. Collect the component errors rather than discarding them when ctx.Err() != nil, and have Run return them wrapped in a sentinel — ErrUncleanStop — so a caller can tell the two cases apart with errors.Is.
  2. Fold Close's joined error into the same signal instead of only logging it.
  3. main maps three outcomes:

Not in #585

Exit codes are a public interface. Adding one needs a deliberate choice of value, a deployment.md entry documenting it, and a test — and #585's scope is extracting the process wiring out of main.go, which is already large and green. Splitting this out keeps that PR's blast radius where it is.

Notes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area/appProcess wiring (internal/app): component build, run, release

    Type

    No type

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions