fix(ingester): rfc 0047 erasure rewrites count as compactions - #709
Conversation
…rror text Copilot follow-ups on #708: an erasure rewrite now feeds the sweep's IO accounting and audit trail like any compaction (partitions/files/rows/ bytes, a compaction event per committed rewrite, gc failures), so ourios.compaction.* stops under-reporting; the OpenFGA read error text names the step. Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org>
|
Warning Review limit reached
Next review available in: 7 minutes Limit details: You’ve used all 1 included review currently available under your plan. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
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
This PR is a follow-up to the RFC 0047 erasure work, ensuring that erasure-driven partition rewrites are treated as first-class compactions in the ingester sweep’s accounting (metrics + audit trail), and tightening an OpenFGA client error message for clearer diagnostics.
Changes:
- Count committed erasure rewrites as compactions in
SweepReport(partitions/files/rows/bytes + compacted file samples + GC failures) and emit aCompactionaudit event per committed rewrite. - Extend the erasure unit test to assert the rewrite produces a compaction audit event and is reflected in sweep IO accounting.
- Improve OpenFGA
read_by_objecterror text (“read response body”) to identify the failing step.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| crates/ourios-ingester/src/compactor.rs | Treat erasure rewrites as compactions for sweep accounting/auditing; strengthen test assertions around audit + IO stats. |
| crates/ourios-core/src/auth/openfga/client.rs | Fix OpenFGA error message wording for failed Read response body reads. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (1)
crates/ourios-ingester/src/compactor.rs:550
- In the erasure rewrite path,
report.gc_failuresis only incremented when a rewrite is committed (o.committed.is_some()). In the main compaction pass,gc_failuresis incremented for everycompact_partition_hookedoutcome (even when no commit happens), so the sweep’s GC-failure accounting can be under-reported for erasure partitions. Consider mirroring the main pass by moving thereport.gc_failures += o.gc_failuresline outside theif let Some(committed)block.
if let Some(committed) = &o.committed {
outcome.partitions_rewritten += 1;
outcome.rows_dropped += o.rows_dropped;
// An erasure rewrite is a compaction like any
// other for the sweep's IO accounting and audit
Summary
Follow-up to #708 (Copilot's two suppressed notes):
partitions_compacted/files_compacted/rows_compacted/bytes_read/compacted_files(bytes written) /gc_failures, plus acompactionaudit event per committed rewrite — soourios.compaction.*no longer under-reports erasure IO and the audit stream shows which files were rewritten. Theconversation_erasedevent still comes last (unit test asserts both).read_by_objecterror text names the failing step ("read response body") instead of the copy/paste"read read".Related
RFC 0047 (#705–#708).
Checklist
cargo fmtcleancargo clippyclean (no new warnings)🤖 Generated with Claude Code
https://claude.ai/code/session_01JZXtbyWoQY19ZGtNecDfgv