perf(algebraic): use hash-set membership for cardinality doc-id constraints - #141
Closed
ajroetker wants to merge 26 commits into
Closed
perf(algebraic): use hash-set membership for cardinality doc-id constraints#141ajroetker wants to merge 26 commits into
ajroetker wants to merge 26 commits into
Conversation
…raints Cardinality, range, and histogram queries funnel through cardinalityDocMatchesConstraints, a linear scan over a doc-id slice that is invoked once per scanned index entry. For histogram/range children the full field prefix is re-scanned per bucket and tested against that bucket's doc-id list, making the scan O(entries * constraints) -- i.e. quadratic in the document count. These were the only algebraic queries measured slower than a plain document scan in the algebraic benchmark. Introduce DocIdConstraintSet, a borrowed-key hash-set built once per scan, and replace the linear membership test at all eight cardinality scan/collect sites. Each membership test becomes O(1) and the scan is a single linear pass again. Results are unchanged (the benchmark cross-checks algebraic output against document-scan checksums). https://claude.ai/code/session_01UVKuQU8S6THBNkSBHoVnzz
Histogram, range, and terms cardinality queries computed each bucket's nested child cardinality by re-scanning the entire child field once per bucket (scanDistributedCardinalityPartialsForDocIds), making the cost O(buckets * field_entries). Build a ChildCardinalityIndex once per child field (a single cursor scan grouping distinct value-keys by document) and look up each bucket's documents against it, turning the work into O(field_entries + bucket_members). Output partials are byte-identical, so cross-engine correctness checksums are unchanged. Quick profile (1000 docs), algebraic engine avg latency: histogram_nested_cardinality 15.2ms -> 5.4ms range_nested_cardinality 14.3ms -> 10.9ms root_cardinality 5.6ms -> 3.5ms
mergeOneSlotValuesAlloc wrapped each fold/merge contribution in two throwaway single-slot Rows (two row allocations, two value dupes, a combine, and a final dupe) just to call law.combineAlloc on one slot. On the per-contribution fold and distributed-merge hot paths this is ~6 allocations for what is a single law combine. law.combineAlloc already returns a freshly owned buffer, so call it directly and fall back to the law identity when it yields null (preserving the previous "never null" contract). Output bytes are unchanged: every algebraic benchmark checksum is identical before and after.
DerivedJoinFoldAccumulator round-tripped each group's running total
through decimal text on every contribution: combineAlloc parsed the
accumulated text and the contribution, added, and re-formatted, once per
folded row.
Hold the running value natively instead - i64 for count, f64 for
sum/sumsquares, and {sum,count} for avg - and format to text only once
in entriesAlloc. encode/parse round-trips exactly for i64 and f64 ("{d}")
and contributions are applied in the same order, so the formatted result
is byte-identical (every algebraic benchmark checksum is unchanged). The
non-additive laws (min/max, booleans, set/tuple union, max-timestamp)
keep the text-merge path, where original-token text and order matter.
The linear doc-id membership helper has no callers since the cardinality scans switched to DocIdConstraintSet hash-set membership.
Adds approximate distinct-count materialization so a cardinality query reads one sketch per bucket instead of rescanning and deduplicating every document's value (O(groups) instead of O(documents)). - hll.zig: dense HyperLogLog sketch (add/merge/estimate/encode) with the classic bias-corrected estimator plus linear counting for small cardinalities. p=14 standalone, p=12 for materializations. Register-wise max union makes a sketch a bounded join-semilattice; folding singletons is byte-identical to direct insertion. Unit tested across scales. - law.zig: HLL as a first-class lattice law (`hll`) - non-invertible, approximate merge - so sketches compose with the existing tensor and distributed-merge machinery. Identity is the empty sketch. - index.zig: `hll_cardinalities` config (group_by + value_field + precision). Sketches are maintained incrementally on ingest by unioning each document's singleton into its group's sketch, and read via approxCardinalityEntriesAlloc (per group) / approxCardinalityTotalAlloc (union across groups). Append-only: deletes need a rebuild and are not folded back. End-to-end test asserts per-group and total estimates land within one of the exact distinct counts. Maintenance reuses the existing field extraction, so this does not thread a new algebra.Op through the query planner; the planner is not yet wired to auto-select HLL materializations for nested cardinality queries.
…dinality Adds `--mode hll`, which maintains per-region/per-product HLL cardinality sketches on ingest and compares the O(groups) sketch read against the exact doc-scan distinct count (terms(region) -> cardinality(customer_id)), reporting build cost, latency speedup, and the approximation error. At docs=5000 (mem backend) the materialized read is ~1.6x faster than the exact doc scan with ~1.3% total error, trading heavier ingest.
Cardinality aggregations now consult a matching materialized HyperLogLog sketch before falling back to the exact distinct-count scan, so a query reads O(groups) sketches instead of rescanning and deduplicating every document's value. - index.zig: approxCardinalityTotalForFieldAlloc answers a root cardinality from the merged union of every group's sketch; approxCardinalityEntriesForGroupAlloc answers a terms(group)-> cardinality(field) query per bucket. Both resolve the query field (name or path) to the materialization's value_field and group layout, and return null - falling back to the exact path - when constraints or an MVCC read generation are present, since sketches are maintained unconstrained and without per-generation visibility. - aggregations.zig: the root cardinality dispatch and the terms+cardinality-children path try the HLL lookup first, matching each bucket's group key to its sketch estimate, and otherwise scan exactly. A planner test asserts root and per-bucket cardinality are served from the sketches and match the exact distinct counts for small inputs.
HyperLogLog sketches cannot subtract a value, so deletes and overwrites previously left the per-group sketches over-counting. removeDoc now marks every materialization the document contributed to as dirty (in the same write txn), and a maintenance pass rebuilds those materializations' sketches from the surviving document facts and clears the marker. The rebuild runs as background work through the BackendRuntime durable-job lane: after a batch that deletes or overwrites documents, the index submits a maintenance job (attachHllMaintenanceLane wires the lane; close drains any pending jobs for the owner). With the inline lane it runs synchronously once the batch commits; with a threaded lane it runs off the write path. When no lane is attached, callers drive runHllMaintenance directly. A test deletes two of three distinct customers from a region and asserts the inline-lane rebuild collapses that region's estimate from 3 to 1.
Every transaction - read or write - cloned the entire entry list to get a snapshot, making reads O(total entries) and ingest O(n^2). That made the in-memory backend pathological for prefix scans (e.g. reading materialized HLL sketches was slower than a doc scan at scale). Hold the store state in a reference-counted, immutable-while-shared RcState. A read transaction now retains the current snapshot in O(1) instead of copying it; a writer still clones into a fresh snapshot and publishes it on commit, releasing the previous one. Readers opened beforehand keep observing the snapshot they retained until they release it, so snapshot isolation is preserved (the existing isolation test and the full db-test suite pass). Writes continue to clone, so this targets the read path; the write/ingest path could be made cheaper later with a persistent ordered structure.
The reference-counted snapshot made reads cheap but writes still cloned the whole sorted entry array per transaction and shifted it on every insert, so ingest stayed O(n^2). Replace the sorted array with a persistent treap (mem_ordered.zig): a balanced BST keyed by (namespace, key) with hash-derived priorities and path-copying, reference-counted nodes. Insert/delete now touch only O(log n) nodes and share every untouched subtree, so a write produces a new version in O(log n) and a snapshot is still O(1). Transactions start from a shared snapshot and publish a new root on commit; older snapshots stay valid until released. The module is leak-checked in isolation (randomized op stream vs a reference map); the full storage suite (1481 tests, 0 leaked) and db-test pass on it.
Addresses the review findings on the materialized HLL cardinality feature:
- In-place updates lost sketch maintenance. The coalesced update path
(tryUpdateDocCoalesced) bypassed addDoc's delta, so re-upserting a doc with
a changed value_field never updated or dirtied the sketch. HLL ingest is now
driven from the written document facts in writeDocFacts (covering the plain
and append-only-bulk add paths), and the coalesced path marks the affected
materializations dirty (old facts can't be subtracted from a sketch) and
warm-folds the new facts pending the rebuild.
- Ingest/rebuild tokenization could diverge. Incremental ingest hashed a JSON
token while the rebuild hashed the stored fact scalar; for bytes/binary
fields these tagged differently ("s" vs "x"), so a rebuild shifted the
estimate. Both paths now hash the same projected fact scalar.
- Stale reads after a delete/overwrite. The query-routing gate only rejected
constraints and MVCC generation, so reads between a delete and the async
rebuild returned the pre-delete over-count. The gate now also rejects a
materialization whose dirty marker is set, falling back to the exact scan
until maintenance lands.
- Maintenance scheduling used a batch-shape heuristic (deleted/overwritten
keys) that missed coalesced updates. It now schedules iff the committed
batch actually left a dirty marker.
- Corrupt sketch could crash the estimator. registersView validated length but
not register values; estimate() shifts by the register, so a byte >= 64 was
an out-of-range shift. registersView now rejects out-of-range registers.
- Planner accounting. The terms-children HLL branch now records planner
selection, mirroring the root cardinality path.
Adds regression tests: corrupt-register rejection, in-place value change, and
bytes-field ingest/rebuild token parity. Full db-test suite passes.
Follow-ups from re-reviewing the previous correctness commit: - In-place updates no longer over-invalidate. The coalesced update path marked a sketch dirty (forcing a full-docfact-scan rebuild) on any fact change, even when the materialization's group_by/value_field were untouched. Dirtying and the warm fold are now gated on whether the fields the sketch actually reads changed between the old and new facts. - Hot-path merges stay cheap and tolerant. registersView validated every register byte, which ran on each per-document fold (merge = register-wise max, which never shifts) and turned a single corrupt stored sketch into an aborted write batch. Split into a cheap structural check (registersView, used by merge) and a full per-register bounds check (validatedRegistersView, used only at the estimate/decode boundary where the shift happens). - Maintenance scheduling is best-effort. scheduleHllMaintenanceIfDirty runs after the batch has durably committed, so it no longer propagates a transient read-txn error out of applyBatchWithOptions. The persisted dirty marker plus the dirty-aware read gate keep results correct if a schedule is skipped. Adds a regression test: an unrelated-field update leaves the sketch clean. hll unit tests and the full db-test suite pass.
…nd writes The store contract is single-writer, but background HLL cardinality maintenance (runHllMaintenance) opens its own write transaction on the durable-job lane's thread, concurrently with foreground applyBatch writes. With the threaded lane that is a data race: on the in-memory backend two write txns each clone the current snapshot and the later commit wins, silently dropping the other's mutations (a lost update); other backends likewise assume a single writer. Add an index-level write mutex that both applyBatchWithOptions and runHllMaintenance hold around their write transaction, so at most one index write txn is open at a time. Readers are unaffected (they use store snapshots). runHllMaintenance re-checks the dirty marker inside the lock so redundant queued jobs collapse to a no-op, and maintenance scheduling stays outside the lock (it only opens a read txn and submits a job). Adds a concurrency regression test that drives 40 interleaved insert/delete batches against a real io_threaded maintenance lane and asserts the final estimate is intact (not zeroed or corrupted by a lost update / torn read).
A delete/overwrite marked a whole materialization dirty and the maintenance pass rebuilt it by rescanning every document fact in the index (O(all docs)) — heavy for a large shard with frequent deletes, even when a single document in a single group changed. Record per-group dirty markers (hllcard_gdirty:<name>:<group_key>) from the affected document's own facts, and rebuild only those groups: resolve each dirty group's surviving members by intersecting the per-axis docfact scalar postings (the secondary index), then recompute just that group's sketch from its members' value tokens. Cost drops from O(all docs) to O(members of the changed groups). An emptied group drops its sketch. The whole-materialization marker remains the fallback (full rebuild) for the cases where the affected group key cannot be derived from the available facts; a full rebuild also clears any pending per-group markers. The read gate and the maintenance scheduler now treat a materialization as dirty if either the whole-materialization marker or any per-group marker is present. Existing HLL tests (delete rebuild, in-place value change, unrelated-field skip, bytes-field parity, threaded-lane concurrency) pass against the scoped path; full db-test suite green.
…rror budget)
Every cardinality aggregation result now states whether it is exact or an
estimate, so a client can never mistake an approximation for a count:
{"value": 4044, "approximate": true, "relative_error": 0.0081} // HLL sketch
{"value": 4096, "approximate": false} // exact scan
- hll.zig: relativeErrorForPrecision(p) = 1.04 / sqrt(2^p), the documented
standard error of a sketch (≈1.6% at p=12, 0.8% at p=14).
- index.zig: hllRelativeErrorForField mirrors the read-gate matching so the
estimate and its error budget always come from the same sketch.
- aggregations.zig: a single cardinalityResultJsonAlloc renders all cardinality
results (root, terms/range/histogram children, distributed-partial, doc-scan,
doc-id metric); approximate results carry relative_error, exact ones omit it.
- Tests updated: exact-path expectations gain "approximate":false; the HLL
planner test asserts value + "approximate":true via a substring helper rather
than pinning the relative_error float formatting.
- ALGEBRAIC.md documents the hll_cardinalities config and the result contract.
Response-only contract (no request-side knob yet); selection stays automatic
(auto). Full db-test suite: 1790 passed, 0 failed.
ajroetker
force-pushed
the
claude/algebraic-index-perf-vSyqv
branch
from
May 31, 2026 01:34
328649e to
f5b695b
Compare
…blic API
The engine already returns whether a cardinality value is exact or an HLL
estimate, but the HTTP boundary dropped it: the OpenAPI AggregationResult had
no such fields and the serializer copied only value/count/min/... So a client
always saw {"value": N} with no way to know it was an estimate.
- specs/openapi/antfly/metadata.yaml:
- AggregationResult gains `approximate` (bool) and `relative_error` (float).
- New CardinalityMode enum (auto|exact|approximate) and an AggregationRequest
`mode` field selecting exact-vs-approximate at query time.
- Regenerated SDKs (make openapi-generate): metadata/public/client types +
root openapi.yaml. openapi-check confirms the regen is deterministic.
- query_contract.zig: parse `mode` -> SearchAggregationRequest.cardinality_mode;
emit approximate/relative_error in toOpenApiAggregationResult (propagates to
terms->cardinality bucket children via the recursive serializer).
- aggregations.zig: cardinality_mode wired into dispatch — exact never consults
a sketch; approximate requires one (errors when none applies); auto unchanged.
- Tests: mode=exact reports approximate:false despite a matching sketch;
mode=approximate serves from the sketch, and errors when no sketch applies.
db-test passes. Note: no production path provisions a sketch yet, so reads are
exact in practice until adaptive (observed) HLL provisioning lands next.
…bserved queries Previously no production path ever populated hll_cardinalities, so approximate cardinality was unreachable: a sketch could only exist if hand-written into the index config. Now a recurring cardinality query promotes one, matching how the adaptive subsystem auto-creates materializations. - Runtime registry: hll sketches move from a fixed config slice to a runtime ArrayList (hll_registry), seeded from config at open() and extendable later. All read/maintenance sites go through hllCardinalities(); promotions append. - Observation + promotion: observeCardinalityForAdaptive(store, group, value) counts a (group, value_field) cardinality shape in its own keyspace; once it hits adaptive.min_observations it registers a sketch, persists a marker, and marks it dirty. The existing maintenance pass backfills it from stored facts. Gated on adaptive.lazy_materialization and skipped for constrained/MVCC reads (matching what the read gate can serve). Best-effort: never fails a query. - Read path: root and terms->cardinality aggregation paths record the shape, so repeated queries promote a sketch and subsequent reads return approximate:true. - Persistence: promotions survive reopen via loadAdaptiveHllCardinalities, which reloads the markers into the registry. Engine test drives the full loop: observe to threshold -> promote -> backfill -> correct per-group estimate, plus reload-after-reopen. db-test: 1793 passed, 0 failed.
…elf-schedule adaptive backfills Make adaptive HLL cardinality sketches reachable on a live server. Two gaps remained after the engine and adaptive-promotion logic landed: 1. No live server attached the durable-jobs maintenance lane to algebraic indexes, so even a promoted sketch never backfilled and never reloaded across restarts. DB.open now hands its backend runtime's durable-jobs lane to the IndexManager (a dedicated owner id) before loading indexes; the algebraic open site attaches the lane to each index and reloads any persisted adaptive markers via loadAdaptiveHllCardinalities. 2. Adaptive promotion happens on the read/query path and only marked the new sketch dirty, relying on a later unrelated foreground write batch to fire maintenance. For the read-mostly cardinality workload adaptive provisioning targets, the sketch would stay dirty indefinitely. observeCardinalityForAdaptive now schedules the backfill itself (outside the write lock) the moment it promotes, so the next read resolves an approximate count. Adds a DB.open integration test that drives observe -> promote -> lane-backfilled approximate count and survives a reopen, exercising the full wiring. Updates the shared-runtime owner-id assertion to account for the dedicated HLL owner id. https://claude.ai/code/session_01UVKuQU8S6THBNkSBHoVnzz
…intenance, not on reads Fold the adaptive HLL cardinality path into the same shape as the existing tensor adaptive lifecycle: the read/query path now only records a durable observation counter, and promotion + backfill move into the leader-gated maintenance pass. Previously observeCardinalityForAdaptive promoted a sketch and scheduled its backfill directly on the query path. On a multi-replica shard that let a follower serving reads mutate derived sketch state, made promotion replica-local and non-deterministic, and put backfill latency on the promoting query. The tensor adaptive path already draws the line correctly — record query shapes on reads, decide/backfill during maintenance — so HLL now matches it: - observeCardinalityForAdaptive -> recordHllCardinalityObservation: bumps the hllobs counter only; never promotes or backfills. - evaluateHllCardinalityCandidates(store): new leader-gated step that scans the observation counters, promotes any over the threshold, and backfills them in one pass. Wired into evaluateAlgebraicAdaptiveCandidates alongside the tensor evaluation, which runs under lockApply and is driven by runUntilIdle on the write/leader path. The IndexManager lane wiring stays — it still backfills write-invalidated sketches off the foreground path. Tests now drive promotion through the maintenance pass (engine-level and through DB.evaluateAlgebraicAdaptiveCandidates). https://claude.ai/code/session_01UVKuQU8S6THBNkSBHoVnzz
…se 1) First consumer of HLL distinct-cardinality sketches in query planning: a recurring terms(field) is a recurring distinct-count of that field, so the terms path now (1) records it as an adaptive observation — promoting an ungrouped NDV sketch through the leader-gated maintenance lifecycle, which also answers cardinality(field) — and (2) consults that sketch to pre-gate the aggregation. When a current sketch shows the field's global NDV (== the number of result buckets) confidently exceeds max_result_buckets, rejectTermsAboveEstimated- BucketLimit fast-fails with the same error.AlgebraicResultBucketLimit the post-scan check raises, before any materialization scan. The comparison uses the low end of the sketch's ~2σ error band so a query that might actually fit is never rejected here, and the authoritative post-scan bucket check remains the backstop. The estimate is advisory: no sketch, a constrained read, or an MVCC-pinned read all decline it and leave today's behavior unchanged. Lookup mirrors the existing cardinality path (done in aggregations.zig where the store txn is in hand; the planner stays pure). No planner.zig changes. https://claude.ai/code/session_01UVKuQU8S6THBNkSBHoVnzz
…s (phase 2) Wires HyperLogLog into the derived join fold so cardinality(field) can be answered across a bounded join — both at the root and per terms bucket. The fold accumulator and merge were already law-agnostic (text slot + law.combineAlloc, which unions HLL register-wise), but the path was gated: lawPreservesDerivedJoinFold returned .hll => false, the request carried only an algebra.Op (which has no cardinality op), and the contribution builder had no sketch path. This change: - Adds an optional `law` override and `hll_precision` to DerivedJoinFoldRequest, so a fold can run as .hll independent of op; the scan derives law from `request.law orelse fromOp(op)` and requires a measure for any non-count law. - Folds a singleton sketch of each matched value per pair. The cardinality target is usually a group dimension, which measureAlloc (measure-parts only) can't see, so ProjectedFact.valueScalarAlloc reads it role-agnostically. - Flips lawPreservesDerivedJoinFold(.hll) to true. HLL unions form a distributive lattice; derived folds read current facts each query, so the law's non-invertibility (no incremental delete) doesn't apply here. Routing + result extraction (aggregations.zig), via the direct law-aware scan rather than the op-keyed tensor-program path: - Root cardinality(field) carrying a join folds per-group sketches and estimates their union. - terms(field) over a join with cardinality children gets a derived-join variant: bucket counts from the count fold, per-bucket distinct counts from an HLL fold grouped by the bucket field. The non-join path would ignore the join, so a join routes here and returns unsupported if it can't be proven. https://claude.ai/code/session_01UVKuQU8S6THBNkSBHoVnzz
…fanout gate (phase 3) Two NDV uses in join planning, both sound (distinct-count, not skew): Sizing — a grouped materialization stores one row per distinct group-key value, so its stored-row count is the NDV of the group key, not the document count. In the adaptive cost model (adaptiveCostInputs), the not-yet-built bucket_cardinality fell back to doc_rows, a gross overestimate that inflates the write cost and suppresses promotion of low-cardinality groupings. adaptiveGroupKeyNdvEstimate now sources it from a current HLL sketch on the (single) group field when one applies, falling back to doc_rows otherwise — never raising the estimate. This reuses the same ungrouped NDV sketch the phase-1 terms path already promotes. Fanout gate — the derived join fold enforces max_fanout per key at scan time (AlgebraicJoinFanoutExceeded). derivedJoinFanoutGuaranteedExceeds pre-gates it: when the average right-side fanout (right facts / NDV of the right join key) already exceeds max_fanout, the maximum per-key fanout must too (max >= mean), so the scan is guaranteed to blow up. scanDerivedJoinFoldEntriesAtGeneration then fails soft (returns null = unsupported) instead of raising mid-scan. This is one-directional by design: HLL gives the mean, not the skew, so it can only prove a blowup, never rule one out. It uses the upper end of the sketch's ~2sigma error band and a bounded fact count (stop one past the threshold) so it never fast-fails a join that might fit and adds no full scan per query. The same scan path also observes the join key so maintenance promotes the NDV sketch the gate needs (unconstrained, non-MVCC reads only) — otherwise coverage is nil, since nothing else promotes join-key sketches. https://claude.ai/code/session_01UVKuQU8S6THBNkSBHoVnzz
… wrong join-free answer
Reviewing phase 3 surfaced a correctness seam in how the pre-gate's decline
threads through the aggregation dispatch. The gate returned null ("declined"),
but for a join aggregation there is no correct cheaper fallback: every fallback
path computes a join-free quantity. computeSingleAggregation routes a null from
the algebraic path to the generic engine over flat hits; the cardinality path
fell through to a join-free exact distinct count; the metric path masked the
decline as the identity value (e.g. 0). So a guaranteed-blowup join aggregation
would silently return a number answering a different question.
The honest outcome on a guaranteed blowup is the same hard error the scan would
have raised (AlgebraicJoinFanoutExceeded) — the gate just raises it before the
O(left x fanout) scan instead of after, as a fail-fast optimization with exactly
today's semantics. So the gate now raises that error rather than returning null.
Also closes the adjacent seam the gate exposed: a cardinality(field) carrying a
join now routes exclusively through the distributive HLL fold and raises
UnsupportedAggregation when the fold can't serve it (not provably distributive),
instead of silently falling through to a join-free distinct count over all docs.
The non-join cardinality paths are unchanged.
Tests: the index-level gate test now asserts the fast-fail error; a new
dispatch-level test drives a guaranteed-blowup cardinality-over-join through
computeAlgebraicAggregation and asserts it raises rather than returning a count.
https://claude.ai/code/session_01UVKuQU8S6THBNkSBHoVnzz
…dentity Closes the last seam from the phase-3 review. algebraicDerivedJoinMetricRawAlloc returned null both when the derived-join fold was empty (no matched pairs -> the identity value, e.g. sum 0) and when it declined (not provably distributive, or no tensor program could be built). The metric dispatch rendered either null as the identity, so a metric-over-join the fold couldn't serve was silently reported as 0 instead of failing — the same class of silently-wrong join-free answer the gate fix removed. The scan already distinguishes the two: an empty join scans to `[]`, a decline scans to null. So the helper now raises UnsupportedAggregation on the null (decline) and still returns the identity for `[]` (empty). All four callers take the result through `try` and already raise UnsupportedAggregation for the adjacent no-fold case, so erroring on decline is consistent; the empty-join identity path is unchanged. A guaranteed fanout blowup still arrives as the more specific AlgebraicJoinFanoutExceeded via the same try. Test: a sum over a join with zero matched pairs returns the identity (no error, non-null result), guarding the empty-vs-declined distinction. https://claude.ai/code/session_01UVKuQU8S6THBNkSBHoVnzz
…teardown race The in-memory backend's persistent treap shares nodes across snapshots and is documented for concurrent readers + a writer, but Node.refs was a plain usize mutated off any lock. A writer's txn.put traverses and retains shared path nodes during an unlocked insert, while a concurrent reader (e.g. a derived-replay worker's txn.abort during teardown) releaseNodes the same shared nodes. The non-atomic counter loses updates under that interleaving and underflows on the next decrement, surfacing as the flaky `present.refs -= 1` "integer overflow" panic in background worker threads. retain/releaseNode now use atomic RMW on the refcount: monotonic add on retain (the caller already holds a live reference, so no ordering vs node contents is needed), and an acq_rel sub on release so the thread that wins the final decrement observes all other threads' uses before it frees. Node contents are immutable after makeNode (copy-on-write), and RcState.refs is already mutated only under the backend mutex, so the node counter was the sole unsynchronized shared mutation — this is the complete, minimal fix. Added a multithreaded regression test (8 threads churning snapshot/retain/ release on a shared tree). It deterministically reproduced the original panic on the pre-fix code (integer overflow, 4/4 runs) and passes cleanly with the fix (3/3); the testing allocator's double-free/leak checks also guard a refcount that drops a node early. Full db-test and public-api-parity suites stay green. https://claude.ai/code/session_01UVKuQU8S6THBNkSBHoVnzz
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.
Cardinality, range, and histogram queries funnel through
cardinalityDocMatchesConstraints, a linear scan over a doc-id slice that
is invoked once per scanned index entry. For histogram/range children the
full field prefix is re-scanned per bucket and tested against that
bucket's doc-id list, making the scan O(entries * constraints) -- i.e.
quadratic in the document count. These were the only algebraic queries
measured slower than a plain document scan in the algebraic benchmark.
Introduce DocIdConstraintSet, a borrowed-key hash-set built once per scan,
and replace the linear membership test at all eight cardinality
scan/collect sites. Each membership test becomes O(1) and the scan is a
single linear pass again. Results are unchanged (the benchmark
cross-checks algebraic output against document-scan checksums).
https://claude.ai/code/session_01UVKuQU8S6THBNkSBHoVnzz