feat: add PostgreSQL cross-process wake-up - #14
Conversation
In-process signalling cannot reach another process, so a commit in a web process left a worker waiting out polling_interval and every reactive update paid up to 100ms before delivery began. The optional adapter listens on a dedicated connection and notifies through the pool, taking measured cross-process wake-up latency from 103.7ms to 2.9ms at p50 against PostgreSQL 17. Polling remains the upper bound, so a missed or failed notification costs latency rather than correctness, and neither signalling nor waiting raises into its caller.
Greptile SummaryThe PR adds an opt-in PostgreSQL LISTEN/NOTIFY wake-up adapter while retaining polling as a correctness fallback.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains. Important Files Changed
Sequence DiagramsequenceDiagram
participant Web as Committing process
participant PG as PostgreSQL
participant Role as Runtime role
participant Sup as Supervisor
Role->>PG: LISTEN on dedicated thread connection
Web->>PG: NOTIFY after commit
PG-->>Role: Wake wait_for_notify
Role->>Role: Poll durable work
Sup->>Role: Request shutdown
Sup->>PG: Disconnect tracked listeners (ensure)
Reviews (5): Last reviewed commit: "fix: release wake-up connections when sh..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
|
||
| # @rbs (timeout: Numeric) -> bool | ||
| def wait(timeout:) | ||
| connection = listening_connection |
There was a problem hiding this comment.
When the supervisor runs multiple runtime roles, the process-wide adapter returns the same cached PostgreSQL connection to every waiter and calls wait_for_notify outside the mutex. A notification is consumed through that one connection instead of independently waking every role, while concurrent connection use can also fail and leave roles asleep until their polling timeout.
Prompt To Fix With AI
This is a comment left during a code review.
Path: lib/solid_objects/wake_up_adapters/postgresql.rb
Line: 40
Comment:
**Shared connection breaks wake-up broadcast**
When the supervisor runs multiple runtime roles, the process-wide adapter returns the same cached PostgreSQL connection to every waiter and calls `wait_for_notify` outside the mutex. A notification is consumed through that one connection instead of independently waking every role, while concurrent connection use can also fail and leave roles asleep until their polling timeout.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.There was a problem hiding this comment.
Correct on both halves, and the test half is the part I got wrong.
The adapter is memoized process-wide and shared across runtime roles, so every waiter used one cached connection and called wait_for_notify on it concurrently. Whichever waiter won consumed the notification and the other roles slept until their polling timeout, which defeats the purpose of the adapter, and concurrent use of a single connection is unsafe regardless.
Fixed in 9a6ca78: a listening connection is opened and tracked per thread, and stop disconnects all of them.
On the test: you were right that it only demonstrated multiple waiters with separate adapters. I rewrote it to share one adapter, and it still passed against the broken code, because it asserted only that each waiter woke, not when. Two of the three were waking at the 5 second timeout, which the assertion happily accepted. It now asserts every waiter wakes in under a second against a 5 second timeout. Against the shared connection that test hangs; with the per-thread connection all seven pass.
Added a second test asserting two threads hold distinct listening connections.
The supervisor memoizes one adapter and shares it across runtime roles, so every waiter used one cached connection and called wait_for_notify on it concurrently. A notification was consumed by whichever waiter won, leaving the other roles asleep until their polling timeout, and concurrent use of one connection is unsafe besides. Open and track a listening connection per thread, and disconnect all of them on stop. The earlier test hid this by giving each waiter its own adapter, and passed even against the shared connection because it could not distinguish waking on a notification from returning at the timeout; it now asserts on latency.
|
@greptileai review |
| connection = Record.connection_pool.send(:new_connection) | ||
| connection.execute("LISTEN #{connection.quote_table_name(channel)}") | ||
| Thread.current[thread_key] = connection | ||
| mutex.synchronize { connections << connection } |
There was a problem hiding this comment.
When a configured PostgreSQL adapter has opened per-thread listeners and the supervisor is restarted in the same process, Supervisor#stop never invokes the adapter's stop, so those dedicated connections remain open and accumulate across restarts, eventually consuming PostgreSQL connection capacity.
Prompt To Fix With AI
This is a comment left during a code review.
Path: lib/solid_objects/wake_up_adapters/postgresql.rb
Line: 97-100
Comment:
**Listener connections survive shutdown**
When a configured PostgreSQL adapter has opened per-thread listeners and the supervisor is restarted in the same process, `Supervisor#stop` never invokes the adapter's `stop`, so those dedicated connections remain open and accumulate across restarts, eventually consuming PostgreSQL connection capacity.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.There was a problem hiding this comment.
Valid. The adapter holds connections outside the pool and Supervisor#stop never released them, so restarting the supervisor in one process accumulated listeners until PostgreSQL ran out of capacity.
Fixed in f61792e: shutdown stops the wake-up adapter when it supports stop, guarded so the default in-process WakeUp, which has no stop, is unaffected, and rescued so a failure there cannot break shutdown.
Regression test: configure the adapter, open a listener, start and stop a supervisor, and assert no connections remain tracked.
A wake-up adapter may hold connections outside the pool, and Supervisor#stop never released them, so restarting the supervisor in one process accumulated PostgreSQL listener connections until the server ran out of capacity. Stop the adapter during shutdown when it supports it.
|
@greptileai review |
| components.reject(&:stopped?).each(&:stop) | ||
| release_wake_up |
There was a problem hiding this comment.
When a component shutdown operation raises, Supervisor#stop exits before release_wake_up, leaving dedicated PostgreSQL listener connections open and allowing them to accumulate across supervisor restarts.
Prompt To Fix With AI
This is a comment left during a code review.
Path: lib/solid_objects/supervisor.rb
Line: 49-50
Comment:
**Shutdown failures skip listener cleanup**
When a component shutdown operation raises, `Supervisor#stop` exits before `release_wake_up`, leaving dedicated PostgreSQL listener connections open and allowing them to accumulate across supervisor restarts.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.There was a problem hiding this comment.
Valid. Cleanup ran after component shutdown, so a component raising in stop skipped it and left listener connections open, which is the same leak as before through a different path.
Fixed: the release now runs in an ensure, alongside clearing the started flag and emitting supervisor.stopped, so a failed shutdown still frees connections held outside the pool and does not leave the supervisor believing it is running.
Regression test: a supervisor whose components raise in stop still releases every listener connection.
WakeUpAdapters.for returns PostgreSQL notifications where the database provides them and the in-process default elsewhere, so applications can opt in with one line that is safe across adapters. It is deliberately not the default: the adapter opens a connection per waiting thread outside the pool, and LISTEN does not survive a transaction-pooling proxy such as PgBouncer, so adopting it is a deployment decision rather than an upgrade side effect. MySQL has no notification primitive and keeps polling.
|
@greptileai review |
Cleanup ran after component shutdown, so a component raising in stop skipped it and left listener connections open. Move the release into an ensure so a failed shutdown cannot leak connections held outside the pool.
|
@greptileai review |
1 participant
Roadmap milestone 2, and the largest remaining term in reactive update latency.
The problem
WakeUpis a plain in-processThread::ConditionVariable. It cannot cross process boundaries, soa commit in a Puma process does not wake a broadcast executor in a worker process, and delivery
waits out
polling_interval, 100 ms by default. Nothing shipped in 0.6.0 or 0.7.x touched this:batching took requests from three to one, state payloads took them to zero, and both still paid
this delay first.
The fix
An optional adapter, opt-in through the seam that already existed:
Measured on PostgreSQL 17, 30 samples, signal sent 2 ms after the waiter began:
WakeUpWakeUpAdapters::PostgresqlThis removes the polling floor rather than shrinking it, which is what the roadmap asked for
instead of just lowering
polling_interval.Design choices
Polling stays the upper bound.
waitblocks onwait_for_notify(timeout), so a missed orfailed notification costs latency, never correctness. The adapter is an optimisation layered over
the existing durable polling, not a replacement for it.
Nothing raises into the caller.
signalruns after a commit, so an error there must notsurface to application code; both
signalandwaitrescue, instrumentwake_up.failed, andreturn false. A failed
waitalso paces itself rather than spinning.Listening uses a dedicated connection.
LISTENis per connection, and a blocking wait must nothold a connection the rest of the runtime needs. The adapter opens one outside the pool and
releases it on
stop.listenis separate fromwaitso a caller can start listening before it blocks, closing thewindow where a notification sent during startup would be missed.
Tests
Six integration tests, skipped on other adapters and run for real against PostgreSQL locally and in
CI: a signal from another connection wakes a waiter well before the timeout, waiting still returns
on timeout when nothing signals, every waiter wakes on one signal, signalling never raises,
waiting never raises and still paces itself, and the adapter satisfies the wake-up contract.
Compatibility
Fully opt-in. No database change, no migration, no initializer regeneration. Applications on
SQLite or MySQL, or that do not configure the adapter, keep the existing polling behaviour
unchanged. No version bump in this PR; say the word and I will add one.
Validation
Standard Ruby, RuboCop, RBS, Steep, and Brakeman clean. I started a local PostgreSQL 17 for this
rather than relying on CI, so the adapter and its numbers are verified against a real server.
Not included
Redis. Milestone 2 lists it as optional, and adding a Redis dependency to a gem whose premise is
"no Redis" deserves its own discussion. The adapter contract is
signalpluswait(timeout:), soone can be added without touching this code.