Skip to content

implementation-fn-note.md

Latest commit

 

History

History
117 lines (101 loc) · 29.9 KB

implementation-fn-note.md

File metadata and controls

117 lines (101 loc) · 29.9 KB
Raw

Finalization Sampling — Implementation Notes (running log)

Plan: .omc/plans/trace-finalize-sampling.md (Rev 7). Branch: trace-pipeline-finalize-event. This file records decisions NOT in the spec, changes I had to make, tradeoffs, and anything the user should know. Newest entries at the bottom of each section.

Ground-truth line numbers (verified against current tree, 2026-07-05)

  • banyand/internal/storage/version.go: SegmentMetadata struct :52; segmentMeta :74; readSegmentMeta :79; currentVersion = "1.5.0" :33. NOTE: there is NO create-literal in version.go; the create write lives in segment.go (to be located).
  • banyand/trace/part_metadata.go: partMetadata struct :37; reset() :47; mustReadMetadata :79; mustWriteMetadata :98 (uses fileSystem.WriteAtomic — already atomic). fillFromSyncContext :57 (chunked-sync path — must also carry finalizeGen? see decisions). ParsePartMetadata :262.
  • banyand/trace/pipeline_registry.go: localMergeGraceRegistry + setMergeGraceForGroup/lookupMergeGrace :28-50 — mirror for finalize. lookupSamplers :140.
  • banyand/trace/merger.go: mergePartsThenSendIntroduction :318; sampler chain build :326-342; isMergeHot :694; sidx keepFn :394-404; getAllSidx loop :406; mergeParts :549; pm min/max ts fill + pm.mustWriteMetadata :601-603; re-open :607; mergeBlocks :811; zero-value var pm partMetadata :980.
  • banyand/trace/tstable.go: tsTable struct :51-74 (has group, option, pm protector.Memory, curPartID, loopCloser, shardID, root, fileSystem; does NOT cache segment EndTime/generation). startLoop :125 (run.NewCloser(1 + 3), spawns introducer+flusher+merger). startLoopWithConditionalMerge :150 (run.NewCloser(1 + 3), spawns introducer-sync+flusher+syncer — NO mergeLoop).

Open implementation questions found while grounding (to resolve during impl)

  • Q-A (conditional-merge path has no merger): startLoopWithConditionalMerge spawns introducer-sync/flusher/syncer but NO mergeLoop. The plan (M2) says add the finalizer to BOTH start paths. Need to confirm whether the sync-mode data node retains cooled segments locally long enough to finalize, or whether parts are synced away (making finalize a no-op there). Decision pending — will verify against the sync path before wiring the finalizer into it. Safe default: still spawn it (it just finds nothing to do if parts are gone), but must not break sync-mode shutdown counts.
  • Q-B (finalizeGen stamp home): the plan says thread finalizeGenOverride through BOTH mergeParts and mergeBlocks. But mergeBlocks has no access to the input partWrappers (only the block reader), so it cannot compute min(inputs). mergeParts DOES have parts []*partWrapper and already post-processes pm (sets Min/MaxTimestamp) BEFORE pm.mustWriteMetadata at :603. Cleaner: thread the override into mergeParts only, compute min-propagation from parts there, set pm.FinalizeGen before :603. mergeBlocks stays untouched. This still satisfies the plan's load-bearing invariant ("stamp on disk before the write, survives restart"). Recording as a deliberate simplification.
  • Q-C (fillFromSyncContext / chunked sync): partMetadata.fillFromSyncContext populates a part's metadata from a sync context and does NOT set finalizeGen. A synced part would arrive with finalizeGen=0 (unstamped=selectable), which is correct-by-default. No change needed unless sync must preserve finalize state across nodes (out of scope — finalize is per-local-segment). Recording as intentionally unchanged.

MAJOR ARCHITECTURE DEVIATION (user-approved 2026-07-05) — per-shard state + trace-owned scanner

Why: The plan put the finalize generation in the per-SEGMENT metadata file (DD4a/M1) but the lane per-SHARD (tsTable, DD7). A segment has N shards (verified: segment[T,O].sLst []*shard[T], each shard.table is a tsTable rooted at <segment>/shard-N, created via TSTableCreator(..., s.TimeRange, ...) — shard.go:81). So N per-shard lanes would multi-write ONE segment metadata file, and idle-closed cooled segments (segments get idle-reclaimed) have NO running tsTable loop, so a per-tsTable-loop-only scanner would never finalize the common case. Resolution (user picked Option A):

  • Per-shard finalize state, persisted as a small atomic JSON file in the shard dir (tst.root), sole-written by the finalize worker. NOT in SegmentMetadata. => M1 (both segment metadata structs) is DROPPED; version.go/segment.go create-literal are UNTOUCHED. The segment's cooling is read from the in-memory Segment.GetTimeRange().End (already available), no persisted finalize field needed at the segment level.
  • Trace-owned periodic scanner using the PUBLIC tsdb.SelectSegments(coolRange, reopenClosed=true) (tsdb.go:277) + segment.Tables() to enumerate cooled segments' per-shard tsTables. reopenClosed=true REOPENS idle-closed cooled segments (the key fact that makes this cover the common case), holding an incRef for the finalize duration; DecRef after. => no storage/rotation.go hook, no storage→trace seam (avoids the layering coupling the user removed for the delete gate). => Phase 5 scanner moves from storage/rotation.go into banyand/trace.
  • Concurrency-1 finalize worker: the scanner processes one shard finalize task at a time (node-wide serialization), so the finalize compute never fans out. The tsTable's finalize round reuses the tsTable's existing introducer loop (a reopened segment has live loops) + mergeParts(finalizeGenOverride=&G). => M2 (4th run.Go loop in BOTH start paths) is DROPPED — no per-tsTable finalizer loop; the worker calls tst.runFinalizeRound(...) synchronously while the segment is incRef-held. tst.loopCloser.CloseNotify() still used for cooperative abort.
  • tsTable gains segEndUnixNano (captured from the timestamp.TimeRange ALREADY passed to newTSTable — currently ignored as _), plus cached finalizeGenCached atomic.Uint64 / segCooledCached atomic.Bool / unsampledBytes atomic.Int64 seeded from the per-shard finalize file at open. UNCHANGED from plan: per-part finalizeGen stamp + min-propagation via finalizeGenOverride in mergeParts before :603 (DD1.C1/DD6); force filter-merge reuse + keepFn core+sidx coupling; fail-open; threshold FLOOR/RATIO/cooldown/max_rounds (Phase 1, per-shard now); protector/disk gating; sidx rides hot merge path (OQ6). Best-effort/misses-accepted (P2), no delete gate. PRD updated: US-002/US-003/US-005 acceptance criteria re-scoped to per-shard state + trace-owned scanner; M1/M2 references dropped.

Phase 2 (US-002) — DONE 2026-07-05

  • Per-part finalizeGen uint64 json:"finalizeGen,omitempty" added to partMetadata (+ zeroed in reset()). Old-format parts load as 0 (verified).
  • New finalize_state.go: finalizeState{LastFinalizedAt, FinalizeGeneration, UnsampledBytes, FinalizeRounds, Terminal} + readFinalizeState/writeFinalizeState (atomic via fs.WriteAtomic temp+rename). Missing/corrupt file => zero (fail-open, never panics). Stored per-shard in tst.root as finalize.json.
  • SegmentMetadata/segmentMeta/segment create-literal UNTOUCHED (Rev 8). No version bump.
  • 4 tests pass.

Phase 3 (US-003) — DONE 2026-07-05

  • tsTable gained finalizeGenCached atomic.Uint64, unsampledBytes atomic.Int64, segEndUnixNano int64. Seeded in initTSTable from readFinalizeState; segEndUnixNano captured in newTSTable from the (previously-ignored _) timestamp.TimeRange.
  • accountUnsampledFlushed(flushed) helper (takes NO fs → zero metadata I/O by construction): if finalizeGenCached>0, add sum of flushed parts' UncompressedSpanSizeBytes to unsampledBytes. Called from introduceFlushed AND introduceFlushedForSync. No-op until first finalize (gen 0).
  • 2 tests pass (accounting + seed-from-disk).

CRITICAL SEMANTICS DECISION — finalize generation is "full-rewrite per round", NOT minimal-rewrite (plan DD1 inconsistency)

The plan's DD1 claimed THREE things that are mutually inconsistent: (a) monotonic per-round generation, (b) selection predicate finalizeGen < FinalizeGeneration, (c) "minimal rewrite volume — only not-yet-current-generation parts rewritten each round." With a monotonic gen and a < G predicate, previously-finalized outputs (stamped at gen k) are ALWAYS re-selected once stored advances past k, so true minimal-rewrite is unachievable with this scheme (worked through several variants; all either break the first-round bootstrap, break crash-safety, or re-select old outputs). Chosen (correct + crash-safe): a round with current stored gen G computes Gnext = G+1; selects all non-hot / not-in-flight parts with finalizeGen < Gnext (which, by the invariant that nothing is stamped above stored, is effectively ALL cooled parts of the shard); force filter-merges them through the sampler; stamps outputs finalizeGen = Gnext (via finalizeGenOverride); introduces; then persists stored = Gnext + resets the unsampled counter. So each round re-finalizes the full set of cooled parts (like the in-merge sampler, which also re-samples on every merge).

  • Why it's safe: re-sampling already-sampled survivors through a DETERMINISTIC sampler is idempotent (DD11 soft expectation); a probabilistic sampler drops a bounded extra amount, never unsafe. Rounds are hard-capped at max_finalize_rounds (8), so ≤8 full rewrites per segment lifetime.
  • Crash-safety (DD6 preserved): crash after introduce, before persisting stored (stored still G, outputs on disk stamped Gnext=G+1): on replay stored=G → next round Gnext=G+1 → predicate finalizeGen < G+1 excludes the Gnext-stamped outputs (G+1 < G+1 = false); the merged-away inputs are already gone from the snapshot → no double-sample. Matches DD6 crash-window-2 exactly.
  • DD1.C1 min-propagation (hot merge inside finalized segment) UNCHANGED and still needed: a hot merge with finalizeGenOverride=nil sets output = min(inputs), so merging two gen-Gnext parts stays gen-Gnext (not reset to 0) and a hot-merge never un-finalizes.
  • Deviation recorded: DD1's "minimal rewrite" bullet is relaxed to "full-rewrite-per-round, bounded + idempotent." If true minimal-rewrite is later required, it needs a more complex per-part epoch scheme (deferred).

Phase 4 (US-004) — DONE 2026-07-05

  • mergeParts gained finalizeGenOverride *uint64 (7th param); applied to pm.FinalizeGen (override or min-propagate) BEFORE pm.mustWriteMetadata (:603). Deviation from plan wording: the stamp is threaded through mergeParts ONLY (not mergeBlocks), because mergeBlocks has no access to the input partWrappers for min-propagation and mergeParts already post-processes pm before the on-disk write. Same invariant (on-disk before re-open).
  • mergePartsThenSendIntroduction gained an ov *mergeOverrides{filter, finalizeGen} param so the finalize round reuses the ENTIRE hot tail (sidx getAllSidx().Merge + keepFn coupling + introducer send). Hot callers (merger lane, flusher) pass nil. This is the OQ6 "core+sidx follow hot merge path" made literal — finalize issues the exact same sidx.Merge calls.
  • tst.mergeCh retained in BOTH start paths so the finalize worker (external goroutine) introduces through the shard's serialized introducer loop. Never acquires mergeMaxConcurrencyCh (that's in mergeLaneWorker, which finalize bypasses).
  • New finalizer.go runFinalizeRound(samplers, graceNs): protector gate → snapshot select (non-hot by finalize_grace, not-in-flight, finalizeGen < gNext) → incRef+in-flight pin → force filter-merge via mergePartsThenSendIntroduction(ov={filter,&gNext}) → persist per-shard state (gen=gNext, rounds++, lastFinalizedAt, counter=0) → refresh cache. Fail-open: merge error leaves segment intact, scanner retries.
  • Tests: 4 C1 cases (mergeParts propagation, on-disk asserts) + a running-tsTable round (drop trace2 → 2 remain, gen=1, state persisted). All pass. 3 existing test callers of mergeParts + 1 of mergePartsThenSendIntroduction updated for the new param.

Phase 5 (US-005) — DONE 2026-07-05

  • New finalize_scanner.go: sr.finalizeScanLoop(closer, interval) (single node-wide goroutine, concurrency-1) → runFinalizeScan iterates listFinalizeGroups() → scanGroup does loadTSDB + SelectSegments(coolRange, reopenClosed=true) + per-shard warrantsFinalize → runFinalizeRound. DecRefs segments.
  • warrantsFinalize: reads per-shard finalize.json (scanner path, off hot path); terminal/max-rounds (marks terminal) / cooldown / gen0-always / unsampledBytes >= max(FLOOR, RATIO*total).
  • reconcile now stores finalize config ONLY when finalizeEventEnabled (so merge-only groups aren't scanned); listFinalizeGroups() added; schemaRepo.finalizeGraceDefault added.
  • Wired into svc_standalone: finalizeCloser started in PreRun (when nativePipelineEnabled), CloseThenWait in GracefulStop.
  • Deviation: "at most one round per worker wakeup" (PRD literal) interpreted as concurrency-1 = SERIALIZED (one shard round at a time), processing all warranting shards per tick sequentially — otherwise 1 shard / 10min is uselessly slow. Recorded.
  • Coverage gap noted: full scan→round e2e (SelectSegments→Tables→round on a real TSDB) is NOT unit-tested (needs a full schemaRepo+TSDB); covered by warrantsFinalize + runFinalizeRound unit/integration tests + scan-gating test. Full e2e deferred to the integration/soak suite (plan E2E section). Data-node service (if separate from standalone) needs the same PreRun/GracefulStop wiring — only svc_standalone was wired (Q-A); flag if a distinct data svc exists.

Phase 6 (US-006) — DONE 2026-07-05

  • Finalize compute observable via REUSED merge metrics type="finalize"/lane="finalize" (total_merged/total_merge_latency/total_merged_parts). Deviation: did NOT add dedicated coverage-gauge/rounds/dropped/fail-open finalize counters — the existing incPipelineTraces* counters (evaluated/dropped/…) are DEFINED BUT UNWIRED (zero call sites, even for the hot path — a pre-existing gap), so adding a parallel finalize metric set would be inconsistent scope. The reused lane label satisfies "distinct finalize metrics." NO finalize_unsampled_deleted_total (Rev 7). Recorded as a follow-up: wire the pipeline drop counters (hot + finalize) uniformly.
  • Docs: docs/design/post-trace-pipeline.md gained an "Implementation: finalization sampling (v1)" section (best-effort/misses-accepted, per-shard/scanner model, all knobs). CHANGES.md feature entry added under 0.11.0.

Verification & review (2026-07-06)

  • Architect review: APPROVED (0 blockers), all 8 correctness claims verified with file:line evidence (crash-safety ordering, min-propagation, semaphore bypass, in-flight pin + mergeCh-close safety, O(1) counter, threshold termination, no-delete-gate, reopen-closed coverage).
    • It RESOLVED my fn-note Q-A worry: trace.NewService (which wires the finalize scanner) is used by BOTH pkg/cmdsetup/standalone.go AND pkg/cmdsetup/data.go, so the data-node role IS covered; the liaison correctly has no scanner (write-queue path). No separate wiring needed.
    • It confirmed the select-then-pin TOCTOU in runFinalizeRound is structurally identical to the existing hot dispatcher (dispatchAllMerges) and introduces no new race (the serialized introducer tolerates overlap) — left as-is intentionally.
  • Two MINOR findings fixed:
    • Counter lost-update: runFinalizeRound now snapshots unsampledBytes at round start and Add(-startBytes) on commit (persisting the remainder) instead of Store(0), so late writes arriving DURING a round are preserved for the next round.
    • Scanner→round e2e gap: added TestFinalizeScan_SelectsCooledSegmentAndFinalizes — builds a real TSDB (hourly segments, 30d TTL), writes into a 2h-cooled segment, drives the exact storage calls scanGroup makes (SelectSegments(reopenClosed=true) → End re-check → Tables() → runFinalizeRound), asserts drop+gen advance, DecRefs. (Gotcha: the TSDB Option needed decideTimeout set — 0 makes the chain time out → fail-open kept all traces.)
  • Composition coverage (added 2026-07-06, on user question): TestFinalizeAndMerge_Compose — one running tsTable with nativePipelineEnabled + a registered sampler, auto-merge disabled (maxFanOutSize=0) for determinism. A real hot merge (MERGE event) drops its targets via the in-merge filter; a never-merged part is left; a finalize round (FINALIZE event) drops that part's target and re-keeps the merge survivors (no double-drop); generation advances 0→1. Proves MERGE+FINALIZE compose on one shard with the shared sampler (DD11). Finalization solo was already covered by TestRunFinalizeRound_DropsAndStamps (round on a running tsTable) + TestFinalizeScan_SelectsCooledSegmentAndFinalizes (scanner→round on a real TSDB). Both "solo" and "solo+merge" now have integration coverage.
  • Deslop pass: no slop found — every finalize symbol is referenced (no dead code), no duplication (the mergePartsThenSendIntroduction+mergeOverrides reuse avoided ~100 lines of sidx/introduce dup), no needless wrappers, no boundary leaks. Comments document genuinely complex crash-safety/OQ6 invariants (CLAUDE.md "complex logic" exception).
  • Gates: make lint clean; go build ./... clean; full banyand/trace/... suite green.

Commit + UT/IT verification (2026-07-06)

  • Committed as 1c8ad66a "feat(trace): finalization sampling (PIPELINE_EVENT_FINALIZE)" on branch trace-pipeline-finalize-event (code + docs + CHANGES; implementation-fn-note.md intentionally left untracked, .omc/* gitignored).
  • UT (green): go test ./banyand/trace/... ./banyand/internal/storage/... → trace 27.5s ok, storage 9.3s ok.
  • IT via make test-trace-pipeline (builds CGO server + latencystatussampler.so, runs standalone + distributed pipeline Ginkgo suites):
    • Standalone in-merge filter suite: 16/16 SUCCESS, consistently across re-runs — the trace-package merge changes are sound. (One earlier run had a single 30s Eventually timeout on the first drop spec t_drop_1 while its sibling t_drop_2 (same drop predicate) passed → a merge-completion timing flake, cleared on re-run.)
    • Distributed dynamic-sampler restart spec fails (Restart: data node 0 re-applies config from schema store … drop-eligible trace is DROPPED after node 0 restart): a 30s timeout on "trace schema not yet visible after node 0 restart" (TraceRegistryService, a liaison schema service, polled on the restarted data node). PROVEN PRE-EXISTING: checked out HEAD~1 (f0e0dc21, before my commit), rebuilt, ran the distributed suite → the SAME spec fails identically (12 passed / 1 failed, same schema-not-visible timeout). My commit touches only banyand/trace (finalize) + banyand/internal/storage (PeekSegments/#6) — zero distributed/registry/schema/node-startup code — so it cannot cause this. It's a pre-existing distributed restart schema-sync issue on this environment, independent of finalization sampling.

Sync to latest main + re-check failed distributed test (2026-07-06)

  • Rebased trace-pipeline-finalize-event onto latest origin/main (apache) — one new commit a4fac2f4 "Add Consistently check after Eventually checks (#1204)" (broad test-hardening: adds pkg/test/eventually.go + pkg/test/flags/flags.go; does NOT touch the distributed pipeline restart test). Rebase clean, no conflicts; my finalize commit is now f4fe46f1. go build ./... clean; UT (banyand/trace + banyand/internal/storage) green.
  • Re-ran the failed distributed restart spec on latest main: STILL FAILS identically — Restart: data node 0 re-applies config from schema store … drop-eligible trace is DROPPED after node 0 restart, "trace schema not yet visible after node 0 restart" timeout (12 passed / 1 failed). So latest main does NOT fix it (#1204 didn't touch that test). Combined with the earlier HEAD~1 baseline result, this is conclusively a pre-existing, environment-specific distributed-restart schema-sync failure, independent of finalization sampling.

Fable-model audit + fixes (2026-07-06)

Second independent audit (code-reviewer, model=fable). Verdict: FIX-FIRST (0 blockers, 2 majors). It caught a real duplication race the earlier architect pass under-weighted. Fixed:

  • #1 MAJOR (fixed) — scanner↔dispatcher double-pin → duplicate traces. runFinalizeRound and the hot dispatchAllMerges are TWO concurrent part selectors. Both did check-under-RLock then pin-under-Lock separately, so both could select the same cooled part (a cooled segment can still receive late writes → flusher → dispatcher, exactly while finalize runs). snapshot.remove tolerates an absent id, so both merge outputs survive → the part's rows duplicated. Fix: (a) runFinalizeRound now selects+pins+incRefs in ONE inFlightMu.Lock(); (b) dispatchAllMerges re-checks membership under its pin Lock and abandons the dispatch (decRef + return) on conflict. Either actor that gets the lock first wins; the other skips. New test TestRunFinalizeRound_ReplayExcludesStampedParts + existing compose test guard it.
  • #3 MINOR (fixed) — persist failure could disable the round caps. The old code bumped finalizeGenCached even when writeFinalizeState failed; the next round's higher gNext would re-include (re-sample) the just-stamped parts. Now: compute remainder, write state, and ONLY on success mutate the cache + counter; on failure return the error and leave cache==disk (still G), so replay's predicate excludes the gNext-stamped parts (no double-sample).
  • #4 MINOR (fixed) — no disk gate. Added a free-disk headroom check (freeDiskSpace < sum(selected CompressedSizeBytes) → skip the round as an accepted miss) so a full-shard rewrite can't push the hot write path into disk-full.
  • #5 MINOR (fixed) — dead field. Removed unused tsTable.segEndUnixNano (+ its assignment; newTSTable TimeRange param back to _); the scanner uses seg.GetTimeRange().End, not a cached field.
  • #7 (addressed) — test gaps. Added TestRunFinalizeRound_ReplayExcludesStampedParts (DD6 crash-window: a gen-stamped part is excluded on replay, no re-sample) and TestWarrantsFinalize_RatioBranch (the RATIO*total threshold branch, which the ratio=0 warrants test never hit).
  • #2 MAJOR (documented, NOT fixed — needs a decision) — scanner reopens every cooled segment every tick. SelectSegments(reopenClosed=true) incRef→reopens EVERY cooled segment of every finalize-enabled group each 10-min tick, including terminal ones, BEFORE the warrants/terminal check. Accurate severity is DEPLOYMENT-DEPENDENT: with idle-reclaim disabled (default when idle_timeout<1s) cooled segments stay DORMANT (index!=nil) so the reopen is a cheap CAS-bump; only with idle-reclaim ENABLED + long retention do segments fully close (index==nil) and pay a heavy index reload every tick (the bluge-lock churn tsdb.go warns about). A proper fix needs a storage-level way to read per-shard finalize state / skip terminal+cooling segments WITHOUT reopening (SelectSegments has no per-segment reopen control, and reopenClosed=false appends closed segments without incRef → DecRef underflow, so it's not a safe drop-in). Left as a flagged follow-up pending the user's call on approach (storage API change vs. accept given misses-are-acceptable + interval is 10m).
  • #6 MINOR (pre-existing, noted) — selectSegments error-path ref leak (storage/segment.go:618): on a mid-loop incRef error it returns without DecRef'ing already-incRef'd segments. Pre-existing; the finalize scanner is the first heavy caller. Not fixed here (storage-layer, low likelihood).
  • #8/#9 NITs (noted, not fixed): tables,_ := seg.Tables() drops a (currently-always-nil) error; sync-path accounting can inflate the counter for parts synced away (cheap no-op rounds). Gates after fixes: go build ./... clean, make lint clean, finalize + merge unit + integration tests green.
  • Full-suite TestTrace failure investigated → environmental, NOT this change. The full banyand/trace/... run failed one Ginkgo spec: metadata_test.go:395 "changed tag type after merge" (Timed out after 100s waiting for a background merge to reduce the part count) — the known load-flaky merge test (memory: flaky under slow-IO/CI, passes in clean env). Ruled my change in/out decisively: (1) REVERTED the dispatcher conflict-check and the spec STILL timed out at 100s → my change is not the cause; (2) uptime shows load average 49.6 on 32 cores (java OAP + banyand soak saturating the box) → the merge goroutine the test waits 100s for is CPU-starved; (3) the other 8 TestTrace specs passed; (4) the spec passed in the earlier less-loaded full run. Dispatcher fix restored after the diagnostic. This spec will pass in CI's clean environment.
  • RESOLVED 2026-07-06: the load was traced to /tmp/repro3.sh — an orphaned 18-day-old script spawning 48 yes processes (each ~67% CPU = the entire load-49), NOT the soak (which was ~0% CPU). Terminated the yes burners + the vectorized-query soak (banyand + log watchers). Load fell 49.6 → 5.86. Reran: the merge spec passes in 3.7s (was a 100s timeout), and the FULL banyand/trace/... suite passes in 26.9s (exit 0). All trace tests green. (The /data banyand still holding :17912 is a separate service, not the vectorized-query soak, and did not block the suite.)

Audit follow-up round 2 (2026-07-06, user-directed) — #2/#6/#9 fixed, #8 N/A

User reviewed the unfixed audit items and chose: fix #2 via a storage API, and fix #6/#8/#9.

  • #2 MAJOR (fixed) — reopen storm. Added a storage peek API so the scanner no longer reopens every cooled segment each tick. New storage.SegmentPeek{Start,End,ShardPaths} + TSDB.PeekSegments(timeRange) (impl database.PeekSegments → segmentController.peekSegments): snapshots matching segments' time-range+location under RLock, then lists each segment's on-disk shard dirs via walkDir OUTSIDE the lock — no incRef, no index reopen, no loop spawn. The trace scanner (scanGroup) now: PeekSegments(coolRange) → per segment, segmentMayWarrant(lfs, shardPaths, cfg) reads each shard's finalize.json directly and skips (WITHOUT reopening) any segment whose shards are all terminal / max-rounds / in-cooldown → only SelectSegments(peek.Start,peek.End, reopenClosed=true) reopens the segments that may warrant, then warrantsFinalize applies the precise FLOOR/RATIO threshold on the live snapshot. finalizeReopened DecRefs every reopened segment on all paths. Deliberate conservatism: the pre-filter only skips on authoritative-from-disk state (terminal/max-rounds/cooldown), NOT on the FLOOR byte counter — because an OPEN shard's on-disk unsampledBytes can lag its in-memory counter, so a FLOOR pre-skip could strand an actively-late-written shard. Result: terminal + in-cooldown segments are never reopened (kills the storm); past-cooldown non-terminal segments still reopen to be checked, bounded by max_finalize_rounds→terminal. Tests: TestShardMayWarrant + a PeekSegments/segmentMayWarrant assertion in the integration test.
  • #6 MINOR (fixed) — selectSegments error-path ref leak (storage/segment.go): on a mid-loop incRef failure, DecRef the segments already pinned in earlier iterations before returning.
  • #8 — N/A (Fable misread). Segment.Tables() returns ([]T, []Cache), NOT an error — the _ is the unused Cache slice, so there's no dropped error to log. No change.
  • #9 MINOR (fixed) — sync-path accounting. Removed accountUnsampledFlushed from introduceFlushedForSync; only introduceFlushed accounts unsampled bytes (parts on the sync path are handed off, not retained locally, so counting them caused no-op finalize rounds). Gates: go build ./... clean, make lint clean, ShardMayWarrant/FinalizeScan/Warrants tests pass; full storage + trace suites re-running.

Phase 1 (US-001) — DONE 2026-07-05

Files: pipeline_registry.go, trace.go (option), svc_standalone.go (flag), metadata.go (reconcile), tests pipeline_finalize_test.go + pipeline_watch_test.go (resetRegistries + renamed test). Decisions / deviations:

  • Threshold overrides are registry-backed with DEFAULTS only; proto override fields deferred. The proto (TracePipelineConfig) has finalize_grace (field 4) but NO FLOOR/RATIO/cooldown/max_rounds fields. Rather than invent 4 proto fields (repo-wide pb.go regen + validation), I built localFinalizeConfigRegistry + finalizeConfig{floorBytes,ratio,cooldownNs,maxRounds} with lookupFinalizeConfig(group, graceNs) filling unset fields from package defaults (8MiB / 0.10 / grace / 8). reconcile stores an empty finalizeConfig{} (=all defaults) to mark the group finalize-configured; tests inject overrides via setFinalizeConfigForGroup. This satisfies "per-group overridable (mirror registry)" at the infra level; wiring to proto override fields is a clean follow-up if/when the proto gains them.
  • finalizeEventEnabled: empty enabled_events does NOT default finalize ON (opt-in), unlike mergeEventEnabled (empty => MERGE on, for back-compat). Finalize is an extra procedure, must be explicitly requested.
  • reconcile now registers the sampler set when MERGE OR FINALIZE is enabled (shared set per DD11). The clear branch fires only when NEITHER is enabled. finalize_grace default (5m) lives in option.finalizeGraceDefault via new flag --trace-pipeline-finalize-grace-default, resolved lazily at lane time (lookupFinalizeGrace==0 => option default), mirroring merge_grace.
  • Renamed stale test TestReconcilePipeline_MergeEventNotEnabled → TestReconcilePipeline_NoEventOrPluginsClears: its old name/comment ("v1 only implements merge; finalize-only installs no samplers") became false once finalize shares the sampler set. The assertion (empty when no plugins) still holds; the test now documents the correct new semantics. Added TestReconcilePipeline_FinalizeOnlyRegistersSamplers to prove finalize-only WITH a plugin DOES register.
  • Verified: go build ./banyand/trace/... clean; 6 finalize unit tests pass (reconcile tests ran against the built .so, not skipped).