Skip to content

Fix fanout read position tracking so silent instances can't advance the checkpoint - #227

Open
czirker wants to merge 2 commits into
masterfrom
bug/fanout-event-report
Open

czirker wants to merge 2 commits into
masterfrom
bug/fanout-event-report

Conversation

@czirker

@czirker czirker commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Fanout could persist a checkpoint past events that their owning instance never read. In the 2026-09-30 13:03 incident, a batch landed after the workers had read but before the master read. Because the master counted only the instances that checkpointed, it advanced past the batch.

  • Every instance reports how far it read. A new ReadTracker (wrappers/fanout-read-tracker.ts) records the furthest eid each instance can safely claim, including when it owned no events. Each instance's response includes that position, so the master takes the min across all instances.
  • Missing reports are an error. reduceCheckpoints throws if an instance didn't report a queue that another instance reported. Set strictCheckpoints: false to turn this off.
  • All instances read the same window. The max eid now comes from __cron.ts (falls back to now if missing or in the future). Workers get it, plus the starting checkpoints, from the master.
  • Typo fix: reduceCheckpoints used checkpointData.read[queue] where it meant checkpointData[queue].
  • Test fix: test/lib.streams.utest.ts now restores Date.now after its counter and devnull tests. Before, it left the clock stubbed to 1000, which broke the new fanout tests whenever they ran after it.

Test plan

  • New unit tests: fanout-read-tracker.utest, fanout-reduce-guard.utest, fanout-read-position.utest (replays the 13:03 incident through the real wrapper)
  • Full *.utest.js suite passes locally: 336 passing, 0 failing

🤖 Generated with Claude Code


Note

High Risk
Changes core fanout checkpoint semantics and master/worker coordination; incorrect behavior could skip or duplicate queue events in production.

Overview
Fixes fanout persisting a checkpoint past events that partition owners never read (e.g. the 2026-09-30 13:03 incident when a batch landed after workers read but before the master).

Read position voting: Each instance now tracks how far it read via ReadTracker, including unowned events it only filtered past. Worker/master responses use reportCheckpoints() so the master still takes the min across all instances, not only instances that called checkpoint.

Guards and read window: reduceCheckpoints throws when an instance is silent for a queue another instance reported (strictCheckpoints: false opts out). The shared max eid comes from getMaxEid (__cron.ts, with fallbacks), and workers receive maxeid plus starting checkpoints from the master. A merge typo (checkpointData.read[queue] → checkpointData[queue]) is fixed.

Tests: New unit/integration tests replay the incident; stream tests restore Date.now/console.log after counter and devnull so later fanout tests are not affected.

Reviewed by Cursor Bugbot for commit 20b0399. Bugbot is set up for automated code reviews on this repo. Configure here.

@cursor

cursor Bot commented Oct 6, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: b399e0db-7004-448d-b501-748f6fe96457)

@ch-snyk-sa

ch-snyk-sa commented Oct 6, 2026 •

Copy link
Copy Markdown

✅ Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
✅ Open Source Security 0 0 0 0 0 issues
✅ Licenses 0 0 0 0 0 issues
✅ Code Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

This branch has not been deployed

No deployments
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.

2 participants