janpeter/Scrum4Me
0
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix(realtime): één gedeelde LISTEN-connectie per kanaal i.p.v. één per SSE-stream #172

Merged
janpeter merged 1 commit from fix/realtime-listen-multiplex into main 2026-08-16 18:40:56 +02:00
Conversation 1 Commits 1 Files changed 10 +549 -207
janpeter commented 2026-08-16 18:08:39 +02:00
Owner
Copy link

Waarom

Alle zes /api/realtime-routes luisteren op hetzelfde kanaal scrum4me_changes, maar openden elk een eigen pg.Client per open SSE-stream. Het aantal Postgres-connecties schaalde daardoor lineair mee met het aantal open browser-streams.

Gemeten op prod 2026-08-16: 35 connecties uit één web-proces, allemaal LISTEN en nul gepoolde. Samen met remote clients liep de gedeelde 100-slots server die dag twee keer helemaal vol, met too many clients tot gevolg — ook voor de queue.

De pool-fix van #171 raakte dit niet. Die werkt aantoonbaar (de app houdt nu nul gepoolde connecties), maar de pool was de consument niet. Dit is een andere fout-klasse in dezelfde storing.

Waarom de timeouts het ook niet vangen: een LISTEN-sessie wordt actief levend gehouden en haalt nooit de idle_session_timeout.

Wat deze PR doet

lib/realtime/notify-hub.ts houdt één connectie per kanaal aan en deelt notificaties in-process uit aan alle abonnees. 27 open streams worden zo 1 connectie.

Twee dingen die deze module moet kloppen, beide uit eerdere schade geleerd:

  • De registry staat op globalThis. Next bundelt servercode per route-chunk, dus gewone module-state zou alsnog één listener per route-bundle opleveren in plaats van één per proces — precies de fout die lib/prisma.ts een pool per module-scope liet openen.
  • De connectie keepalivet. De database heeft sinds vandaag idle_session_timeout aan; zonder periodieke no-op wordt een stille LISTEN-connectie server-side weggesweept.

Bij verbindingsverlies krijgt elke abonnee onError en wordt losgekoppeld; de SSE-route sluit dan zijn stream en de EventSource van de browser herverbindt. Dat is exact het gedrag dat de losse clients al hadden — alleen nu gedeeld. Het kanaal wordt gevalideerd tegen een identifier-patroon, want het gaat geïnterpoleerd de LISTEN in.

Meegenomen, beide gevonden tijdens dezelfde diagnose

Ik heb deze twee erbij gedaan omdat een PR die de connectie-explosie aanpakt en ze bewust laat liggen maar 27 van de 35 connecties dekt.

  1. lib/hub/queue-server.ts kreeg een expliciete pool-max via de bestaande poolMaxFromUrl. Zonder die waarde valt node-postgres terug op 10 en negeert het de connection_limit uit de URL — dezelfde fout-klasse als #171, in een tweede bestand.
  2. startQueueListener verving queueListener zónder de vorige te sluiten, en een pg.Client kan zijn error méér dan eens emitten. Eén storing spawnde daardoor meerdere listeners: er stonden er acht tegelijk op agent_queue. Nu wordt de voorganger gesloten en bewaakt een generatieteller de herstart.

Verificatie

  • 9 nieuwe tests op de hub: één connectie bij 25 abonnees, fan-out, unsubscribe zonder de connectie te sluiten, aparte connectie per kanaal, teardown die alle abonnees losmaakt en daarna herverbindt, late events van een dode client genegeerd, keepalive-query, kanaalvalidatie, en geen achtergebleven abonnee als de eerste connect faalt. 9/9 pass.
  • De bestaande SSE-routetests (solo-stream, notifications-stream) blijven groen — 6/6.
  • npm run typecheck: 296 fouten mét én zónder deze wijziging. Met regelnummers genormaliseerd zijn de twee foutverzamelingen identiek; de wijziging voegt er geen enkele toe. (Die 296 zijn pre-existing in de dev-clone door een stale generated client / dirty vendor/scrum4me-shared.)
  • npm test: 17 falende regels mét én zónder de wijziging, niets nieuw en niets stilletjes opgelost.
  • npx eslint op alle tien gewijzigde bestanden: exit 0.

Wat dit niet oplost

De hub-question-listener in runtime-server.ts gebruikt nog een eigen Client voor scrum4me_changes. Dat is één langlevende connectie, geen per-request lek, dus die heb ik bewust laten staan. Wel heeft hij geen keepalive: met idle_session_timeout actief wordt hij elke ~15 minuten weggesweept en herverbindt hij via zijn eigen on('end')-pad. Functioneel zelfherstellend, maar het is onnodige churn — kandidaat voor een volgende PR.

En de tweede helft van de storing zit niet in deze repo: ~45 van de connecties komen van remote clients over Tailscale (MCP-clients die met wait_for_job op hetzelfde kanaal luisteren). Die dragen geen application_name, waardoor ze alleen via NAT-forensiek toewijsbaar zijn.

## Waarom Alle zes `/api/realtime`-routes luisteren op **hetzelfde** kanaal `scrum4me_changes`, maar openden elk een eigen `pg.Client` per open SSE-stream. Het aantal Postgres-connecties schaalde daardoor lineair mee met het aantal open browser-streams. Gemeten op prod 2026-08-16: **35 connecties uit één web-proces, allemaal `LISTEN` en nul gepoolde**. Samen met remote clients liep de gedeelde 100-slots server die dag twee keer helemaal vol, met `too many clients` tot gevolg — ook voor de queue. **De pool-fix van #171 raakte dit niet.** Die werkt aantoonbaar (de app houdt nu nul gepoolde connecties), maar de pool was de consument niet. Dit is een andere fout-klasse in dezelfde storing. Waarom de timeouts het ook niet vangen: een `LISTEN`-sessie wordt actief levend gehouden en haalt nooit de `idle_session_timeout`. ## Wat deze PR doet `lib/realtime/notify-hub.ts` houdt **één connectie per kanaal** aan en deelt notificaties in-process uit aan alle abonnees. 27 open streams worden zo 1 connectie. Twee dingen die deze module moet kloppen, beide uit eerdere schade geleerd: - **De registry staat op `globalThis`.** Next bundelt servercode per route-chunk, dus gewone module-state zou alsnog één listener per route-bundle opleveren in plaats van één per proces — precies de fout die `lib/prisma.ts` een pool per module-scope liet openen. - **De connectie keepalivet.** De database heeft sinds vandaag `idle_session_timeout` aan; zonder periodieke no-op wordt een stille `LISTEN`-connectie server-side weggesweept. Bij verbindingsverlies krijgt elke abonnee `onError` en wordt losgekoppeld; de SSE-route sluit dan zijn stream en de `EventSource` van de browser herverbindt. **Dat is exact het gedrag dat de losse clients al hadden** — alleen nu gedeeld. Het kanaal wordt gevalideerd tegen een identifier-patroon, want het gaat geïnterpoleerd de `LISTEN` in. ## Meegenomen, beide gevonden tijdens dezelfde diagnose Ik heb deze twee erbij gedaan omdat een PR die de connectie-explosie aanpakt en ze bewust laat liggen maar 27 van de 35 connecties dekt. 1. **`lib/hub/queue-server.ts`** kreeg een expliciete pool-`max` via de bestaande `poolMaxFromUrl`. Zonder die waarde valt node-postgres terug op 10 en negeert het de `connection_limit` uit de URL — dezelfde fout-klasse als #171, in een tweede bestand. 2. **`startQueueListener`** verving `queueListener` zónder de vorige te sluiten, en een `pg.Client` kan zijn `error` méér dan eens emitten. Eén storing spawnde daardoor meerdere listeners: er stonden er **acht tegelijk** op `agent_queue`. Nu wordt de voorganger gesloten en bewaakt een generatieteller de herstart. ## Verificatie - **9 nieuwe tests** op de hub: één connectie bij 25 abonnees, fan-out, unsubscribe zonder de connectie te sluiten, aparte connectie per kanaal, teardown die alle abonnees losmaakt en daarna herverbindt, late events van een dode client genegeerd, keepalive-query, kanaalvalidatie, en geen achtergebleven abonnee als de eerste connect faalt. **9/9 pass.** - De bestaande SSE-routetests (`solo-stream`, `notifications-stream`) blijven groen — 6/6. - `npm run typecheck`: **296 fouten mét én zónder** deze wijziging. Met regelnummers genormaliseerd zijn de twee foutverzamelingen **identiek**; de wijziging voegt er geen enkele toe. (Die 296 zijn pre-existing in de dev-clone door een stale generated client / dirty `vendor/scrum4me-shared`.) - `npm test`: **17 falende regels mét én zónder** de wijziging, niets nieuw en niets stilletjes opgelost. - `npx eslint` op alle tien gewijzigde bestanden: exit 0. ## Wat dit niet oplost De hub-question-listener in `runtime-server.ts` gebruikt nog een eigen `Client` voor `scrum4me_changes`. Dat is één langlevende connectie, geen per-request lek, dus die heb ik bewust laten staan. Wel heeft hij geen keepalive: met `idle_session_timeout` actief wordt hij elke ~15 minuten weggesweept en herverbindt hij via zijn eigen `on('end')`-pad. Functioneel zelfherstellend, maar het is onnodige churn — kandidaat voor een volgende PR. En de tweede helft van de storing zit niet in deze repo: ~45 van de connecties komen van **remote clients over Tailscale** (MCP-clients die met `wait_for_job` op hetzelfde kanaal luisteren). Die dragen geen `application_name`, waardoor ze alleen via NAT-forensiek toewijsbaar zijn.
fix(realtime): één gedeelde LISTEN-connectie per kanaal i.p.v. één per SSE-stream
All checks were successful
CI / Lint, Typecheck, Test & Build (pull_request) Successful in 4m9s
Details
CI / Deploy Manual (workflow_dispatch) (pull_request) Has been skipped
Details
CI / Detect deploy-relevant changes (pull_request) Has been skipped
Details
CI / Deploy Preview (PR) (pull_request) Has been skipped
Details
CI / Deploy Production (main) (pull_request) Has been skipped
Details
50f6c6ee0d 
Alle zes /api/realtime-routes luisterden op hetzelfde kanaal, maar openden elk
een eigen pg.Client per open stream. Het aantal Postgres-connecties schaalde
daardoor lineair mee met het aantal open browser-streams.

Gemeten op prod 2026-08-16: 35 connecties uit één web-proces, allemaal LISTEN
en nul gepoolde — samen met remote clients liep de gedeelde 100-slots server
twee keer vol. De pool-fix van #171 raakte dit niet: de pool was niet de
consument.

notify-hub houdt per kanaal één connectie aan en deelt notificaties in-process
uit aan alle abonnees. Twee dingen die het moet doen kloppen:

- de registry staat op globalThis, want Next bundelt servercode per route-chunk
  en gewone module-state zou alsnog één listener per route opleveren — dezelfde
  fout die lib/prisma.ts een pool per module-scope liet openen;
- de connectie keepalivet, want de database heeft idle_session_timeout aan en
  zou een stille LISTEN-connectie anders wegsweepen.

Bij verbindingsverlies krijgt elke abonnee onError en wordt losgekoppeld; de
SSE-route sluit dan zijn stream en de EventSource van de browser herverbindt.
Dat is exact het gedrag dat de losse clients al hadden.

Verder in deze PR, beide gevonden tijdens dezelfde diagnose:

- lib/hub/queue-server.ts kreeg een expliciete pool-max. Zonder die waarde valt
  node-postgres terug op 10 en negeert het de connection_limit uit de URL —
  dezelfde fout-klasse als #171, in een tweede bestand.
- startQueueListener verving queueListener zonder de vorige te sluiten, en een
  pg-Client kan zijn 'error' meer dan eens emitten. Eén storing spawnde zo
  meerdere listeners: er stonden er acht tegelijk op agent_queue. Nu wordt de
  voorganger gesloten en bewaakt een generatieteller de herstart.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
s4m-codex-reviewer requested changes 2026-08-16 18:11:39 +02:00
s4m-codex-reviewer left a comment
Copy link

Verdict: REQUEST_CHANGES

geen gekoppeld plan gevonden — beoordeeld op codekwaliteit + product-standaarden.

Findings

  • error — lib/realtime/notify-hub.ts:115: als client.connect() slaagt maar client.query(\LISTEN ${channel}`)faalt, wordt de net geopende pg-client nergens geregistreerd en ook niet gesloten. DesubscribeToChannel-catch verwijdert alleen de subscriber; teardown()zietstate.client === nullen kan deze client dus niet opruimen. Bij tijdelijke LISTEN/query-fouten kan dit alsnog DB-connecties laten hangen, precies in het realtime-connection pad dat deze PR wil stabiliseren. Zet de client vóór de LISTEN in state of sluit hem inconnect()in een catch/finally voordat de fout doorgegeven wordt, en test datend()/closePgClientSafely` wordt aangeroepen bij een LISTEN-fout na succesvolle connect.

Opmerkingen

De nieuwe gedeelde channel-hub en de route-fanout behouden het bestaande server-side filtercontract uit de realtime-docs, en de toegevoegde tests dekken de normale fanout, reconnect en channel-validatie goed. De startup-cleanup-case mist nog dekking.

# Verdict: REQUEST_CHANGES geen gekoppeld plan gevonden — beoordeeld op codekwaliteit + product-standaarden. ## Findings - error — `lib/realtime/notify-hub.ts:115`: als `client.connect()` slaagt maar `client.query(\`LISTEN ${channel}\`)` faalt, wordt de net geopende pg-client nergens geregistreerd en ook niet gesloten. De `subscribeToChannel`-catch verwijdert alleen de subscriber; `teardown()` ziet `state.client === null` en kan deze client dus niet opruimen. Bij tijdelijke LISTEN/query-fouten kan dit alsnog DB-connecties laten hangen, precies in het realtime-connection pad dat deze PR wil stabiliseren. Zet de client vóór de LISTEN in state of sluit hem in `connect()` in een catch/finally voordat de fout doorgegeven wordt, en test dat `end()`/`closePgClientSafely` wordt aangeroepen bij een LISTEN-fout na succesvolle connect. ## Opmerkingen De nieuwe gedeelde channel-hub en de route-fanout behouden het bestaande server-side filtercontract uit de realtime-docs, en de toegevoegde tests dekken de normale fanout, reconnect en channel-validatie goed. De startup-cleanup-case mist nog dekking.
Sign in to join this conversation.
Reviewers
No reviewers
s4m-codex-reviewer
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
janpeter/Scrum4Me!172
No description provided.