feat(event): ADR-0010 — bounded page rotation with generation-tagged reuse (design + prototype) - #1869
Conversation
…agged reuse Design-only ADR, landed before any prototype per architecture.md §14 and ADR-0009's explicit "revisit before adding page rotation" stop. Specifies RotatingAdmissionChannel<N, P>: a fixed preallocated pool of P page slots addressed by a monotonic u64 page epoch (slot = epoch mod P, generation tag = epoch as u32); rotation attempted only on typed page/descriptor-full results and only before ring reservation, preserving ADR-0009's phase discipline; typed PageQuotaExhausted backpressure with zero page/ring mutation when all P slots are outstanding; a single consumer→producer released-epoch Release/Acquire reclamation edge whose safety rests on the frame lease's mutable borrow of the consumer (no crossbeam-epoch for the SPSC topology — explicitly non-transferable to any multi-reader future); page-boundary committed-count checks that strengthen ADR-0009's terminal semantics; and an ABA/wrap argument bounded by the at-most-P live-epoch window. Also enumerates the evidence classes the implementing PR must provide (rotation seams, quota refusal, stale-generation injection, boundary shortfall, fault injection, differential oracle across rotations, Loom release/rebind models, Miri, sanitizer lanes, zero-allocation proof). Ledger honesty: adr/index.md marks 0010 Proposed design-only; ROADMAP Week-4 progress and the Implementation_Status v2 ledger state that no rotation code exists; nothing moves to shadow/qualified.
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
📝 WalkthroughWalkthroughChangesThe PR adds design-only ADR-0010 for bounded page rotation and generation-tagged reuse, then updates the ADR index, documentation navigation, quality inventory, roadmap, and implementation-status records. ADR-0010 Documentation
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a design-only Architectural Decision Record (ADR-0010) specifying bounded page rotation with generation-tagged reuse, updating the roadmap, implementation status, and documentation index accordingly. The review feedback highlights two critical correctness issues in the proposed design: a slot addressing discrepancy when the page epoch exceeds P is not a power of two, and an initialization bug in the quota check protocol that could lead to premature slot overwrites before the consumer releases them.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| Pages are identified by a monotonically increasing **page epoch** `e` | ||
| (`u64`, starting at 0). Epoch `e` occupies pool slot `e mod P` and stamps its | ||
| descriptors with `arena_generation = e as u32` (wrapping). The consumer | ||
| recovers the slot index from a descriptor as | ||
| `descriptor.arena_generation as u64 mod P` and then requires **exact | ||
| generation equality** with the slot's currently-bound page before any |
There was a problem hiding this comment.
There is a potential correctness issue in the slot addressing logic when the epoch e exceeds P is not a power of two (or more generally, a divisor of arena_generation is stored as e as u32 (wrapping), the consumer recovers the slot index using descriptor.arena_generation as u64 mod P, which is equivalent to e mod P.\n\nIf P is not a divisor of P = 3), then P to be a power of two (e.g., P always divides e & (P - 1)).
| Pages are identified by a monotonically increasing **page epoch** `e` | |
| (`u64`, starting at 0). Epoch `e` occupies pool slot `e mod P` and stamps its | |
| descriptors with `arena_generation = e as u32` (wrapping). The consumer | |
| recovers the slot index from a descriptor as | |
| `descriptor.arena_generation as u64 mod P` and then requires **exact | |
| generation equality** with the slot's currently-bound page before any | |
| Pages are identified by a monotonically increasing **page epoch** `e`\n(`u64`, starting at 0). Epoch `e` occupies pool slot `e mod P` (where `P` is restricted to a power of two) and stamps its\ndescriptors with `arena_generation = e as u32` (wrapping). The consumer\nrecovers the slot index from a descriptor as\n`descriptor.arena_generation as u64 mod P` and then requires **exact\ngeneration equality** with the slot's currently-bound page before any |
| Acquire-load consumer released_epoch [A] | ||
| require active_epoch + 1 <= released_epoch + P (slot free?) | ||
| on failure: return typed PageQuotaExhausted — nothing mutated |
There was a problem hiding this comment.
There is an initialization/sentinel bug in the quota check protocol.\n\nIf released_epoch is initialized to 0, the producer's check active_epoch + 1 <= released_epoch + P will falsely succeed for the first P rotations even if the consumer has not actually released any epochs yet.\n\nFor example, if P = 2 and released_epoch is initialized to 0:\n1. Producer starts at active_epoch = 0 (slot 0).\n2. Producer rotates to active_epoch = 1 (slot 1): 1 <= 0 + 2 (True).\n3. Producer rotates to active_epoch = 2 (slot 0): 2 <= 0 + 2 (True).\n\nAt this point, the producer overwrites slot 0 (epoch 0) even though the consumer has not yet stored released_epoch = 0 (which only happens when the consumer validates the first descriptor of epoch 1).\n\nRecommendation:\nSpecify that released_epoch must be initialized to a sentinel value indicating that no epochs have been released yet (for example, u64::MAX or -1 in wrapping/signed comparison terms), so that the first overwrite of slot 0 (rotating to epoch P) is correctly blocked until the consumer explicitly stores released_epoch = 0.
| Acquire-load consumer released_epoch [A] | |
| require active_epoch + 1 <= released_epoch + P (slot free?) | |
| on failure: return typed PageQuotaExhausted — nothing mutated | |
| Acquire-load consumer released_epoch [A]\n require active_epoch + 1 <= released_epoch.wrapping_add(P) (slot free, with released_epoch initialized to u64::MAX)\n on failure: return typed PageQuotaExhausted — nothing mutated |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/adr/0010-bounded-page-rotation-generation-reuse.md`:
- Around line 68-71: Update the bounded page rotation specification to replace
the ambiguous released_epoch with a released_count initialized at zero, use the
strict check active_epoch + 1 < released_count + P, and release by storing
released_count = e + 1. In
docs/adr/0010-bounded-page-rotation-generation-reuse.md lines 68-71, 109-111,
120-125, 139-141, 179-180, and 225-226, update the algorithm, reclamation edge,
sequence diagram, window bounds, memory layout, and alternatives terminology; in
ROADMAP.md lines 70-74, rename the reclamation edge accordingly.
- Around line 141-144: Correct the bounded-generation proof in the ADR: remove
the claim that the type-level P: usize bound enforces P < 2^32, and state that
implementations must enforce this limit with an explicit const assertion or
equivalent validation. Preserve the requirement that the u32 generation tag
remains unambiguous only when P < 2^32.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f3a44177-2b57-4677-a586-ccb892628d43
📒 Files selected for processing (6)
ROADMAP.mddocs/Documentation_Quality_Report.mddocs/Implementation_Status.mddocs/adr/0010-bounded-page-rotation-generation-reuse.mddocs/adr/index.mdmkdocs.yml
| on page/descriptor full: | ||
| Acquire-load consumer released_epoch [A] | ||
| require active_epoch + 1 <= released_epoch + P (slot free?) | ||
| on failure: return typed PageQuotaExhausted — nothing mutated |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | 🏗️ Heavy lift
Data race / ABA guard flaw: released_epoch ambiguity and off-by-one overwrite.
The rotation algorithm uses an ambiguous released_epoch index (if initialized to 0, it means both "no epochs released" and "epoch 0 released") and an off-by-one condition (active_epoch + 1 <= released_epoch + P). This mathematical flaw allows P+1 live epochs, permitting the producer to overwrite the live tail page before the consumer ever releases it. The design must track a released_count (starting at 0) and use a strict < bound.
docs/adr/0010-bounded-page-rotation-generation-reuse.md#L68-L71: Change the load toreleased_countand the check torequire active_epoch + 1 < released_count + P.docs/adr/0010-bounded-page-rotation-generation-reuse.md#L109-L111: Change step 2 toRelease-stores released_count = e + 1.docs/adr/0010-bounded-page-rotation-generation-reuse.md#L120-L125: Update the reclamation edge text and sequence diagram to usereleased_count.docs/adr/0010-bounded-page-rotation-generation-reuse.md#L139-L141: Update the window bounds text to[released_count, active_epoch].docs/adr/0010-bounded-page-rotation-generation-reuse.md#L179-L180: Renamereleased_epochtoreleased_countin the memory layout block.docs/adr/0010-bounded-page-rotation-generation-reuse.md#L225-L226: Renamereleased-epoch countertoreleased-count counterin the alternatives text.ROADMAP.md#L70-L74: Renamereleased-epoch reclamation edgetoreleased-count reclamation edgein the Week 4 progress narrative.
📍 Affects 2 files
docs/adr/0010-bounded-page-rotation-generation-reuse.md#L68-L71(this comment)docs/adr/0010-bounded-page-rotation-generation-reuse.md#L109-L111docs/adr/0010-bounded-page-rotation-generation-reuse.md#L120-L125docs/adr/0010-bounded-page-rotation-generation-reuse.md#L139-L141docs/adr/0010-bounded-page-rotation-generation-reuse.md#L179-L180docs/adr/0010-bounded-page-rotation-generation-reuse.md#L225-L226ROADMAP.md#L70-L74
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/adr/0010-bounded-page-rotation-generation-reuse.md` around lines 68 -
71, Update the bounded page rotation specification to replace the ambiguous
released_epoch with a released_count initialized at zero, use the strict check
active_epoch + 1 < released_count + P, and release by storing released_count = e
+ 1. In docs/adr/0010-bounded-page-rotation-generation-reuse.md lines 68-71,
109-111, 120-125, 139-141, 179-180, and 225-226, update the algorithm,
reclamation edge, sequence diagram, window bounds, memory layout, and
alternatives terminology; in ROADMAP.md lines 70-74, rename the reclamation edge
accordingly.
| `[released_epoch, active_epoch]`, a window of width `<= P`. The `u32` | ||
| generation tag is unambiguous while `P < 2^32`, which the type-level | ||
| `P: usize` bound enforces absurdly early; the epoch counter itself is `u64` | ||
| and non-wrapping for any realistic process lifetime (`2^64` rotations). |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
usize does not bound P < 2^32 on 64-bit architectures.
On 64-bit platforms, usize::MAX is 2^64 - 1, which easily permits P >= 2^32. The proof relies on a false claim about the type system. If P exceeds 2^32, the u32 generation tag will wrap and ABA safety is lost. Either clarify that a const assertion is required, or correct the claim about usize.
💡 Proposed fix
- FIFO, so a descriptor observable by the consumer references an epoch in
- `[released_epoch, active_epoch]`, a window of width `<= P`. The `u32`
- generation tag is unambiguous while `P < 2^32`, which the type-level
- `P: usize` bound enforces absurdly early; the epoch counter itself is `u64`
+ FIFO, so a descriptor observable by the consumer references an epoch in
+ `[released_count, active_epoch]`, a window of width `<= P`. The `u32`
+ generation tag is unambiguous while `P < 2^32`, which a `const` assertion
+ must enforce (since `usize` allows larger values on 64-bit systems); the epoch counter itself is `u64`📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| `[released_epoch, active_epoch]`, a window of width `<= P`. The `u32` | |
| generation tag is unambiguous while `P < 2^32`, which the type-level | |
| `P: usize` bound enforces absurdly early; the epoch counter itself is `u64` | |
| and non-wrapping for any realistic process lifetime (`2^64` rotations). | |
| FIFO, so a descriptor observable by the consumer references an epoch in | |
| `[released_count, active_epoch]`, a window of width `<= P`. The `u32` | |
| generation tag is unambiguous while `P < 2^32`, which a `const` assertion | |
| must enforce (since `usize` allows larger values on 64-bit systems); the epoch counter itself is `u64` | |
| and non-wrapping for any realistic process lifetime (`2^64` rotations). |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/adr/0010-bounded-page-rotation-generation-reuse.md` around lines 141 -
144, Correct the bounded-generation proof in the ADR: remove the claim that the
type-level P: usize bound enforces P < 2^32, and state that implementations must
enforce this limit with an explicit const assertion or equivalent validation.
Preserve the requirement that the u32 generation tag remains unambiguous only
when P < 2^32.
Summary
Two commits, design-first as ADR-0009's "revisit when" clause required:
Commit 1 — the design (ADR-0010, Proposed).
docs/adr/0010-bounded-page-rotation-generation-reuse.mdspecifiesRotatingAdmissionChannel<N, P>: a bounded series of page epochs over one SPSC ring — epoch addressing (generation = epoch as u32, at mostPlive), rotation only on typed page-full results and only before ring reservation (ADR-0009's[P]→[R]→[C]phase discipline unchanged), typedPageQuotaExhaustedbackpressure with zero mutation, a single consumer→producerreleased_epochRelease/Acquire reclamation edge whose safety rests on the frame lease's mutable borrow of the consumer (borrow checker replaces crossbeam-epoch for this SPSC topology — explicitly non-transferable to any multi-reader future), seam-strengthened terminal semantics, and an ABA/wrap argument bounded by the at-most-Plive-epoch window.Commit 2 — the unwired prototype (
lib/event/src/rotating.rs), permitted by Proposed status, with deviations disclosed in the ADR's new §Prototype notes:shadowwiring;Pis a power of two ≥ 2; construction rejects a nonzeroarena_generation(epochs own the tag).Evidence (per the ADR's verification contract)
tests/rotating.rs, wired into the ASan/TSan CI lanes): 10k variable-length events across ~1.2k rotations under live quota backpressure — no loss, duplication, reordering, or payload mismatch; plus a pool-bound test proving a slow consumer caps live pages atP.[S]/ handoff publication /[R]/ boundary[E]under all interleavings, bounded exploration. Honest note: the first version used an unbounded poll loop and demonstrably exploded Loom's state space (killed after 15+ minutes); rewritten to the crate's bounded-attempts + deterministic-drain pattern (25s).--test rotating; all-features clippy-D warnings; fmt; workspace check.Status honesty
ADR-0010 stays Proposed (index updated: "landed design-first; prototype follows in the same review"). The fabric remains
target: noshadowtraffic, no protected evidence, no performance claims.ROADMAP.mdWeek-4 progress and theImplementation_Status.mdv2 ledger name the remaining gates, including in-place slot reuse.Test plan
cargo test -p aegis-eventand--all-features(9 suites, incl. 11 rotating unit + 2 stress tests)cargo test -p aegis-event --features loom loom_— 13 modelscargo +nightly miri test -p aegis-event— full crate-Zbuild-std) on--test rotatinglocally; CI lane extendedcargo clippy -p aegis-event --all-targets --all-features -- -D warnings,cargo fmt --all -- --check,cargo check --workspacenode scripts/validate-docs.mjs— 0 errors/warnings