Repository navigation
Closed
Labels
app/idPackage/app: id (pkgs/ or apps/ in this repo)Package/app: id (pkgs/ or apps/ in this repo)kind/performancePerformance problem or optimizationPerformance problem or optimizationpriority/normalOrdinary priority (the default)Ordinary priority (the default)source/pr-comment-reviewFiled by the pr-comment-review jobFiled by the pr-comment-review jobtask/triageQueue packet kind: verify, reproduce, and route this itemQueue packet kind: verify, reproduce, and route this item
Description
Finding (from PR review, never addressed)
Every replication subscription materializes a complete copy of the world journal, and keeps it for the whole session:
replication_snapshotdoes a fullstd::fs::readof the journal file, and it runs inside the world actor's serial command handler — so each new feed blocks that world's writes while the whole journal is read, and each subscriber gets its own full copy (CodeRabbit, Major).- In the network layer, the owned initial
snapshotlocal inserve_streamstays in scope across the indefinite update loop aftersend_snapshotfinishes with it, so every long-lived follower retains a full historical journal copy for its entire session, even after compaction replaces that history (Codex, P2).
Raised on PR #4589:
- CodeRabbit (Major, Stability & Availability), 2026-11-02T21:37:25Z: feat(id): serve signed world journal replication (#806 phase 2) #4589 (comment) —
pkgs/id/src/world_store.rs:208: replace the full-journal read in the actor's feed handling with an incremental or bounded read - Codex (P2), 2026-11-02T21:31:55Z: feat(id): serve signed world journal replication (#806 phase 2) #4589 (comment) — "Release the initial snapshot before following updates" (
pkgs/id/src/world_replication_net.rs:125): explicitly drop the snapshot after the initial transfer or scope its ownership to that transfer
No-response evidence
- PR feat(id): serve signed world journal replication (#806 phase 2) #4589 had a single commit; both comments predate the merge (merged 2026-11-02T22:21:20Z as-is).
- All 12 review threads on PR feat(id): serve signed world journal replication (#806 phase 2) #4589 are unresolved (GraphQL
reviewThreads, checked 2026-11-03); no replies to either comment.
Still live on current main (verified 2026-11-03 via gh api raw reads)
pkgs/id/src/world_store.rson main (:206-207):replication_snapshotstill beginslet bytes = std::fs::read(&self.path).pkgs/id/src/world/actor.rson main (:1577-1591): theReplicationFeedhandler still calls that snapshot synchronously in the actor's command handling.pkgs/id/src/world_replication_net.rson main (:114-127):serve_streambindssnapshotand passes&snapshottosend_snapshot; the owned buffer remains in scope for the whole update loop that follows.
Acceptance criteria
- Feed setup no longer performs an unbounded full-journal read inside the actor's serial handler (incremental/bounded read, or the read moved off the actor), and the network layer drops the initial snapshot buffer once the initial transfer completes.
- A test or measurement shows per-subscriber memory no longer scales with full journal size for the session's lifetime.
- Or: a documented decision on this issue.
Found by the pr-comment-review job from unaddressed PR review comments.
Metadata
Metadata
Assignees
Labels
app/idPackage/app: id (pkgs/ or apps/ in this repo)Package/app: id (pkgs/ or apps/ in this repo)kind/performancePerformance problem or optimizationPerformance problem or optimizationpriority/normalOrdinary priority (the default)Ordinary priority (the default)source/pr-comment-reviewFiled by the pr-comment-review jobFiled by the pr-comment-review jobtask/triageQueue packet kind: verify, reproduce, and route this itemQueue packet kind: verify, reproduce, and route this item
Activity
dezren39 commentedon Nov 3, 2026
🤖 Agent note · filed
dezren39 commentedon Nov 3, 2026
🤖 Agent note · picked up
dezren39 commentedon Nov 3, 2026
🤖 Agent note · done
Triage verdict: CONFIRMED on current main (fresh
gh apiraw reads, 2026-11-03):pkgs/id/src/world_store.rs—replication_snapshotstill beginslet bytes = std::fs::read(&self.path): a full, unbounded journal read on every snapshot.pkgs/id/src/world/actor.rs— theReplicationFeedhandler subscribes, then synchronously callsjournal.replication_snapshot()inside the actor's serial command handling, so each new feed blocks that world's writes for the duration of the whole-journal read.pkgs/id/src/world_replication_net.rs—serve_streambinds the ownedsnapshot, passes&snapshottosend_snapshot, and never drops it; the buffer stays in scope across the indefinite update loop that follows. The loop'sResetbranch sends its ownresetsnapshot, so the initial buffer is not needed after the initial transfer — an explicit drop (or scoping the ownership to the transfer) satisfies that half.No decision is needed from Drew: the fix is well-scoped. Enqueued as task 6713 (small_fix, repo-health-code-pilot) carrying issue #4725's acceptance criteria, including a test/measurement showing per-subscriber memory no longer scales with full journal size for the session lifetime. Issue #4725 stays open for task 6713.
1 remaining item
dezren39 commentedon Nov 3, 2026
🤖 Agent note · handed off
OpenCode worker session bound and prompted for this already-claimed task.
dezren39 commentedon Nov 3, 2026
🤖 Agent note · picked up
dezren39 commentedon Nov 3, 2026
🤖 Agent note · done
cargo test --manifest-path pkgs/id/Cargo.toml replication_— 22 passed, 0 failed on the merged tree. Codex re-review on the new head completed with no findings. PR fix(id): read replication snapshots off actor #4740 is now MERGED (merge commit 581a16e), closing issue id(world-replication): every subscription reads the whole journal inside the actor and retains the copy for the session (PR #4589) #4725. No duplicate implementation was started.