docs(rfc-0014): ingest write path — record sink & flush policy (drafted) - #240
Conversation
Draft RFC 0014, the missing ingest write path: production wires `NoOpRecordSink`, so mined records are dropped today. Specifies a buffering `RecordSink` that accumulates `MinedRecord`s per partition and flushes them to Parquet objects on the RFC 0013 `Store` seam, with a hybrid flush policy — per-partition size + age window, force-flush on WAL segment rotation (RFC 0008) — under a hard buffered-bytes ceiling with backpressure, reusing the WAL's batch/rotation machinery. Records reach the sink post-WAL-durable, so an un-flushed buffer is always recoverable by replay (§3.4). Scope is narrow: the flush policy + the sink. Server `Store` wiring and compaction's `publish_cas`-on-S3 migration (which together green RFC0013.6) are follow-on, tracked as open questions. §§1–4/7/8 filled; §5/§6 sketched for the `specified` pass. Touches CLAUDE.md §4 (small-file), §3.4 (WAL durability), §3.7 (multi-tenancy) — hence RFC-gated per §5.1. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 47 minutes and 59 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ 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 (1)
📝 WalkthroughWalkthroughAdds RFC 0014, a 282-line design document specifying a ChangesRFC 0014 — Ingest Write Path
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 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
Adds a new drafted RFC (0014) documenting the missing ingest write path from mined records to Parquet objects, including a proposed buffering RecordSink and a hybrid flush policy that ties size/age triggers to WAL segment rotation.
Changes:
- Add RFC 0014 (“Ingest write path — record sink and flush policy”) as a new doc.
- Link RFC 0014 from
docs/SUMMARY.mdso it appears in the mdBook navigation.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 10 comments.
| File | Description |
|---|---|
| docs/SUMMARY.md | Adds RFC 0014 to the RFC index for navigation. |
| docs/rfcs/0014-ingest-write-path.md | Introduces drafted RFC 0014 specifying the record sink + flush policy design and testing/acceptance sketches. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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/rfcs/0014-ingest-write-path.md`:
- Line 7: The created date in the RFC metadata is set to 2026-06-17, which is
one day in the future when it should reflect today's date of 2026-06-16. Update
the created field from 2026-06-17 to 2026-06-16 to align the RFC metadata with
the actual draft date and maintain chronological consistency throughout the
document.
- Around line 103-105: The RFC document uses inconsistent language when
describing the max_buffer_age threshold, alternating between "reaches" and
"exceeds" without explicitly stating whether the boundary is inclusive or
exclusive. This ambiguity can lead to off-by-one errors during implementation.
Clarify the boundary semantics by choosing one consistent term (either "reaches"
for inclusive >= semantics or "exceeds" for exclusive > semantics) and apply it
uniformly across all sections that reference max_buffer_age, including both the
section at lines 103-105 (Age trigger description) and the section at lines
213-215 (also referenced in the comment). Ensure the chosen wording aligns with
the acceptance criterion that states the flush happens on the next tick.
🪄 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: fc4e92d3-0fc6-40c5-83b0-fae6c591998b
📒 Files selected for processing (2)
docs/SUMMARY.mddocs/rfcs/0014-ingest-write-path.md
…er_age boundary (review)
RFC 0014 (
drafted) — the missing ingest write path. Production wiresNoOpRecordSink, so mined records are dropped today; this RFC specifies how they become Parquet objects on the store.The gap
Every ingest layer is built and tested (OTLP → WAL → miner; Parquet writer/reader; compaction; the RFC 0013
Storeseam + buffer-and-putWriter) — but the miner emits into aRecordSinkwhose only production impl isNoOpRecordSink. No RFC specifies when mined records flush to a Parquet file. That flush policy is a §4 (small-file) / §3.4 (WAL-durability) / §3.7 (multi-tenancy) decision → RFC-gated per CLAUDE.md §5.1.The design (per maintainer direction)
Scope (narrow)
Just the flush policy + the
RecordSink. Follow-on (tracked as open questions): server constructs/injects aStore(RFC 0004 local-vs-S3), and compaction's manifest publish adoptsManifest::publish_cason S3 — together those green RFC0013.6.Status
drafted: §§1–4, 7, 8 filled; §5 acceptance (RFC0014.1–.6) + §6 testing sketched for thespecifiedpass. Mermaid flow diagram included (mdbook build verified). Doc-only PR.🤖 Generated with Claude Code
Summary by CodeRabbit