Repository navigation
Expose fulltext operation telemetry and latency histograms - #81
Merged
Merged
Conversation
This was referenced Oct 31, 2026
Open
10 of 51 tasks
1 of 18 tasks
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
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.
⊙ Problem
Expose low-overhead fulltext observability and operation latency metrics #62 needs observations that distinguish admission failures, queue waits, native execution and publication delays. Status totals lack latency distributions and logical batch outcomes.
🤔 Deliver the supported scope together?
Yes: native operations, facade batches and Tantivy reader observations share one contract. Unsupported engine observations remain explicitly unavailable; consumers own export and application metrics.
💡 Solution
Add
telemetry()andconfigureTelemetry()to writer and reader handles. Collection is opt-in; callers start/stop without reopening. Counters/timing totals always accumulate; histograms/bytes start disabled. Final scalars survive close without retaining index resources. Source builds require ABI 14 / telemetry API 1; version selection stays with release.🤔 Keep accounting always on?
Yes. Kyle accepted always-on counters/timing totals at
077dffc. Worker-banked errors trade scalar storage for lower contention. Their incremental ordinary-search cost remains uncertain and needs merge review. Histograms and snapshot-byte observations remain opt-in; preserve targets and raw evidence.⚖️ Alternatives
🤔 Reduce sharing without stronger ownership assumptions?
Use atomic worker banks; ticket selection stays stable and writer timing unchanged. Exclusive-writer loads/stores add an ownership invariant without a measured need. Status changes complicate terminal observations, directory scans add I/O, and a monitoring SDK adds a dependency. The banks remain an internal implementation choice.
🔧 Changes
Product and architecture tour — ownership, lifecycle, compatibility and tests
Product and architecture tour
Measure the layer that owns the work
Which observations belong to fulltext, and which remain with Tantivy or Harper?
ts/native.ts exposes synchronous
telemetry()andconfigureTelemetry()on writer and reader facades. ts/telemetry.ts counts logical batches, successful partial frames and preparation rejections separately from native commands. The batch facade records partial frame progress; search and trace reject closed preparation before handoff, including caller accessors that begin close during encoding. The reader facade delegates the same contract. A direct packed frame is not a logical batch; a JavaScript validation failure is not a native attempt. Bigint values retain counter precision, and fixed error-code labels exclude query text and record IDs.docs/observability.md defines counter, histogram, queue and reader semantics, including cooperative deadlines and unavailable measurements. Harper still owns end-to-end latency, authorization, hydration, replication and source-log lag. Telemetry serialization/parsing is synchronous on the caller thread; the final guidance recommends staggering collection across indexes and benchmarking the intended exporter schedule. Partial configuration merges omitted flags from native authority; full configuration skips that read. Construction retains a valid token if its first observation fails, so later snapshots/configuration recover. No native snapshot is read on search or batch paths; sequential rewrap inherits native settings while concurrent owners remain unsupported.
Observation ownership
Each layer contributes observations at its existing boundary; scraping does not invoke a search.
flowchart LR F[JavaScript facade] --> N[Native admission and actors] N --> T[Tantivy searcher] F --> B[Logical batches and preparation] N --> C[Operation counters and queues] T --> R[Cached reader observations] B --> S[Telemetry snapshot] C --> S R --> SCount admitted work exactly once
When does a rejected request become a command that must complete?
src/native.rs⚠️ Look hardest: admission arms a completion ticket under the successful queue-insertion lock, before actors can see it. Rejected insertion leaves the ticket inactive. src/telemetry.rs makes activation and settlement idempotent, including dropped commands and poison-driven close requeue. Native cumulative counters wrap modulo 2^64; snapshots are relaxed observations, so conservation checks apply at quiescence rather than during transitions. Worker binding selects one bank before dispatch; ticket selection remains fixed through settlement/drop and all increments stay atomic. Worker failure-error labels use those same banks; admission rejections and unbound drains retain shared fallback accounting. Mutation counters remain shared. The memory/scrape invariant records three 640-byte banks per worker: 7.5 KiB at four workers and 120 KiB at 64, plus fixed alignment/container overhead. Scrapes aggregate scalars before JSON; no new per-query allocation or writer-clock change is introduced.
native.rs:1694
Queue timing includes any permit wait after dequeue; execution starts afterward. Commit, reader-reload and teardown phases retain their own success/failure counts, so a committed write is distinguishable from reader-visible publication. src/DESIGN.md records that optional byte observation runs after command settlement and before the diagnostic
writer-end: that gap alone does not prove a stalled engine operation.Cache reader observations without retaining the index
What survives close, and when can observation add work?
docs/native-backend-implementation.md explains reuse of Tantivy's public document, deleted-document, segment, generation and space-usage APIs. Cheap counts update only after actual reader replacement. The weak token retains scalars; forced close preserves the canonical closed state, and teardown accounts for resource and merge-worker joins. The shared lifecycle codes align native status and telemetry labels; the parity regression checks initialization and all four wire codes/JSON labels. Optional snapshot bytes run on the writer actor after successful publication/reload settlement, outside reader/cache locks, and may increase subsequent queue wait. Unchanged segment/deletion sets reuse the successful byte observation; its age is separate from cheap-stat freshness. These bytes exclude old snapshots, uncommitted files and in-progress merge files, and observation failure cannot change publication or reload results.
Require a matching facade and addon
How does an application recognize this unreleased telemetry capability?
src/lib.rs advances native ABI to 14 and declares telemetry API 1 while retaining query API 7. ts/load-addon.ts checks that capability and all three internal telemetry exports. ts/addon.d.ts includes the generated runtime capability type. The existing status/open/close binary shapes and packed protocol 8 remain unchanged.
test/load-addon-resolution.test.mjs rejects missing exports and unsupported telemetry versions while retaining package-resolution controls; test/native.test.mjs checks the runtime contract. README.md distinguishes unreleased ABI 14 from published 0.8.0 ABI 13, and docs/limits.md preserves the existing Rust migration guidance with the current source-tree ABI. docs/metadata-migration.md corrects the published release status without changing rebuild procedures. No package version is selected by this PR.
Exercise failure, concurrency and resource release
Which controls make the accounting and lifetime promises reviewable?
test/native-telemetry.test.mjs covers native versus logical work, partial batches, histogram toggles, rejection privacy, queue saturation/expiry, actual search-actor panic, poison before admission, commit versus reader visibility, byte-cache freshness/failure, concurrent search scrapes and final post-close counters. Its facade distribution control pins zero, inclusive boundaries, overflow, histogram freeze, bigint decoding and snapshot isolation. The byte-freshness test pins
snapshotBytesPendingas measurement needed, including when enabling schedules no work. Its accessor-close regression fails without the final handoff guards and verifies preparation rejection without a native attempt. New controls prove sequential native-setting inheritance, exact snapshot-call counts and recovery from constructor/partial-configuration read failures through public writer and reader handles. Panic-stage tests preserve the unknown-class fallback; production and the known-class regression share a selector, and an always-ordinary mutation fails. Worker-error tests cover locality, shared rejection/drain, unknown labels and wrapping. The new panic-and-drain regression checks one bound ticket plus queued fallback completions exactly once. test/fixtures/telemetry-counter-stress.mjs runs 10,080 mixed ordinary/expensive/trace operations per histogram mode. test/telemetry-counters.test.mjs exercises that public contract; test/telemetry-counters.optimized.test.mjs repeats it against the optimized addon. Both verify final counts after close. The delete-only regression proves byte-cache invalidation when a deletion preserves segment count; the concurrency control requires an observed queued/dispatched native search instead of unfinished JavaScript callers.test/native-telemetry-lifecycle.test.mjs runs lifecycle controls in isolated processes. test/fixtures/telemetry-lifetime.mjs proves counter-state reclamation after the closed facade is collected; its forced GC is a lifetime-test control, not a performance technique. test/fixtures/telemetry-worker.mjs supports worker-termination/reopen coverage. test/fixtures/telemetry-close-failure.mjs preserves counters through cleanly released and unproven close failures.
Export locally and keep overhead evidence bounded
How can a consumer use this, and what has performance qualification established?
examples/observability.mjs emits vendor-neutral JSON with explicit bigint conversion. Collection starts disabled, then enables, pauses through another search, resumes cumulative counters and observes final close. No wrapper timer or recording switch is added. Writer/reader regression cases verify the same policy with real native queries and independent optional flags. Harper opt-in lifecycle acceptance is recorded in Implement full-text generation lifecycle, rebuild, eviction, backup, and status; actual runtime wiring awaits compatible release adoption. package.json includes that example in smoke discovery and adds the observability benchmark command.
benchmarks/observability.mjs uses separate baseline/candidate modules in fresh child processes, alternating pair order, fixed correctness checks and separate histogram, polling and byte profiles. It verifies ordinary versus expensive native routing before timing. Its query timer stops at API completion before validation, and its bounded windows fully drain callers before validation, excluding the oracle from throughput. CPU/event-loop observations use those same windows; staggered polling reports per-handle counts and scrape durations. test/observability-benchmark.test.mjs runs the real driver with test/fixtures/observability-benchmark-clock.mjs, an isolated deterministic child clock that distinguishes 5 ms of API work from 100 ms of oracle work. benchmarks/DESIGN.md records this isolation protocol. docs/observability-qualification.md preserves historical reports with their validation-inclusive scope, records the new optimization/total-cost and scrape measurements separately, and retains the historical classification correction: exact-total requests measured expensive work even when labeled ordinary. The classified matrices completed 13.44 million correct timed queries before the final close guards. The later isolated optional-cost control completed another 2.24 million on historical runtime
b3f9be1; its 10 ms stress polling showed measurable cost with histograms disabled. Tight overhead targets remain unestablished. These local observations do not qualify production capacity or sustained memory. The root design index links the bank and qualification invariants. The committed benchmarks/observability-scrape-cost.mjs reproduces the bounded idle topologies, raw per-handle samples and configuration. test/fixtures/observability-scrape-clock.mjs verifies scrape counts, settings and cleanup failures without treating a fake clock as native performance evidence. Matching-baseline mode labels optimization-only comparisons; short polling cells explicitly report insufficient samples. Both drivers retain failed fixtures and primary/secondary failures. The paired driver requires clean close after success and rollback after failure; reports are emitted only after verified cleanup. The qualification guide records the exact four/sixteen-handle active control commands, without changing the original process policy. The latest qualification section records native failure/success controls, mixed producer failures and native-only aggregation. Historical runtimef4f51ddidentifies the 5.04 million-query series, 96,000 public snapshots and 56,000 native snapshot samples. At four workers, banked failure recording is about five times faster than the old shared recorder; single-worker rows are slower. Three public-call repetitions retain 96,000 samples, with p50 near 0.28–0.29 ms through 64 workers. Seven ordinary-search pairs per cell and one targeted repeat validate 5.04 million queries but leave one histogram-on cell unresolved: median throughput loss 2.58% then 1.25%, both ranges crossing zero. Raw hashes, exact recorder revisions and the initial acceptance scope are preserved; these are local fixture observations. A same-build no-polling/staggered-polling control ate63301ccompleted another 28 million correct timed searches across seven alternating pairs at four/sixteen indexes. At roughly one call/second/handle, median paired throughput loss was 0.82%/1.24% (ranges -3.48–2.77%/-1.17–5.75%); p99 rose 0.55%/1.36%. Basic accounting stayed active, optional flags stayed off, and each handle had 11–12 calls. Call p50 was 0.385/0.350 ms; synchronous call wall time occupied median 0.195%/0.684% of mixed elapsed time, not CPU utilization. This supports optional caller-controlled polling without a new wrapper switch, while preserving uncertainty and the separate basic-accounting cost. The managed reconciliation preserves all 13 filed deferrals in Expose low-overhead fulltext observability and operation latency metrics #62; raw search-preparation error consistency remains in Align direct Rust search validation with the public packed API #65. Accept anchored shorthand citations in state traces #400 fixes the citation reader without changing fulltext source. The final managed delta review converged at033dd61with both provenance and lifecycle advisories resolved. Seven overflow groups remain explicitly unchecked. The qualification-history requirement preserves measured commits through integration; all 23 linked source references resolve to ancestors ofdf54b0b. Accepted accounting costs, historical failed evidence and publication alignment remain tracked.The short-soak driver adds an optional publication minimum without changing duration-only defaults. Its CI regressions use real native delayed publication to cross the one-second phase, then verify success, intentional failure and the absent-option control. Invalid and missing values reject before artifacts are created. Fixture guidance preserves the 40-second child/45-second test bounds. The qualification guide records eight Node 24/26, coverage off/on run-throughs and separates fixture completion evidence from performance and sustained-memory qualification.
✅ Verification
npm run checkat5170009passed 185 Rust tests (nine ignored) and 537 Node/worker/package tests. Twelve optimized tests passed; production addon restored. Final033dd61only tightens guide wording. Regression and mutation controls failed before fixes.Historical measurements retain 5.04 million timed queries and 96,000 public snapshots. Optional polling’s median throughput cost was 0.82%/1.24% across 28 million queries at four/sixteen indexes. Basic accounting remains active; incremental cost is uncertain.
🤔 Final review delta?
All 13 deferrals are filed. Claude/Composer review converged; both advisories resolved. Version alignment awaits release. Historical Windows qualification timeout remains tracked; its cause is unproven.
Comment generated by kAIle (GPT-6)
Release target: main — new observability API; version selection deferred to release.
Related PRs: #82 overlaps (coordinate shared ABI/loader declarations with Add query boosts and optional full-text scoring), 3 others independent
Complexity: complicated
Deferred findings
f8bb6c1668f0d6b6major performance —src/telemetry.rs— Per-search Ticket Arc Metrics clone/drop contends one refcount line across JS thread and all workers; 16/64-worker path unmeasured — filed Expose low-overhead fulltext observability and operation latency metrics #627f007587637804a2minor reliability —package.json— Tree keeps version 0.8.0 but requires ABI 14; published ABI-13 0.8.0 platform packages are rejected before local fallback — filed Expose low-overhead fulltext observability and operation latency metrics #620f8522346a859f88nit correctness —src/native.rs— Classifier panic always counted as ordinary search even if the request was expensive — filed Expose low-overhead fulltext observability and operation latency metrics #6290ddd36244cc9d01nit maintainability —ts/native.ts— Search preparation failures rethrown without normalizeNativeError — filed Align direct Rust search validation with the public packed API #652335b89f8c04c2bfnit performance —src/telemetry.rs— Scrape walks every configured worker bank; cost scales with searchThreads even when idle — filed Expose low-overhead fulltext observability and operation latency metrics #62488edead300a34banit performance —src/native.rs— Poison/failure storms hit shared admission and error atomics, not worker banks — filed Expose low-overhead fulltext observability and operation latency metrics #62571ef85f754f945enit correctness —src/telemetry.rs— Enabling snapshotBytes sets snapshotBytesPending with no observation scheduled — filed Expose low-overhead fulltext observability and operation latency metrics #62657d962ab7f02c8anit maintainability —benchmarks/observability-scrape-cost.mjs— Scrape child closes with rollback even after a successful measurement — filed Expose low-overhead fulltext observability and operation latency metrics #62259bce1b9a03dd27nit performance —src/telemetry.rs— Poison/failure storms hit shared admission and error atomics, not worker banks — filed Expose low-overhead fulltext observability and operation latency metrics #62b679793ce2beb276nit maintainability —package.json— benchmark:observability script skips build/run-local wrapper used by sibling benchmarks — filed Expose low-overhead fulltext observability and operation latency metrics #62🤔 defer this PR's major performance finding
f8bb6c1668f0d6b6(Per-search Ticket Arc Metrics clone/drop contends one refcount line across JS thread and all workers) to #62 instead of fixing it here?Review-Coverage: authored=codex; ran=cursor-composer,claude; adjudicated=domain(claude); declined=gemini,cursor-grok,muse; rounds=21; full=7 @ 033dd61
Review-Attention: study ~20m (decisions: preserve-measured-history, deferred-f8bb6c16) @ 033dd61