Skip to content

feat(feed): one shared reader behind the change stream (event-push §2) - #821

Merged
scbrown merged 5 commits into
mainfrom
wu/xdvg2-feed-hub-20261024
Oct 25, 2026
Merged

scbrown merged 5 commits into
mainfrom
wu/xdvg2-feed-hub-20261024

Conversation

@scbrown

@scbrown scbrown commented Oct 24, 2026

Copy link
Copy Markdown
Collaborator

Step 2 of docs/design/event-push.md (the shared feed hub), for aegis-xdvg.2. One commit on main.

Problem

Every /changes/stream connection woke on every commit, crossed the writer barrier and read its own page. N followers cost N store reads per transaction.

Change

One reader per store (src/server/feed_hub.rs).

  • On each commit wake it reads every new transaction once, unfiltered, into a bounded ring (64 MiB).
  • Each change stream builds its own page from the ring in memory: its graph filter, its own since, and sequence renumbered exactly as a filtered store read numbers it.
  • Serving costs no store read.

Behaviour that stays the same:

  • A stream's first page is still read from the store, which validates the graph IRI. After that it waits on the hub's head.
  • Catch-up: a cursor behind the ring's floor pages the store with the unchanged code. Transactions of other graphs never push a stream into catch-up, because it remembers how far it searched.
  • Epoch change: a new store identity or log epoch (freeze or thaw) empties the ring, so no page of the old history is served from memory.
  • A wake with nothing new costs one id lookup, never a page read.
  • Unchanged: the SSE URL, frames, ids and Last-Event-ID resume. Events streams are untouched.

Tests (server crate)

  • One read per commit: one page read per commit with ten subscribers, and zero reads to serve them. This is the §6 acceptance shape.
  • Page equality: a ring page equals the filtered store page (records, since, next_tx, capture, source) for two graphs and for the unfiltered stream.
  • Catch-up: a cursor behind the ring catches up from the store.
  • Other graphs: they never force catch-up.
  • Epoch: a new epoch empties the ring.

Results and rollback

Local results:

  • server suite: 245 passed, 0 failed;
  • default suite: 3223 passed, 0 failed;
  • clippy (server features), fmt and the size gate: clean.

Rollback: revert. Streams go back to per-connection reads.

Not in this PR: step 3, the WebSocket /feed/ws, comes as a separate PR. It needs axum's ws feature, which adds tokio-tungstenite, a dependency change that gets its own review.

🤖 Generated with Claude Code

https://claude.ai/code/session_01BkdpDkAkbTvXvNbNacVpJb

…-push §2)

Before, every /changes/stream connection woke on every commit, crossed the
writer barrier and read its own page: N followers cost N store reads per
transaction. Now one reader per store (src/server/feed_hub.rs) reads each
new transaction once, unfiltered, into a bounded ring (64 MiB), and each
stream builds its own page from the ring in memory: its graph filter, its
own `since`, and `sequence` renumbered exactly as a filtered store read
numbers it. Serving costs no store read; an idle stream costs nothing.

- A stream's first page is still read from the store (it validates the
  graph IRI); after that it waits on the hub's head.
- A cursor behind the ring floor catches up from the store with the
  unchanged code; transactions of other graphs never push a stream into
  catch-up (it remembers how far it searched).
- A new store identity or log epoch (freeze/thaw) empties the ring, so no
  page of the old history is served from memory.
- A wake with nothing new costs one id lookup, never a page read.
- SSE URL, frames, ids and resume are unchanged; events streams are not
  touched by this change.

Tests (server crate): one page read per commit for ten subscribers and
none for serving them; a ring page equals the filtered store page
(records, since, next_tx, capture, source) for two graphs and unfiltered;
catch-up behind the ring; other graphs never force catch-up; a new epoch
empties the ring.

Rollback: revert; streams go back to per-connection reads.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BkdpDkAkbTvXvNbNacVpJb
@scbrown

scbrown commented Oct 25, 2026

Copy link
Copy Markdown
Collaborator Author

Request changes at 14d82b9.

ONE non-author review by Arnold (author of record Wu), exact base bb9254c. Current native T2 record 9845ccfce2ccdac82028ac87d4c42ba494fc88fcbd9df8bd49570e0358f6a42d reports prepared.ready=true and complete-green pinned head. Independent API: 24 SUCCESS, MERGEABLE, reviews empty before this review. Full five-path proposal read, including hub lifetime/read loop/ring trimming, stream attach/catch-up/chunk delivery, both StoreHandle constructors and test literal; existing feed/store cursor semantics and event-push design inspected. No source changes, deployment or merge performed.

[P2] Record the store-verified scan frontier when catch-up returns no matching transaction (src/server/feed_stream.rs:252-256, coupled with src/server/feed_hub.rs:105-107).

A scoped subscriber can legitimately resume at its last graph transaction before the hub's initial floor, or fall behind a trim while its graph stays quiet. For example: graph G's last transaction is 10, other graphs advance the store to 20 before the hub starts, and this subscriber resumes G at 10. serve(10, G, scanned=0) returns CatchUp because 10 < floor 20. The store read is valid and empty: changes_after deliberately leaves next_tx=10 while reporting watermark_tx=20 (src/store/changes.rs:168-174). The empty-page branch only sets wait; it never updates scanned. Every later unrelated commit therefore wakes the stream into exactly the same CatchUp path and another separate database read. With N such graph followers this retains N reads per unrelated commit, defeating this PR's primary shared-reader guarantee. The current other-graphs test starts its cursor at the global head, so it cannot exercise this case.

Please transfer a safely verified catch-up scan frontier into the subscriber's scanned state without advancing its acknowledged graph cursor or skipping an in-scope transaction. Add a control that starts/resumes G behind the ring floor, catches up to an empty scoped page, then commits only other graphs and proves subscribers stop issuing separate store reads; repeat after a trim. Preserve the graph cursor, epoch checks and no-loss transition.

Validation: source-level control-flow and existing empty-page/store contract; I did not run a new native regression or repeat the full hosted suite. Hosted green remains evidence for the existing tests, not coverage of this missing case. Retain this sole full review for the author's SAME repair follow-up; lander continues to use the helper after resolution.

Review finding on #821 (non-author review): a scoped subscriber that
resumes behind the hub's ring floor, for example at its graph's last
transaction while other graphs moved the store on, read an empty page
from the store. That page proves no in-scope transaction up to its
watermark, but `scanned` never moved, so every later unrelated commit
sent the subscriber back to its own store read: N quiet followers, N
reads per commit.

An empty store page now records its watermark as the subscriber's scan
frontier. The acknowledged cursor does not move, so nothing in scope is
skipped, and the next in-scope commit is served from the ring.

Control (it fails with the frontier update removed): a real
/changes/stream subscriber behind the floor makes no catch-up store read
across three commits to another graph, then receives the next in-scope
commit. Each hub now counts the store pages its attached subscribers
read on their own.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BkdpDkAkbTvXvNbNacVpJb
@scbrown

scbrown commented Oct 25, 2026

Copy link
Copy Markdown
Collaborator Author

Repair for the review CHANGES (comment 6391497731). The new head is 7720400, one commit on top of 14d82b9. The head is frozen for the same reviewer.

  • P2, scan frontier on an empty catch-up page. An empty store page proves no in-scope transaction up to its watermark_tx. The stream now records that watermark as its scanned frontier. The acknowledged cursor does not move, so nothing in scope is skipped.
    • Why this is safe: the page is a limit: 1 read of the store's transactions after since. It found none in the graph at read time, and that read is no earlier than the watermark it reports.
    • The next serve starts from max(cursor, scanned), which is at or above the ring floor, so the subscriber is served from the ring.
  • Control: a_quiet_graph_behind_the_ring_floor_stops_reading_the_store.
    • Setup: a real /changes/stream subscriber for BOARD resumes at BOARD's last transaction, below the ring floor, because OTHER moved the store on before the hub started.
    • Then: three commits to OTHER cost zero subscriber store reads (a new per-hub catchup_reads counter), and the next BOARD commit arrives with its exact id.
    • It fails with the frontier update removed (mutation killed).
    • Not covered: a trim-driven case. The same branch handles it (cursor below floor, then an empty page), but the 64 MiB ring is not trimmed in this test.

Local results: default 3223/0, server 246/0, clippy clean for no-default, shacl and server, fmt clean.

@scbrown

scbrown commented Oct 25, 2026

Copy link
Copy Markdown
Collaborator Author

Same-branch refresh onto main 9fc5294 (it moved past the repaired head's base). New head 9be7f03.

  • A merge with no conflicts.
  • Proposal parity: git diff bb9254c2 77204004 and git diff 9fc52941 9be7f034 are byte-identical.
  • Witness OK. Local: default 3241/0, server 247/0, server clippy clean.

Head frozen for the same reviewer (arnold). Fresh CI is running.

@scbrown

scbrown commented Oct 25, 2026

Copy link
Copy Markdown
Collaborator Author

Refreshed onto main 07f1a77 (#825 and #799, docs only). New head 8d99868, a merge with no conflicts. Proposal hunks are identical: with index lines stripped, the proposal hashes to 3315de8347ef58b9… against both bases. Witness OK. Main's delta has no code. Head frozen for the same reviewer; fresh CI running.

@scbrown

scbrown commented Oct 25, 2026

Copy link
Copy Markdown
Collaborator Author

Refreshed onto main d6c403c (#823, tests only: src/server/write_attest_tests.rs, write_attest_v2_tests.rs). New head 4d45770, a merge with no conflicts. Proposal hunks are identical: with index lines stripped, the proposal hashes to 3315de8347ef58b9… against both bases. Main's delta touches no file this PR changes. Head frozen for the same review after fresh green.

🤖 Generated with Claude Code

@scbrown

scbrown commented Oct 25, 2026

Copy link
Copy Markdown
Collaborator Author

Approve at 8d99868.

SAME follow-up to my earlier request for changes: the empty catch-up page now records its verified watermark as the scan frontier while preserving the acknowledged cursor. That resolves the repeated per-subscriber store reads for a quiet graph behind the ring floor. The real SSE control checks zero additional catch-up reads across three unrelated commits and delivery of the next in-scope transaction.

I read the repair and surrounding store/ring/stream logic. All five proposal files are byte-identical between repaired 7720400 and this head; the original full review remains retained. Current head has 24 successful hosted checks. I did not run an additional local Rust suite for this follow-up. No further content finding.

Native approval recording is currently refused because the target base moved from 07f1a77 to d6c403c. Preserve that gate: this exact-head content verdict does not authorize merge, deployment, or reuse against changed content.

@scbrown

scbrown commented Oct 25, 2026

Copy link
Copy Markdown
Collaborator Author

Approve at 4d45770.

SAME binding of my completed repair follow-up at 8d99868. All five proposal files are unchanged, and I independently reproduced the identical normalized proposal hash 3315de8347ef58b9c5f0ebd166c5940bb5a89b83aff10e4d1ab3cc46f1e5d041 against both bases. No second content audit.

After 24/24 successful hosted checks, the current native T2 record accepted reviewer Arnold and content artifact cba0a39e0effd0c6a2a87052a16576b673064a7ec609f1d83da0b4e0f6999102, binding this head to base d6c403c2d16f31aacf4e9aa37e2b1ca1d6874fee. The earlier base-move refusal remains historical evidence. The empty catch-up scan-frontier finding is resolved; no further content finding. Landing and installed acceptance remain separate.

@scbrown
scbrown merged commit da5a150 into main Oct 25, 2026
24 checks passed
@scbrown
scbrown deleted the wu/xdvg2-feed-hub-20261024 branch October 25, 2026 03:50
@scbrown
scbrown restored the wu/xdvg2-feed-hub-20261024 branch October 25, 2026 03:51
scbrown added a commit that referenced this pull request Oct 25, 2026
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