amazon: reconnect the LISTEN connection on failure instead of dying
A dropped connection, a Postgres restart, or any other error on the dedicated LISTEN connection previously propagated all the way out of ProcessEvents, and main.go's top-level shutdown logic treats that error channel firing the same as a fatal server error - taking down the entire application over a hiccup on one background connection that has nothing to do with serving HTTP traffic. This matters more with eleven other platforms already sharing the same trigger+notify shape in the migrations with no Go processor yet. Adds (*Mocks).reconnectOrStop: on a real failure (not an ordinary shutdown), logs a warning and re-establishes LISTEN after a backoff that starts at 1s, caps at 30s, doubles on repeated immediate failures, and resets once a reconnect actually succeeds. ProcessEvents' select no longer returns on a LISTEN error - it loops back in with fresh channels instead. Verified against a genuinely killed connection (pg_terminate_backend, targeting the backend via pg_stat_activity matched on its LISTEN query text), not a simulated one - both in a manual check and in the new TestProcessEvents_ReconnectsAfterListenConnectionDrops test. Closes out all four insights from the domains/amazon design review. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XEDaCB7C2NEBgyvqEtZuxY
This commit is contained in:
@@ -0,0 +1,26 @@
|
||||
# Work Summary — 2026-08-05 21:55
|
||||
|
||||
## Task
|
||||
Insight #4 from the `domains/amazon` design review, and the one the user flagged as most consequential: a LISTEN-connection error (dropped connection, Postgres restart, network blip) currently propagates all the way out of `ProcessEvents`, and `main.go`'s top-level shutdown logic treats that the same as a fatal server error - taking down the *entire application*, not just the Amazon event processor. Significant because the same trigger+notify shape is already migrated (SQL-side) for eleven other platforms with no Go processor yet.
|
||||
|
||||
## Design confirmed with user before implementing
|
||||
Of three options presented (reconnect/retry around LISTEN; decouple event-processing failure from app shutdown in `main.go`; both), the user chose reconnect/retry only, with a warning logged - not touching `main.go`'s shutdown behavior at all.
|
||||
|
||||
## Changes
|
||||
`domains/amazon/mock.go`:
|
||||
- New constants `initialListenReconnectBackoff` (1s) and `maxListenReconnectBackoff` (30s).
|
||||
- New `(*Mocks).reconnectOrStop`: given the error that came off `errCh`, distinguishes an ordinary shutdown (nil error, or `ctx` already done - returns `stop=true`) from a real failure. For a real failure: logs `Warn("lost connection while listening for notifications; reconnecting", "error", ..., "retry_in", backoff)`, waits out the backoff (still respecting `ctx` cancellation), and calls `listenForNotifications` again. Backoff resets to its initial value on a successful reconnect and doubles (capped) on an immediate repeat failure (e.g. the pool itself being unreachable), so a persistently-down DB backs off rather than hot-looping.
|
||||
- `ProcessEvents`'s `select` no longer returns on `errCh`/closed-`notifCh` directly - both paths now go through `reconnectOrStop`, looping back into the main loop with fresh channels instead of exiting.
|
||||
|
||||
## Verification
|
||||
- Before writing the real test, manually confirmed the mechanism end-to-end with a throwaway program (scratchpad, not committed): started `ProcessEvents`, looked up its LISTEN connection's backend PID via `pg_stat_activity` (matching on `query = 'LISTEN mock_shop_amazon_event_inserted'`, which Postgres keeps showing while a connection sits idle), and killed it with `pg_terminate_backend`. Logs showed the exact expected sequence: the real connection error, the warning with `retry_in=1s`, then processing resuming on schedule.
|
||||
- New test `TestProcessEvents_ReconnectsAfterListenConnectionDrops`, using that same real-kill technique (via a new `terminateListenConnection` test helper) rather than a simulated failure: confirms `ProcessEvents` does *not* return after the connection is killed, and that the reactive path (insert → dispatch) still works afterward, proving the reconnect actually restored a working LISTEN.
|
||||
- `go build ./...` / `go vet ./...` clean.
|
||||
- `go test ./domains/amazon/... -v -race`: all 7 tests pass (6 existing + 1 new).
|
||||
- 10x repeated runs (`-count=1 -race`) with no flakes, ~3.5-3.7s each (the new test alone takes ~2s, waiting out the real 1s backoff plus recovery time).
|
||||
- `make test`: full suite green.
|
||||
- Deliberately **skipped** `make test-against-dev-db` this round: a live `go run .` process was found running against the dev DB at verification time, and this test kills a LISTEN connection by matching on query text - safe against the dedicated test DB, but running it against dev risked hitting that live process's own connection instead of (or alongside) the test's. It would have recovered gracefully (that's the entire point of this fix), but there was no need to disrupt a possibly-in-use process just to re-prove what the test DB run already confirmed. Flagged to the user rather than done silently.
|
||||
|
||||
## Follow-ups / not done here
|
||||
- This closes all four insights from the original `domains/amazon` design review (2026-08-04). `domains/reports` test coverage remains open, deferred by the user to later.
|
||||
- Worth being aware of for next time: `terminateListenConnection`'s PID lookup matches purely on query text, with no way to scope it to "this specific test's connection" if multiple `Mocks` instances are ever LISTEN-ing concurrently against the same database (e.g. a real dev server running at the same time as `test-against-dev-db`). Not fixed - just something to check for before running this specific test against a shared/live database.
|
||||
Reference in New Issue
Block a user