Sd merge hard ceiling#4
Merged
Merged
Conversation
for_each_live_posting_matches_positions_all ran on the KB fixture segment "_2", which has no deletions, so a dropped or inverted delete filter would not have been caught. Add a test on the multiseg fixture's segment "_1" (real .del, local doc 1 genuinely deleted): it fetches title:backup's postings via the delete-agnostic read_freqs, confirms the deleted doc is present there but absent from for_each_live_posting's output, and asserts the two counts actually differ so the assertion can't be vacuous. Verified by temporarily inverting the filter in for_each_live_posting (dropping the `!`) — both this test and the existing for_each_live_posting_matches_positions_all failed as expected — then reverting.
Lets the merge write a term's postings one doc at a time instead of collecting them into a Vec first. Kept add_term as an independent oracle (does not call the new methods) so the byte-identity test comparing the two paths stays meaningful; only the low-level .tis/.tii tail encoding is shared via a new dump_tis_entry helper.
…erm) Rewires phase 3 of merge_segments_streaming to use the Task 2 begin_term/add_posting/end_term API on top of the Task 1 for_each_live_posting lazy decoder, instead of gathering the whole term's positions into a TermPostings before writing it. Contributing segments are walked in ascending index, and for_each_live_posting yields ascending local docs within each, so postings arrive in ascending new-id order with no per-term gather/sort needed. Adds the streaming_matches_batch_with_a_term_in_every_doc differential test (a term present in every doc across segments) as the hot-term regression guard, and a `hot` scenario in optimize_bench.rs to empirically measure the per-term heap spike.
merge_segments_streaming's per-doc/per-posting streaming (3a71de4) writes through raw File handles, so add_posting/add_doc issue one unbuffered write_all syscall per posting/doc — for a term with doc_freq N that's N syscalls instead of one, regressing optimize_ms on hot-term merges. Wrap the temp .fdt/.frq/.prx File handles in a fixed-size BufWriter (8 KiB, independent of doc_freq) before handing them to StoredStreamWriter / TermDictStreamWriter. The .tis/.tii sinks stay untouched (in-RAM Cursors, need Seek for the term-count back-patch, written per-term not per-posting). finish() on both writers already calls flush() on every sink generically, so the buffered bytes are guaranteed on disk before write_cfs_streaming reads the temp files back for CFS assembly.
add_posting now debug-asserts that doc ids arrive strictly ascending within a term (the sort that used to guarantee this was removed in the merge rewrite, so the underflow in doc_delta is now reachable from a caller bug rather than provably impossible). end_term resets cur_doc_freq to 0 before writing its entry, so a stray second end_term call without an intervening begin_term is a no-op instead of emitting a duplicate .tis/.tii entry. Both are internal-invariant hardening with no effect on correct usage; byte output is unchanged (stream_within_term_matches_add_term_byte_for_byte and all merge streaming_matches_batch_* tests stay green).
Contributor
Code Metrics Report
Details | | main (6c97324) | #4 (bf2c82b) | +/- |
|---------------------|----------------|--------------|-------|
+ | Coverage | 96.8% | 96.9% | +0.0% |
| Files | 36 | 36 | 0 |
| Lines | 5274 | 5518 | +244 |
+ | Covered | 5110 | 5347 | +237 |
- | Test Execution Time | 4s | 5s | +1s |Code coverage of files in pull request scope (95.8% → 96.0%)
Reported by octocov |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.