Skip to content

fix(redis): read the reconciliation backlog on its own connection - #6

Merged
LounisBou merged 3 commits into
mainfrom
fix/redis-reconciliation-reader
Sep 29, 2026
Merged

LounisBou merged 3 commits into
mainfrom
fix/redis-reconciliation-reader

Conversation

@LounisBou

@LounisBou LounisBou commented Sep 25, 2026 •

Copy link
Copy Markdown

A subscriber reconnecting with a Last-Event-ID makes reconciliate() await a LRANGE key -size -1 of the whole stored list on the command connection. The watchdog pings that same connection under readTimeout, and a Redis connection answers in order, so the PONG waits behind the LRANGE reply. With a list of 30000 updates of about 4 KB each, the reply takes more than 5 seconds to stream (clue/redis-protocol re-parses the multi-bulk from its start on every chunk), the timeout rejects, Hub::die() stops the hub, every EventSource reconnects with its Last-Event-ID and the loop repeats. This is what a hub run with pingInterval=2&readTimeout=5 did on a live deployment: four replicas each dying about 7 seconds after their start, 455 exits in 21 minutes, until pingInterval=0 was set back.

Reproduced on 0.5.1 with Redis 7.2.4: a 30000-entry list and one subscriber reconnecting with a Last-Event-ID end the hub with TimeoutException: Timed out after 5 seconds about 9 seconds after boot, while an empty list, a subscriber without Last-Event-ID or readTimeout=0 never do. A similar scenario, three subscribers reconnecting at once on a 35 MB list, ends 0.5 the same way, so the timeout on the command connection predates bpolaszek#52. A publish landing during a reconciliation waited behind the same LRANGE.

The reconciliation reads now go through a third lazy client, so the pinged connection never waits behind a bulk reply. RedisTransport takes it as an optional $reader argument defaulting to the command client, and the factory creates it. The watchdog is unchanged: the command and the subscription connections are both still pinged under readTimeout. The reader is not pinged, the lazy client closes it after its idle period, and a reconciliation that fails no longer ends the hub.

Integration tests against a real Redis (FREDDIE_TEST_REDIS_DSN, exported by the integration job of the CI, skipped when unset) fail on the current code for the head-of-line blocked PONG and for a hub dying on a busy but healthy connection, and pass with the fix: the PONG answered under a second during a reconciliation, two concurrent reconciliations completing without a death, a paused Redis still ending the hub within pingInterval + readTimeout, a killed subscription connection still ending it at once, the subscribed connection still answering ["pong", ""], and pingInterval=0 still disabling the watchdog. A unit test checks that the factory creates the third client.

@LounisBou
LounisBou marked this pull request as ready for review September 25, 2026 09:43
@LounisBou
LounisBou changed the base branch from upstream-main to main September 25, 2026 09:47
@LounisBou
LounisBou changed the base branch from main to upstream-main September 25, 2026 09:49
@LounisBou
LounisBou changed the base branch from upstream-main to main September 25, 2026 12:46
@LounisBou LounisBou closed this Sep 25, 2026
@LounisBou LounisBou reopened this Sep 29, 2026
@LounisBou
LounisBou merged commit ceb5dae into main Sep 29, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant