test(bench): streaming corpus store builds + opt-in log4j severity - #350
Conversation
The B1/B2 query-store builds streamed everything except the corpus load itself: corpus::load materializes ~2-4x the raw bytes as a Vec<OtlpLogRecord>, capping the loadable corpus far below the 10-100 GiB the scale targets speak to. corpus::stream is a lazy per-line iterator over the same single walker (collect_files) — same records, same order, same accounting, pinned record-for-record by test — and build_store consumes it via harness::run_streaming, so peak memory is flat at any corpus size. The --gates path keeps the eager load. OURIOS_CORPUS_SEVERITY=log4j (opt-in, unknown values a hard error) extracts the first standalone level token from plain-text lines so LogHub HDFS-family corpora get a real error band for B1's predicate; the default stays §3.3's pinned INFO, so recorded template-keyed numbers (A1/C1/C2) are untouched. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 10 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. 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: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughCorpus loading in ourios-bench is refactored to separate file discovery (CorpusFiles/collect_files) from record ingestion, with shared txt_record and jsonl_line_records helpers reused by eager and streaming paths. Harness streaming now uses Result-typed input. TxtSeverity is threaded through store builders and benchmarks, with tests updated accordingly. ChangesCorpus Streaming and Severity Wiring
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Benchmark as b1.rs / b2.rs
participant Store as store.rs
participant Corpus as corpus.rs
participant Harness as harness.rs
Benchmark->>Store: build_b1_store/build_query_store(corpus_dir, txt_severity)
Store->>Corpus: stream(corpus_dir, txt_severity)
Corpus-->>Store: CorpusStream + file metadata
Store->>Harness: run_streaming(stream, callback)
Harness->>Corpus: pull next Result<OtlpLogRecord>
Corpus-->>Harness: OtlpLogRecord (via jsonl_line_records/txt_record)
Harness->>Store: callback(&record)
Store-->>Benchmark: BuiltStore/B1Store or no_lines_error
Possibly related PRs
🚥 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 |
There was a problem hiding this comment.
Pull request overview
This PR updates the ourios-bench crate to (1) build B1/B2/query stores from a streaming corpus iterator (removing the need to materialize the entire corpus in memory) and (2) add an opt-in plain-text severity extraction mode (OURIOS_CORPUS_SEVERITY=log4j) to unlock severity selectivity for large text corpora while keeping the default baseline behavior unchanged.
Changes:
- Introduce a streaming corpus loader (
corpus::stream) and route query-store builds throughharness::run_streamingto keep peak memory usage low for large corpora. - Add
TxtSeveritywithFixed(default) andLog4j(opt-in) modes, wired into B1/B2 benches and store builders. - Add/extend tests to pin stream≡load equivalence and validate log4j severity extraction behavior.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| crates/ourios-bench/src/store.rs | Plumbs TxtSeverity through store builders and switches the build pipeline to stream records instead of eagerly loading them. |
| crates/ourios-bench/src/lib.rs | Re-exports TxtSeverity as part of the bench crate API surface. |
| crates/ourios-bench/src/harness.rs | Adds run_streaming and reworks run to delegate to the streaming loop. |
| crates/ourios-bench/src/corpus.rs | Splits traversal into collect_files, adds streaming iterator (CorpusStream), and implements opt-in log4j severity extraction for plain-text. |
| crates/ourios-bench/benches/b1.rs | Reads TxtSeverity from env and passes it to build_b1_store. |
| crates/ourios-bench/benches/b2.rs | Reads TxtSeverity from env and passes it to build_query_store. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/ourios-bench/src/corpus.rs (1)
524-538: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAvoid cloning the corpus file list
store.rsonly needs the accounting fields fromCorpusFiles, soCorpusFiles.filesdoesn’t need to stay live alongsideCorpusStream. Moving the file list intoCorpusStreamwould avoid keeping two copies resident during the run.🤖 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 `@crates/ourios-bench/src/corpus.rs` around lines 524 - 538, The corpus stream setup currently clones the collected file list, leaving the same files resident in both CorpusFiles and CorpusStream. Update stream to move the files vector into CorpusStream instead of cloning it, and adjust CorpusFiles usage so store.rs still gets only the accounting fields it needs while CorpusStream owns the iterator source.
🤖 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.
Nitpick comments:
In `@crates/ourios-bench/src/corpus.rs`:
- Around line 524-538: The corpus stream setup currently clones the collected
file list, leaving the same files resident in both CorpusFiles and CorpusStream.
Update stream to move the files vector into CorpusStream instead of cloning it,
and adjust CorpusFiles usage so store.rs still gets only the accounting fields
it needs while CorpusStream owns the iterator source.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: cdc29269-774b-4115-ab2a-5328e0a90c18
📒 Files selected for processing (6)
crates/ourios-bench/benches/b1.rscrates/ourios-bench/benches/b2.rscrates/ourios-bench/src/corpus.rscrates/ourios-bench/src/harness.rscrates/ourios-bench/src/lib.rscrates/ourios-bench/src/store.rs
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The callback is now fallible: a Parquet write / observe error stops the mine at the failing record instead of stashing the error and CPU-mining the remaining corpus (minutes wasted at the 10-100 GiB scale this path exists for). Also scopes the memory-flat claim to the record/raw-byte dimension (the file list is O(files)). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…streaming loop Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…he band map Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Unblocks the 10–100 GiB B1/B2 scale run (the RFC 0006 §8 canonical band the thesis targets speak to). Two bench-crate changes, both leaving every recorded §9 number comparable:
1. Streaming corpus path for the query-store builds
corpus::loadmaterializes the whole corpus as aVec<OtlpLogRecord>(~2–4× raw bytes), which caps the loadable corpus well below the scale targets on the §1 baseline hardware (32 GiB). Mining and Parquet flush were already per-record — the eager load was the only blocker.corpus::stream(dir, severity): a lazy per-line iterator over the same walker (collect_filesis now the single traversal both paths consume — ordering, cycle guard, and §3.4.1 byte accounting cannot drift), yielding the same records in the same order with the same timestamps. Equivalence is pinned bystream_replays_load_record_for_record.harness::run_streamingis the real loop;harness::run(&CorpusLoad)delegates (the gate paths hold the corpus anyway; the transient per-record clone is off every measured quantity).build_store(b1/b2) now streams; peak memory is flat at any corpus size. Empty-corpus rejection keepscorpus::load's exact message.--gates(A1/C1/C2) still uses the eager path — untouched behavior.2. Opt-in log4j severity extraction for plain-text corpora
The §3.3 loader pins every plain-text line to
INFO, so B1 skips plain-text corpora outright (distinct_severities == 1) — meaning B1 has no corpus in the ≥10 GiB class (the OTLP captures are ~1 GB).OURIOS_CORPUS_SEVERITY=log4j(never the default; unknown values are a hard error) extracts the first standaloneTRACE/DEBUG/INFO/WARN/WARNING/ERROR/FATALtoken — what a file-tailing agent would emit asseverity_text— giving LogHub HDFS-family corpora a real error band for the B1 predicate. §3.3's pinned-INFO doc-comment explicitly anticipated this divergence ("a documented baseline to diverge from"); the default stays that baseline, so A1/C1/C2 template keying and all recorded numbers are unchanged.build_query_store/build_b1_storegain theTxtSeverityparameter (bench-lib API); both benches resolve it from the env.Tests
stream_replays_load_record_for_record— record-for-record stream≡load equivalence incl. file/byte accounting.log4j_severity_extraction— token mapping, no-token INFO fallback, substring non-match,Fixednever extracts.b1_store_over_log4j_text_gets_error_band_selectivity— the B1 predicate materializes under the opt-in mode; the existingFixedselectivity-skip test is unchanged.Invariants / hazards
Fixeddefault only).🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes