Add distributed notifications via PostgreSQL LISTEN/NOTIFY#27
Add distributed notifications via PostgreSQL LISTEN/NOTIFY#27
Conversation
Route all change notifications through a ChangeNotifier interface to enable horizontal scaling. For PostgreSQL, notifications are broadcast via pg_notify and received via a dedicated LISTEN connection (using DATABASE_DIRECT_URL to bypass connection poolers like Supavisor). For SQLite, a LocalNotifier dispatches directly to the in-memory eventsManager preserving current behavior. https://claude.ai/code/session_01ETtJh2CpT5zn9Vab3yfRZ7
Replace TEST_PG_DATABASE_URL env var requirement with testcontainers-go. Tests now spin up a disposable PostgreSQL container automatically via Docker, making them fully self-contained and CI-friendly. - Add testutil/pgcontainer.go shared helper using postgres:15-alpine - Update store/postgres/pg_test.go with TestMain container lifecycle - Update notifier/pg_test.go with TestMain container lifecycle - Add testcontainers-go v0.31.0 dependency https://claude.ai/code/session_01ETtJh2CpT5zn9Vab3yfRZ7
| log.Printf("pgNotifier: listen connection lost: %v, reconnecting in %v", err, delay) | ||
| select { | ||
| case <-time.After(delay): | ||
| delay = min(delay*2, maxReconnectDelay) |
There was a problem hiding this comment.
Is there any potential that a client could miss an event during the reconnect? I'm wondering if we could trigger a change to the handler so the client pulls
There was a problem hiding this comment.
You mean send notification to all clients on reconnect?
There was a problem hiding this comment.
Unless there is a different way to handle it. I'm more interested if there's potential to miss a notify and the client doesn't pull
There was a problem hiding this comment.
yes there is a potential to miss the notify if the connection is down. My concern in sending all clients a notification is that it will create a spike on the service at that time.
If we think this is important I can explore more alternatives. One of them I think to maintain a record per user with the last change, update it on every change and use when calling notify it will also clear that table. On re-connection we can go over that table and notify missing notifications.
There was a problem hiding this comment.
Perhaps we can use the revision increment to detect changes. I will check.
syncer_server.go
Outdated
| storage = pgStorage | ||
|
|
||
| if config.PgDirectUrl != "" { | ||
| changeNotifier = notifier.NewPGNotifier(pgStorage, config.PgDirectUrl, em.notifyChange) |
There was a problem hiding this comment.
Is it worth validating/testing the PgDirectUrl and falling back to local notifier? A failure would mean the server would run without a working notifier
There was a problem hiding this comment.
Perhaps panicking is better? Otherwise we won't know why clients don't get notifications.
There was a problem hiding this comment.
Yeah, if we have good oversight on panicking instances
3 participants
Summary
This PR introduces a distributed notification system that enables multi-instance deployments to broadcast data sync changes across all instances using PostgreSQL's LISTEN/NOTIFY mechanism, while maintaining backward compatibility with single-instance SQLite deployments.
Key Changes
New notifier package with abstraction layer for change notifications:
ChangeNotifierinterface supporting both local and distributed notificationsLocalNotifierfor single-instance deployments (SQLite or local-only mode)PGNotifierfor multi-instance deployments using PostgreSQL LISTEN/NOTIFYPostgreSQL LISTEN/NOTIFY implementation:
pg_notify()Configuration updates:
PgDirectUrlconfig option for direct PostgreSQL connectionsDATABASE_URLandDATABASE_DIRECT_URLare configuredIntegration with SyncerServer:
SetRecordto usenotifier.Notify()instead of directeventsManager.notifyChange()Implementation Details
pfor pubkey,cfor clientID)https://claude.ai/code/session_01ETtJh2CpT5zn9Vab3yfRZ7