feat(wal,ingester,server): segment rotation + rotation-triggered snapshot cadence (RFC0008.6) - #188
Conversation
…esce on failure (RFC0008.6) Rotation happens in append, before the write, so a frame never straddles segments: the size check includes the frame about to land (the §6.9 lower bound guarantees any legal frame fits a fresh segment), and the age comes from the segment UUIDv7's embedded mint time — §6.5's 'since its header was written', surviving reopen with no extra persisted state. An empty segment never age-rotates. The rotation sequence is fdatasync(old) -> create fresh segment + header -> fsync(parent dir), so the new entry is durable before any frame lands in it and a closed segment can never carry a torn tail (which recovery treats as RFC0008.5 corruption). Any step failing quiesces the WAL: the triggering append surfaces the IO error, every later append returns QuiescedAfterRotationFsyncFailure until an operator intervenes; sync stays available — acking frames already in the old segment is safe. All three RFC0008.6 arms are live. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…§6.9 / RFC0008.10 arm) The pipeline detects a WAL segment change between consecutive durable offsets and fires an installable rotation hook with the rotation-point high-water mark — the old segment's last durable offset — BEFORE the rotating batch's records reach the miner, so a snapshot taken by the hook reflects exactly the frames at or below the mark. serve() wires the hook to the per-tenant snapshot writer (best-effort, like the other cadence points). This activates §6.9's primary cadence, previously blocked on rotation itself; the RFC0008.10 snapshot-cadence arm is live (real WAL driven across its age cap; artefact stamped with the old segment's offset and reflecting only the pre-rotation batch). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 36 minutes and 54 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR implements WAL segment rotation mechanics with a rotation-cadence hook that fires when durable segments change. The WAL gains quiesced-on-failure state, pre-write rotation ordering, and age-based rotation. The ingester pipeline detects rotation via durable offset changes and fires an installed callback before handing records to the miner. The server wires the hook to persist per-tenant snapshots at rotation boundaries. ChangesWAL Rotation and Rotation-Cadence Snapshots
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Pull request overview
Implements RFC0008.6 WAL segment rotation (size-cap + age-cap) and wires a rotation-triggered snapshot cadence hook through the ingester pipeline into the server, enabling RFC0008.10’s rotation cadence behavior.
Changes:
- Add pre-append rotation logic to
Wal::append(size/age triggers) with quiesce-on-rotation-failure behavior. - Introduce
IngestPipeline::with_rotation_hookand fire it on detected segment changes before the rotating batch reaches the miner. - Add integration tests covering WAL rotation arms and rotation-triggered snapshot cadence end-to-end.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| crates/ourios-wal/tests/rfc0008_6_rotation.rs | Adds RFC0008.6 tests for size-cap, time-cap, and quiesce rotation behavior. |
| crates/ourios-wal/src/lib.rs | Implements rotation + quiesce behavior inside append, plus UUIDv7-based age calculation. |
| crates/ourios-server/src/receiver.rs | Wires the ingester rotation hook to snapshot writing as the primary cadence point. |
| crates/ourios-ingester/tests/rfc0008_10_rotation_cadence.rs | Adds RFC0008.10 test asserting rotation triggers correctly stamped snapshots. |
| crates/ourios-ingester/src/receiver/pipeline.rs | Adds rotation hook API + firing semantics before miner ingest on segment change. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…rent-dir fsync Copilot caught the ordering gap: the parent fsync made the new directory entry power-cut-durable while the 24 B header bytes were not, so a crash in that window would surface the next open's header read as OpenError::Corrupt — a benign crash turned unrecoverable. The header is now fdatasync'd first; failure quiesces like every other rotation step. No new test: the effect is kernel cache ordering, unobservable from userspace without filesystem fault injection — the quiesce path the new step joins is already covered by the RFC0008.6 quiesce arm. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…lure The variant covers any rotation-step failure (closing fdatasync, fresh-segment creation, parent-dir fsync), so the fsync-specific name misled operators and log parsing; the Display string and the quiesce arm's test name follow. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@crates/ourios-ingester/src/receiver/pipeline.rs`:
- Around line 176-181: The direct call to rotation_hook (the hook(&self.miner,
prev) invocation inside the if-let) can panic and unwind through the ingest
pipeline, poisoning the shared pipeline mutex; wrap the hook invocation in
std::panic::catch_unwind so panics are contained: call the hook inside a closure
passed to catch_unwind (use std::panic::AssertUnwindSafe if necessary to satisfy
bounds), log or otherwise swallow the Err returned by catch_unwind, and continue
without letting the panic propagate; keep the if-let condition
(self.rotation_hook.as_mut(), before, self.last_durable) and only replace the
direct hook(...) call with the catch_unwind-protected invocation.
🪄 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 Plus
Run ID: d6324e7d-6a23-4346-9548-0a37cd172039
📒 Files selected for processing (5)
crates/ourios-ingester/src/receiver/pipeline.rscrates/ourios-ingester/tests/rfc0008_10_rotation_cadence.rscrates/ourios-server/src/receiver.rscrates/ourios-wal/src/lib.rscrates/ourios-wal/tests/rfc0008_6_rotation.rs
… never unwind ingest Review round 3: the age comparison now uses the full Duration — whole-second truncation delayed the cap by up to a second past the configured bound (the time-cap arm tightened accordingly: the very next append after crossing the cap rotates, and the empty-segment skip is now the load-bearing protection in the first phase). The rotation hook runs under catch_unwind: a panicking snapshot writer must not poison the shared pipeline mutex and halt ingestion over a best-effort cache write — pinned by a test that panics in the hook and asserts the batch still acks, reaches the miner, and the pipeline stays usable. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ion age Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
What
Segment rotation (RFC0008.6) plus the §6.9 rotation-triggered snapshot cadence it was blocking.
ourios-wal— rotation inappend, before the write:fdatasync(old)→ create fresh segment + 24 B header →fsync(parent dir)— the new entry is durable before any frame lands in it, and a closed segment can never carry a torn tail (recovery treats that as RFC0008.5 corruption).Ioerror, every later append returnsQuiescedAfterRotationFsyncFailureuntil an operator intervenes (a fresh open).syncstays available: the old segment is still the append target, and acking frames already written to it is safe.Rotation cadence (
ourios-ingester/ourios-server):IngestPipeline::with_rotation_hook: fired once per detected segment change with the miner as it stands and the rotation-point high-water mark (the old segment's last durable offset), before the rotating batch's records reach the miner — so a snapshot taken by the hook reflects exactly the frames ≤ the mark.serve()wires the hook to the per-tenant snapshot writer (best-effort, like the post-recovery and shutdown cadence points). This activates the §6.9 primary cadence and completes the deferral noted in feat(miner,ingester,server): snapshot restore v2 — per-tenant restore + startup recovery driver #187.Tests
#[cfg(unix)]read-only-root injection; quiesce persists after the condition clears;syncstays Ok; no half-created segment).Invariants / hazards
wal_unflushed_bytesresets on the rotation's closing fdatasync, keeping the RFC0008.9 bound achievable.Checks run
cargo fmt --all --check,cargo clippy --all-targets --all-features -- -D warnings,cargo test --all-features(full workspace, zero failures),cargo doc --no-deps --all-features,cargo bench --no-run— all green locally.RFC 0008 remaining red after this: .1 (wal-before-ack), .3 (recovery O(n)), .4/.5 (torn-write/corruption arms), .8 (batched fsync), .9 (unflushed bound).
🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Tests