Remote JobCancel() can be silently lost while the notifier is reconnecting (no durable-poll fallback)

[chrisgeo]

Summary

Client.JobCancel() delivers its cancellation signal to a running job's executor exclusively via Postgres LISTEN/NOTIFY. If the notifier happens to be mid-reconnect (its exponential backoff loop, internal/notifier/notifier.go) at the moment the NOTIFY is sent, that specific signal is lost — not delayed — because Postgres NOTIFY is fire-and-forget and only reaches sessions that are actively LISTENing at commit time. The job keeps running unaware it was cancelled, and the only recovery is JobRescuer's stuck-job sweep, which defaults RescueStuckJobsAfter to one hour when unconfigured.

This is a real, reproducible failure mode, not a theoretical one — details and evidence below.

Mechanism

1. notifier.go's run loop treats any listener error (a transient connection reset, a pooler cycling the backend, a brief network blip, etc.) as cause to disconnect and retry with exponential backoff: 0.5s, 1s, 2s, 4s, 8s, 16s, 32s, 64s (rivershared/util/serviceutil.ExponentialBackoff, MaxAttemptsBeforeResetDefault = 7; the run loop's own attempt counter starts at 0, one step earlier than ExponentialBackoff's doc comment assumes, so the very first retry lands at 0.5s before the schedule settles into the documented 1s→64s cycle on later retries). Each connect/listen attempt also has its own 10s timeout (listenerTimeout).
2. While the notifier is in that backoff window, it is not LISTENing on river_control (or any topic) at all.
3. If JobCancel() is called during that window, its NOTIFY commits successfully but has no active listener to reach — it is gone. JobCancel() itself has no visibility into notifier connectivity and returns success regardless (correctly — the durable state write, cancel_attempted_at in the job's metadata, did succeed).
4. The running job's executor never observes ctx.Done(). It keeps running until it completes normally, or until JobRescuer eventually rescues it — up to an hour later by default, since RescueStuckJobsAfter isn't tied to cancel_attempted_at specifically; it's the same general stuck-job window.

Confirmed by reading the source directly (not inferring from behavior) against v0.44.0 — this is not a stale/fixed-since concern; I checked master too (currently a few commits past v0.44.1, none touching notifier/producer/rescuer).

Evidence

Found while investigating an intermittent CI failure in a compatibility test harness that exercises River under session-mode PgBouncer pooling (session-mode pins one backend connection per client for its whole life, which is the documented-correct pooling mode for River's LISTEN-dependent components per the River PgBouncer docs). The harness's own JobCancel()-driven cancellation test would intermittently hang until its bounded wait (20s) expired, under CPU-constrained CI runners. That investigation turned up two separate, independently-confirmed issues on our side, only one of which is River's: (1) our own connection-pool sizing was too tight under load — our bug, already fixed; (2) the mechanism described in this issue, confirmed present in River's source and unaffected by our pool fix.

For the mechanism in this issue specifically: I'm reporting it because I read and understand the code path, not because I caught the notifier's reconnect/backoff loop firing at the exact instant of a specific test hang (that's a narrow window to catch live, and I don't want to overclaim a live capture I don't have). What I can say with confidence: NOTIFY semantics in Postgres are fire-and-forget by design (not River-specific), the notifier's backoff loop is real and unchanged as of the current master, and the two combine straightforwardly into exactly the failure mode described above with no additional assumptions needed.

Suggested direction

#1135 raised a related situation earlier this year, and the response there (also echoing the original design discussion in #630 back in 2024) was that a durable-poll fallback for all running jobs on all clients is rightly seen as prohibitive query load for what's framed as a rare event. I think that's the right call for that shape of fallback — but there's a narrower version worth considering:

A much cheaper, targeted version of that idea: poll only the (typically tiny) set of jobs that actually have metadata->>'cancel_attempted_at' set — i.e., jobs someone has already explicitly tried to cancel — rather than every running job. That set should be empty almost all the time in a healthy system, making the query cost negligible in the common case while providing deterministic convergence in the rare case a NOTIFY is genuinely lost. This could live client-side (each client polling its own currently-running job IDs against that condition) or as an addition to the existing rescuer's responsibilities with a much shorter, cancel-specific window than the general stuck-job RescueStuckJobsAfter.

Happy to discuss further or take a stab at a PR if there's interest in this direction.

[brandur]

Hey Chris, yep sounds like a real problem.

Happy to discuss further or take a stab at a PR if there's interest in this direction.

Feel free to take a stab a PR.

A much cheaper, targeted version of that idea: poll only the (typically tiny) set of jobs that actually have metadata->>'cancel_attempted_at' set — i.e., jobs someone has already explicitly tried to cancel — rather than every running job. That set should be empty almost all the time in a healthy system

Sounds like it could probably work, although keep in mind you'd still have to use the (state, finalized_at, id) index and then filter down on that, so we'd want to make sure it doesn't cause an undue number of slower queries (just get your LLM to do some benchmarking).

One other alternative approach would be to program each client so that after its listener reconnects (after losing a connection), it polls all its active jobs and checks for cancel_attempted_at, cancelling them as necessary. That'd have the benefit of cancel polls being quite rare, and you could poll jobs by primary key.