invertedstore: land the segment-based index component (standalone, unwired) - #106
Merged
Conversation
…o prod Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
3-round reviewed; replaces pebble-backed core/invertedindex with a segment-based store (write-once sorted runs + tiered merge + segment-local term-id forward map). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
int64 docid, 4-byte BE tableId, invertedValue (addsByteLen+adds+dels), and the nKw-prefixed forward value (tombstone=nKw 0; fixes the spike's ordinal-0 aliasing). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
SSTable blocks (inline small / external large), ordinal-ordered term-dict region, 25-byte footer with both data+dict codec ids, scanPrefix, ord->string resolve. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…pl plan Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Atomic replace (tmp+fsync+rename+dir-fsync); missing MANIFEST -> fresh empty; no recovery watermark (recovery is indexer-driven). JSON, FormatVersion-led. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Open/Close, CreateTable/DeleteTable (sync via queue.RunFunc, persisted in MANIFEST); per-table head (latest action per (kw,docid) + in-memory docid dedup); spill at CapBytes writes an L0 segment via segWriter and installs a new MANIFEST. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ge, concurrency, differential Completes the self-contained store (design T3-T6, T8, T11): - P5 Store-level chunk-LRU + newest-wins forwardKeywords (ord->string resolve) - P6 Search (prefix newest-wins union) + GetDocs (exact, no prefix leak) - P7 Update/Batch (async; term-id full re-post + per-keyword tombstones; delete) - P8 tiered merger: per-(kw,docid) newest-wins reconciliation (fixes add->del->add), ord->ord remap + term-dict rebuild, covering-merge reclamation - P9 concurrency: atomic.Pointer snapshot, head RWMutex, refcount-deferred deletion - P12 differential vs invertedindex: identical hit sets, table isolation, recovery 75 tests pass, -race clean, go vet clean. Built via subagent-driven-development (impl + 3 cross-reviewers + fix per phase). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…/T10) #105 already gave invertedindex its own forward map + 3-arg Update and dropped documents/symbols doc-words. This wires the live path to core/invertedstore behind a new invertedindex.Indexer interface (both *invertedindex.Index via IndexerAdapter and *invertedstore.Store satisfy it; shared SearchResult): - documents.Store / symbols / engine / searcher / server retyped to invertedindex.Indexer - server constructs invertedstore.Open(<ver>/invertedstore, mpsc, Options{AutoMerge:true}) - StorageVersion 1.5 -> 1.6 + 1.5 cleanup + reindex-on-upgrade - indexer-driven crash recovery (invertedstore reconcile / ForwardDocids) - re-apply the two shared-mpsc deadlock fixes (documents.Delete, symbols.AddFunctions must not call the async Update/table-ops from inside a worker task) + no-deadlock guards Whole workspace green: go build/vet/test ./... pass in both modules. Built via subagent-driven-development (scout + impl + 3 cross-reviewers, 0 hard issues, gated). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…wed) + task breakdown bottomDeadFraction is 73% of build CPU on a cold linux build (full-decompression scan after every spill, structurally 0 on a clean build). Replace with O(#segments) metadata deadFraction: per-segment segMeta.Postings + per-table liveByTable (recomputed on Open, catalog-gated, incremental in applyBatch). Synchronous orphan reclaim on Open closes the DeleteTable-window-crash byte leak. Spec + task breakdown each cross-reviewed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…unt (deadFraction 'written') Count adds+dels at spill (head.go) and at the merge keep-path (merge.go, so a covering merge's dropped keys contribute 0 and the output's Postings == its live adds). Bump FormatVersion 1->2 (greenfield). Cross-checked by decoding the segment, not a constant. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…Ords; ForwardDocids wraps it The segment newest-wins forward resolution is factored out so the Open live-recompute reuses the exact same tombstone/newest-wins/per-table semantics (no parallel scan that could drift from Search), surfacing each live docid's ords (ForwardDocids discards them). Behavior of ForwardDocids unchanged — all 7 reconcile tests stay green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…inct, DeleteTable-aware) Maintained in-lock in applyBatch (delta = new distinct − old distinct, dedup(old) since the forward stores raw keywords); DeleteTable drops the partition in O(1). Plain arithmetic counter (missing key = 0, no CreateTable seed). Tests drive the real Update+sync path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ds (catalog-gated) Open rebuilds liveByTable from the segments' [F] records via the shared forEachLiveSegmentForward (distinct ords, newest-wins, only catalog tables) after publishSnapshotLocked — the segment-anchored authority for the live counter, so crash recovery cannot inflate it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ments) deadFraction The core swap: deadFraction = 1 - Σ(catalog liveByTable) / Σ(segMeta.Postings), clamped — a metadata sum, no decompression. Deletes the bottomDeadFraction streaming scan that cost 73% of build CPU and was structurally 0 on a clean build. Running sum catalog-gated to match the Open recompute. coveringMergeHook counts covering merges (both triggered + forced paths). Tests: cold-build-never-fires regression guard, trigger-fires-on-deletes, covering-preserves-live, tiered/spill invariance, + the §5 live-written≤headCap invariant. (Spec §8.1 0.33→0.667 fix.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…leteTable-window crash) A crash after DeleteTable's MANIFEST write but before its scheduled covering merge installs leaves the dropped table's segments on disk. Open now detects a non-empty segment covering a non-catalog table and runs ONE covering merge via q.RunFunc — AutoMerge-INDEPENDENT (the default is off, where triggerMerge no-ops and the bytes would leak). Catalog-gated recompute already keeps it out of liveByTable. Empty segments skipped (MinTable=0 would false-positive). Tested AutoMerge-off. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…unt) §8.5(a) head-only loss + over-replay, §8.5(b) partially-durable + over-replay: live equals the true distinct count (durable docs' re-Update nets Δ0 via segment forward read) — the round-2 BLOCKER guard. §8.7 add->del->add in ONE batch pins the in-batch old-selection path. Threshold 0.25 needs no recalibration (cold=0 never fires; 60% deletes fires; verified in trigger_test). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The 95s cold build is GC-churn + single-worker serialization (merge 34 + spill 28 + addPosting 19 + forward 8, all on one mpsc worker). A: merge COMPUTE off-worker, install on-worker (single mutator preserved — review killed the two-writer/four-MANIFEST-writer hazard). B: forward docid-range skip. C: per-op alloc churn (memory play, ~0-3s wall). D: keep zstd. E: backpressure by postings. Honest target A+B+C+E = pebble parity (~55-65s); F (spill-encode off-worker) needed to beat it. Numbers review-calibrated, re-measured per change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…→multi-review→breakdown→multi-review→TDD/workflow Codifies the SDD flow as the top working principle: no code edits (not even a prototype, spike, or 'just measure it') before a written spec is multi-agent-reviewed, task-broken-down, and that breakdown cross-reviewed. Per the user's repeated direction. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…+head-fix+A+B+C+E+G+F) 3-round multi-agent review. Key outcomes: (F0) inline term dict kills the writeTermDict re-read, free -9s zero-concurrency — missed by v1; (head-fix) lazy dels map -5-8s, addPosting floor is ~12-14s not 19; (A) merge COMPUTE off-worker (single mutator preserved), add input refcounts; (F) spill encode off-worker HARDENED against the silent-corruption BLOCKER (forwardKeywords must consult the spilling tier — worker's own diff), atomic detach/install lock sections, NextSegId at detach, copy-under-lock head lifetime, spilling docid-skip so F doesn't undo B, maxInflightSpills bound; (G) Open sweeps orphan seg files (the 'GC'd on Open' comment was false). Goal: best achievable (~25-32s), measured per change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Bite-sized TDD tasks implementing spec v4, sequenced F0 -> head-fix -> B -> A -> C/E -> G -> F (free single-threaded wins first, F last + hardened). Each item independently measured on real ext4 + committed. Flags a deadlock in the spec's F bound (worker-blocks-detach vs worker-side install) and encodes the deadlock-safe synchronous-fallback correction for the cross-review. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…-review Resolves all BLOCKERs/MAJORs from the multi-agent cross-review: - define searchDocidsForTest (was used by the B1 gate but never existed) - F0: genuine red via a block-reread counter (was a fake/half-applied red) - F dispatchSpill: reserve slot before detach, no worker-side channel send (the prior fix reintroduced the deadlock on spillCh) + spillSem/spillWG - idxbench commands: add required -tokens/-data flags - sequence E before A/F (queue-saturation vs RunFunc installs) - covering-hook fires post-install (counts completed merges); liveTables staleness window documented + test required - Task 4: new off-worker race test + waitMergeIdle convergence test - Task 3: name the onForwardRead tests (stay green, do not relax) - Task 7A: concrete Search tier snippet; Task 7D crash-stub teardown - R1 resolutions section records every finding + fix Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…/major A fix is unverified until a FRESH review round confirms it (a deadlock fix routinely reintroduces a deadlock elsewhere). Iterate review rounds until a full round returns zero Blocking/Major; applied-but-not-re-reviewed is not done. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
R2 (3 reviewers) confirmed R1 BLOCKER fixes compile/correct but found the deadlock fix migrated the cycle into CloseAndWait, plus new gaps: - CloseAndWait deadlock: spillWG.Wait() must run off-worker (BLOCKER) - off-worker install-failure stranded the spilling entry forever -> retry + hold-slot-on-giveup (BLOCKER) - installSpill publish-then-remove ordering pinned (MAJOR) - delete maybeMerge/maybeCoveringMerge (dead after A -> cov-gate break) (MAJOR) - C.1 fast-path-taken hook+test; search-regression/disk measurement step (MAJOR) - forwardKeywords recursive-RLock caution; 7C onSpillingProbe (was a non-existent hook); concrete ForwardDocids tier code; drop dead accessor Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
R3 (2 reviewers) confirmed all R2 concurrency fixes correct; fixed 2 MAJORs in the R2 deltas: - C.1 applyOneOp signature said (over bool, err error) but call sites need error-only; 'lines 122-174 verbatim' wrongly included inBatch/seen loop bookkeeping -> corrected to applyOneOp(...) error, bookkeeping stays in loop - give-up durability claim was false (CloseAndWait flushes s.head not s.spilling) -> reclassified as crash-equivalent volatile loss - MaxInflightSpills/MaxInstallRetries concrete withDefaults (zero retries would no-op the loop -> instant strand); installBackoff const + time import pinned Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
R4 (2 independent reviewers) both verdict 'zero Blocking/Major, ready to implement'. The review loop converged R1->R2->R3->R4. Breakdown satisfies AGENTS.md Principle 0 stage 4 (cross-review until clean); ready for stage 5 (TDD implementation), order F0->head-fix->B->E->A->C->G->F. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…re next Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…dit code The coordinator must NOT hand-edit product code in the main loop — every code edit happens inside a Workflow subagent. Per item: red->green->gates, then multi-agent review loop until zero blocker/major, then commit, then next item. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…press guard (R5) The F0 workflow's implementation surfaced a defect the 4 breakdown-review rounds missed: the finishDictReread counter (fired inside writeTermDict) is a tautology once writeTermDict is deleted (no call site -> 0 by construction), and a byte-identical oracle passes against BOTH old and new code. Replaced with the genuine, PERSISTENT discriminator: finish() must decompress ZERO data blocks (old writeTermDict decompresses every [I] block; inline + openSegment decompress none) via an onDecompress hook on codec.decompress. Lesson: TDD-in-practice caught what static review could not. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…give-up wording) Breakdown review verdict: READY to implement. Two MINOR doc-precision nits folded. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…me id (F v5) Move the residual spill encode off the mpsc worker. One in-flight spill; the seg id is assigned at INSTALL (encode writes seg-tmp-<n>.dat, install does NextSegId++ + rename) so the parked spill always installs with the highest id = newest -> no spill-vs-spill or merge-vs-spill inversion, no merge deferral. A worker-controlled blockProducer gate (Cond.L==&s.mu, producer for-loops before AddFunc) bounds the live head only on over-cap+spillInFlight; E unchanged. installSpill publishes-before-remove and re-dispatches an over-cap head on ANY table. CloseAndWait drains the encode off the worker (caller goroutine, install-first). G sweeps seg-tmp-*. B1 silent-corruption gated at zero concurrency. idxbench measurement deferred (needs the lx.gob corpus). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…e map reuse Measured on lx (94.5k docs, post-F): build 45s (beats pebble 64s), disk 238 MiB (2.7x < pebble), but build peak RSS ~1 GB (pebble 610) from 30 GB alloc churn -> ~25% CPU in GC. mergeSegments is 44% alloc / 31% CPU; its per-keyword adds/dels maps (merge.go:275-276) alone are 2.1 GB flat -> C.4: hoist+clear+reuse the two maps (consumed per key, safe). C.2 decompress scratch (1.95 GB), C.3 segWriter encode scratch (encodeDocs/appendUvarint/flushDictChunk/encodeForward, ~6 GB). RSS/GC wins (merge is off-worker), not build-wall. Head maps out of scope. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…egrity test (review) Aliasing review: C.4 (merge map reuse) + C.3 (encode value scratch) SAFE. C.2 (per-cursor decompress reuse) UNSAFE naively — addEntry retains the cursor key via blkFirst->firstKey->finish UNCOPIED, so reusing the block corrupts the persisted block-index first-key, which the differential hits-test MISSES (a too-early sort.Search start still finds the key). Fix: copy first-key at capture (segment.go:119) + a dedicated block-index-integrity test. C.4 must clear() both maps unconditionally incl. the dropped-key path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Profile (lx, post-F): 30 GB build alloc churn -> ~25% CPU in GC, ~1 GB peak RSS. mergeSegments is 44% alloc. C.4: hoist the per-keyword adds/dels maps out of the merge loop, clear()+reuse them each key (2.1 GB saved; cleared unconditionally incl. the dropped-key path). C.3: reuse a per-segWriter encode-value scratch for encodeDocs/encodeForward (addEntry copies immediately). C.2: per-cursor decompress scratch reuse + copy blkFirst at capture (segment.go) so a reused block can't corrupt the persisted block-index first-key. RSS/GC win (merge is off-worker), hits identical. Block-index-integrity test guards C.2 (the differential misses it). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…dered ops slice)
Measured (idxbench -peakheap): build peak LIVE heap ~467 MB (->~1 GB RSS) is
HEAD-dominated — addPosting 188 MB (66%) is the per-keyword map[int64]struct{},
~48-96 B overhead each even for a 1-doc keyword. THAT is why store needs ~1 GB
build RSS vs pebble's 610 (compact memtable); GOMEMLIMIT only masks it; C.2-4
(churn) didn't move it because peak RSS = the live head. The map's dedup is
redundant (encode appendDeltaDocs already sort+dedups); only the cross add-vs-del
latest-wins is real. Change: postingDelta{ops []int64} (docid<<1|isAdd, append);
resolveOps (stable-sort, last-per-docid) at spill + Search, non-mutating
(copy-before-sort: M2 + concurrent Search). ~5-6x less head memory; on-disk
format byte-identical. Gated by differential hits-identical + resolveOps unit test.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…-docid contract (review) Review confirmed resolveOps provably equivalent to the map (empirically tested every sequence) + all readers/format/Postings preserved. Fixed 2 MAJORs: (1) packing precondition 0<=docid<2^62 (idtable monotonic counter) + assert + struct/parallel fallback; (2) the sort MUST be sort.SliceStable keyed on docid ONLY (v>>1) — sorting the full packed int64 is WRONG (isAdd low bit makes an add always last -> add->del mis-resolves). Plus: resolveOps fresh scratch per call; head_lazy_dels_test.go (reads pd.adds/dels as maps) must be rewritten; drop the posting() +16 estimate. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…slice) Thin TDD wrapper over the 2-round-reviewed spec §5b: resolveOps unit test (incl. the add->del discriminator the wrong sort fails + a no-mutate test), behavior- preservation via the differential hits-identical, RSS measurement via idxbench -peakheap. 7A-7E + acceptance follow. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… 4 setToSlice sites Breakdown review: NEEDS FIX (mechanical). Use the existing eqInt64s (not the undefined eqInt64 / name-trap) + sort.Slice + import sort; spell out all FOUR setToSlice->resolveOps sites (Search/GetDocs x live+spilling) + DELETE setToSlice; refresh stale adds/dels comments. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…cking unsatisfiable
The H IMPLEMENTATION surfaced (3rd time TDD caught a spec defect static review
missed) that the store's docid is the FULL int64 range — TestDifferential_Int64-
DocidFullRange feeds 1<<62/MaxInt64 — so the v1/v2 docid<<1|isAdd packing's
<2^62 precondition is UNSATISFIABLE. Revised per Principle 0: primary is the
parallel postingDelta{docids []int64; isAdd []uint64} bitset (full range, same
~8.1 B/op, ~6-8x < the map). Measured: peak inuse 156 MB, addPosting map hog
GONE. Also: H's +8/op accounting shifts spill cadence -> surfaces a latent F B1
test cleanup-ordering -race (fix the test, a 5th file). resolveOps: stable-sort
by docid, last-per-docid (non-stable fails add->del).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… is the remaining work Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…set (H)
Replace postingDelta{adds,dels map[int64]struct{}} (66% of build peak live heap: ~48-96 B/keyword map overhead even for a 1-doc keyword) with postingDelta{docids []int64; isAdd []uint64} — a parallel ordered op log, O(1) append, ~8.1 B/op. resolveOps (stable-sort by docid, last-op-per-docid wins, non-mutating fresh scratch) reproduces the map latest-wins at spill + Search/GetDocs. On-disk format byte-identical (differential hits-identical 2,414,505, incl. the full-int64-range docid test the rejected docid<<1 packing could not represent). Measured: peak inuse 156 MB (vs ~284), the addPosting map hog GONE (head ~26 MB). Also fixes a latent F B1 test cleanup-ordering -race (drain spills before nil-ing the encodeSpillBlock hook) that H's +8/op spill-cadence shift surfaced.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… key C.4 (581383a) hoisted mergeSegments' per-keyword adds/dels reconciliation maps out of the loop and clear()+reused them. Go's clear() empties a map but RETAINS its bucket capacity, so a map once grown by a high-cardinality keyword stays huge and every later key's `for d := range adds` drains O(retained capacity) instead of O(key size) — turning the merge into O(numKeys × peak). On the lx corpus (94559 docs, 2.41M hits) this regressed the full build 6.5×: 46s → 277s. Root-caused via CPU profile (63% of build CPU in maps.Iter.Next + ctrlGroup.matchFull under mergeSegments), a bisection (a52da8d 46s vs current 277s landing on 581383a), and env ruled out: a tmpfs build was identically slow (fsync not the cause) and a 1/5-corpus build was CPU-bound and healthy. Fix = revert ONLY C.4: declare adds/dels FRESH per key again (the a52da8d structure). Keep C.2 (segWriter blkFirst copy) and C.3 (encodeScratch reuse) — not implicated. Output is byte-identical (a fresh empty map == a clear()ed one); the full differential suite is unchanged. Measured on lx (/workspace xfs, same session): build 277s -> 42.6s (6.5x; regression removed) peak RSS 484 -> 393 MiB (-19%; the buggy reused map stayed resident) disk 233.6 -> 234.9 MiB (same) hits 2414505 (identical) search 8840 -> 9029 us/q (noise; reader path untouched) Adds merge_highcardinality_test.go: a high-cardinality keyword flanked by many tiny keywords, through a tiered AND a covering merge, asserting per-keyword adds/dels via segInvRecords. Coverage for the map-drain shape; NOT a perf guard (the bug is perf-only — see the spec). invertedstore go-cov 94.6%; -race green. SDD artifacts: docs/design/invertedstore-merge-mapreuse-regression-fix-{spec, tasks}.md (spec → 2-round multi-agent review → tasks → review → workflow TDD, all converged to zero Blocker/Major). Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Claude <noreply@anthropic.com> Co-Authored-By: Happy <yesreply@happy.engineering>
…eep invertedstore unwired) invertedstore is not production-ready yet (scale gaps: hot-keyword merge/search OOM, no max-seg cap, single-JSON MANIFEST). Revert the LIVE server backend from invertedstore to the pebble-backed invertedindex so this branch can land its invertedstore component + groundwork without flipping production. Variant 1 — keep the abstraction: - server constructs invertedindex.New wrapped in NewIndexerAdapter, returning the invertedindex.Indexer seam (not *invertedstore.Store, not *invertedindex.Index); restore the pebble indexdb (storage.Open .../index) + defer Close. - StorageVersion 1.6 -> 1.5; drop "1.5" from the cleanup list (it would have wiped the now-live 1.5 store). - KEEP: the invertedindex.Indexer/Batch/SearchResult interface + the pebble adapter, the shared-mpsc deadlock fixes (documents/symbols), and the entire core/invertedstore component (now unwired — exercised only by its own tests + the idxbench harness); documents.New/symbols.Init stay typed to the Indexer seam. - Tests that used invertedstore as the live/test backend switched to invertedindex.New + NewIndexerAdapter (+ flush-aware polling for pebble's flushed-read visibility); deadlock guards and invertedstore component tests kept. Both modules green: go build/vet/test ./... + -race on invertedstore/invertedindex. Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Claude <noreply@anthropic.com> Co-Authored-By: Happy <yesreply@happy.engineering>
…n roadmap - core/invertedstore/README.md: durable status + measured data (build 42.6s / 234.9 MiB / 393 MiB RSS on lx; store-vs-pebble A/B; spill+merge cadence ~23 tiered merges; Fanout/write-amp sweep) + size-tiered strategy vs Lucene/RocksDB + corrected ground-truth facts + the C.4 clear()-retains-capacity footgun + the known scale gaps (hot-keyword OOM, no max-seg cap, deletion trade). - docs/design/invertedstore-luceneization-exploration.md: the pre-spec design exploration (forward/inverted split, per-doc delete, max-seg cap, bloom-vs-dict, + the merge-memory/scale reframe), adversarially reviewed. - docs/design/invertedstore-luceneization-implementation-plan.md: the sequenced roadmap (D0 harness → streaming merge → streaming search → max-seg cap → one StorageVersion reindex) + decision resolutions + holistic search impact. Docs only; no code change. invertedstore stays the unwired go-forward backend. Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Claude <noreply@anthropic.com> Co-Authored-By: Happy <yesreply@happy.engineering>
Revert ALL integration code outside core/invertedstore back to main, keeping core/invertedstore as a standalone component: - restore the 25 modified files (collection/documents/engine/symbols/ server/storage) to main's version - delete the 6 files added on the branch (deadlock tests, e2e test, invertedindex adapter.go/indexer.go) - decouple core/invertedstore from the now-removed invertedindex.Indexer/ Batch interfaces: NewBatch/Update return *Batch, drop the compile assertions, fix doc comments; keep SearchResult = invertedindex.SearchResult Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
# Conflicts: # AGENTS.md
… seam in invertedindex The integration seam was reverted out (core/invertedindex stays pristine — it is a published package other projects depend on). Fix the stale Status line that claimed invertedstore "satisfies the invertedindex.Indexer seam"; it is now a deliberately unwired standalone component, integration deferred until it is stable at scale. Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Claude <noreply@anthropic.com> Co-Authored-By: Happy <yesreply@happy.engineering>
codetrek
changed the title
…ction coverage ≥80%
Two CI failures after landing invertedstore standalone:
1. App "Check Formatting": 6 invertedstore test files were not gofmt'd
(apply_fastpath/crash_recovery/dictcache/merge_offworker/search/spilling_read).
gofmt -w (go 1.24.2, CI's version).
2. Core "Coverage gate": go-cov's strict per-function ≥80% gate failed on 9
invertedstore functions — coverage the removed integration test
(engine/invertedstore_e2e_test.go) used to provide. A standalone component must
cover its own functions:
- DELETE the dead `blockDiskSize` (0%, no call sites, unused).
- Add focused invertedstore tests for the error/lifecycle branches:
readManifest (malformed/EISDIR) 72.7→100, writeManifest (marshal error via a
test-only nil-default `marshalManifestErr` hook) 75→100, writeManifestBytes
(write/rename error) 66.7→81, newSegWriter 66.7→100, mustReadAt (short read)
50→100, sweepOrphanSegments (seg-tmp sweep) 78.9→84.2, newChunkLRU 66.7→100,
fileSize (stat error) 75→100.
Verified with go 1.24.2 (CI's version): go-cov reports ZERO invertedstore functions
<80% (pkg 95.7%); gofmt clean; go test + -race ./invertedstore/ green; root build
clean. Scope unchanged — still only core/invertedstore + invertedstore docs + AGENTS.md
differ from main.
Generated with [Claude Code](https://claude.ai/code)
via [Happy](https://happy.engineering)
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Happy <yesreply@happy.engineering>
…while-open Core Windows CI ran the invertedstore suite for the first time (prior runs were matrix-canceled by the ubuntu coverage-gate failure) and the whole suite failed on two POSIX-only file semantics: 1. writeManifestBytes fsync'd the directory (os.Open(dir)+d.Sync()) to make the MANIFEST rename durable — runs on EVERY spill + merge-install. Windows cannot FlushFileBuffers a directory handle, so it errored on every MANIFEST write. Fix: extract syncDir(dir) — POSIX open+Sync unchanged (byte-identical durability), no-op on Windows (NTFS journals metadata; a dir fsync is neither needed nor supported for rename durability — same as bolt/badger/pebble). 2. installSpill renamed the off-worker temp segment to its final name while the segment's read fd was still open. Go's os.Open on Windows lacks FILE_SHARE_DELETE, so MoveFileEx refuses to rename an open source (ERROR_SHARING_VIOLATION). Fix: renameSegmentFile reseats the fd across the rename (close → os.Rename → reopen), applied to the install rename + both rollback renames. Safe: installSpill runs on the worker before publishSnapshotLocked, so no reader holds the fd yet. POSIX behavior/durability unchanged. Verified on Linux (go 1.24.2): go build/vet/ test/-race ./invertedstore/ green; gofmt clean; go-cov invertedstore 95.7%, no function <80%. The Windows path itself can only be confirmed on Windows CI. Known follow-ups (Minor, pre-existing pattern): renameSegmentFile/openSegment swallow a reopen error (rare fd-exhaustion → nil fd → read panic); drainMerge has timing- dependent (flaky) coverage. Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Claude <noreply@anthropic.com> Co-Authored-By: Happy <yesreply@happy.engineering>
…d in a3a8543) a3a8543 added renameSegmentFile (Windows rename-while-open fix) but the staging missed this test file, so CI saw renameSegmentFile at 62.5% (only the happy-path coverage from the off-worker spill tests) and the Linux coverage gate failed. This file covers BOTH the success reseat and the rename-FAILURE reseat-at-source branch, taking renameSegmentFile ≥80%. Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Claude <noreply@anthropic.com> Co-Authored-By: Happy <yesreply@happy.engineering>
…nal branch
drainMerge's <-mergeSignal branch is timing-FLAKY through the normal stop path
(mergeLoop's select{<-mergeStop; <-mergeSignal} picks randomly, so a pending
trigger is only ~50% observed by drainMerge). That flakiness intermittently drops
drainMerge to 50% and fails the Linux per-function coverage gate (it just did on CI).
Cover it deterministically instead: stop the background loop, inject a pending
trigger, call drainMerge directly, and assert it collapses the >=Fanout L0 segments
(a real merge, not a touch). drainMerge → reliably 100%.
Generated with [Claude Code](https://claude.ai/code)
via [Happy](https://happy.engineering)
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Happy <yesreply@happy.engineering>
…s diagnostic dump
The "Test (macOS/Windows)" step runs under GitHub's default `bash -e`. With
`set -o pipefail`, a failing `go test | tee vitest.log | grep` pipeline returns
non-zero, and `-e` aborts the script IMMEDIATELY — before `status=${PIPESTATUS[0]}`
and the `if fail: cat vitest.log` dump. So a macOS/Windows failure logged only the
grep'd `--- FAIL` summary, never the assertion detail (the full -v log). Add
`set +e` so the status is captured and the full log is dumped on failure, then
`exit "$status"` still fails the job.
(General CI fix; needed to debug the invertedstore Windows-portability failures on
this branch.)
Generated with [Claude Code](https://claude.ai/code)
via [Happy](https://happy.engineering)
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Happy <yesreply@happy.engineering>
…ows fd hygiene) The Core Windows CI job failed ~24 invertedstore tests with "TempDir RemoveAll cleanup: remove ...seg-*.dat: The process cannot access the file because it is being used by another process." Those tests open a *Store (each Open opens seg-*.dat read fds) and return without CloseAndWait, so the still-open fds block t.TempDir's RemoveAll on Windows. On POSIX this is hidden by unlink-open-fd semantics. CloseAndWait already closes every segment fd (retireKeepFile) — the fix is simply to call it before the test returns. - newForwardSkipStore / newBackpressureStore: register t.Cleanup(CloseAndWait) (covers the ApplyFastPath / SpillingTier / ForwardSkip / Backpressure tests). - single-store leakers: add `defer s.CloseAndWait()` right after Open. - reopen / crash-recovery tests: close the final reopened instance too. - spill_offworker leakers: t.Cleanup(CloseAndWait), ordered LIFO after the body's hook-clear cleanup so the close-time flush never blocks on a parked encode. - TestSpillF_CrashLosesDetachedHeadNoOrphan intrinsically abandons a live store to model a mid-spill process death, so it cannot CloseAndWait — skip it on Windows (matching internal/core/symbols/seal_idtable_durability_test.go); the property is still exercised on Linux/macOS. Test-only; no product code changed. Verified with go1.24.2: full + -short invertedstore suites green, -race green, gofmt clean, go-cov gate 95.8% (no function below 80%). Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Claude <noreply@anthropic.com> Co-Authored-By: Happy <yesreply@happy.engineering>
…e the go-cov gate)
sortSegMetasById (merge.go) is a pure insertion sort exercised only by the
timing-dependent AutoMerge/merge paths, so its per-function coverage flaked:
the inner swap line runs only when the merge feeds it out-of-order segMetas.
On this branch's CI run it dropped to 66.7% and tripped the go-cov <80%
CRITICAL gate (it had passed the prior runs purely by merge-ordering luck).
Pin it with a direct, deterministic white-box unit test: an unsorted
>=2-element input ({3,1,2}->{1,2,3}) forces the swap and the comparison-false
early stop every run, plus reverse/already-sorted/single/empty cases assert
the exact resulting Id order (reflect.DeepEqual — a real invariant, not
coverage theater). No Store/queue/t.TempDir, so it is fast and leak-free.
Verified with go1.24.2: the new test is green, gofmt clean, and the go-cov
gate now reports zero CRITICAL functions (sortSegMetasById >=85%,
core/invertedstore 95.8%).
Generated with [Claude Code](https://claude.ai/code)
via [Happy](https://happy.engineering)
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Happy <yesreply@happy.engineering>
The docs/design/invertedstore-* files (spec / task-breakdown / plan / exploration / implementation-plan / per-regression fix notes — 11 files, ~6.6k lines) are one-off process artifacts, not as-built architecture docs, so they do not belong in the tree. The durable conclusions and measurements they produced already live in core/invertedstore/README.md (status, measurements, merge strategy vs Lucene/RocksDB, key findings, the C.4 regression, and the known scale gaps / roadmap). Removing them leaves PR #106 as core/invertedstore/** (code + README) + AGENTS.md, nothing else. Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Claude <noreply@anthropic.com> Co-Authored-By: Happy <yesreply@happy.engineering>
oc-engteam
approved these changes
Jul 7, 2026
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
2 participants
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.
Lands the segment-based invertedstore (a future, LSM-like alternative to the pebble-backed
invertedindex) as a standalone, deliberately unwired component. Production is unchanged — it keeps running the pebbleinvertedindex, exactly asmain.Why unwired (not integrated)
invertedstore is not yet proven to replace
invertedindexat scale (hot-keyword merge/search OOM, no max-seg cap, single-JSON MANIFEST — seecore/invertedstore/README.md"Known scale gaps"). There is no reason to modify a mature, externally-depended-upon package (core/invertedindexis published) for an unproven replacement. So this branch adds invertedstore as a component and reverts every integration change outside it; integration is deferred until invertedstore is stable.Scope (verified:
git diff main HEADtouches ONLY these)core/invertedstore/**— the full component + tests + the ingestion-perf work (F v5 off-worker spill, C.2–4, H compact head, the C.4clear()-retains-capacity footgun fix that 6.5×'d the build) + crash-recovery (reconcile.go). Decoupled from anyinvertedindexinterface (own*Batch;SearchResultreuses the existinginvertedindex.SearchResultstruct).core/invertedstore/README.md+docs/design/invertedstore-*.md— measurements, findings, roadmap.AGENTS.md— the SDD working-principles update.Everything else is byte-identical to
main:core/invertedindexuntouched (noIndexer, no adapter);documents/engine/symbols/serverkeep concrete pebble*invertedindex.Index.Verification
Both modules green:
go build/vet/test ./...+-raceoninvertedstore/invertedindex. (Only the untrackedcore/cmd/idxbenchdev harness fails to build — intentionally out of scope.)Squash-merge (the branch carries a
Merge maincommit to converge out-of-scope files to main's HEAD; the squashed diff is the scope above).Generated with Claude Code