fix(engine): #872 lock-free memtable byte accounting for the resource sampler - #883
Open
MavenRain wants to merge 1 commit into
Open
fix(engine): #872 lock-free memtable byte accounting for the resource sampler#883MavenRain wants to merge 1 commit into
MavenRain wants to merge 1 commit into
Conversation
The resource sampler thread ("xerj-memtable-sampler", spawned in
engine.rs spawn_resource_sampler) calls total_memtable_bytes() every
SAMPLE_INTERVAL_MS=100 ms. That summed ShardedFtsMemtable::size_bytes()
per index, and size_bytes() read-locked every one of the 16 shards. At
464 idle indices that is 464 x 16 x 10 = ~74,240 shard lock
acquisitions per second of pure idle overhead, and any held shard write
lock stalled the sampler for the duration of the hold.
Fix: every FtsMemtable shard of one index now shares a single
Arc<AtomicUsize> aggregate with its ShardedFtsMemtable. All 11
total_bytes write sites mirror the observed delta into the atomic:
insert_analyzed, insert_raw_bytes_with_seq, insert_raw_bytes_fresh,
insert_pretokenized_inner, insert_pretokenized_arc, remove (min-clamped
like total_bytes itself), the four full-reset drains
(drain_with_sources_raw, drain_raw, drain, drain_with_sources), and
drain_shard_inner. Mirroring at the FtsMemtable level keeps the
accounting correct on the with_shard_mut raw-insert bypass that turbo
ingest uses. ShardedFtsMemtable::size_bytes() is now one Relaxed load
and takes no locks. A standalone FtsMemtable (new / with_registry)
keeps its own private counter and its size_bytes() is unchanged.
Note: the issue sketch names AtomicU64; the implementation uses
AtomicUsize because total_bytes is usize. Same width on 64-bit targets,
and it avoids a cast at every mirror site.
Tests added: size_bytes_does_not_take_shard_locks (size_bytes returns
while a shard write lock is held; the holder self-releases on a bound
so a lock-summing regression fails instead of hanging) and
aggregate_bytes_matches_full_recount_after_every_mutator (the aggregate
equals a full per-shard recount after every mutator, ending at zero).
Gates run: cargo fmt --check, build, clippy --no-deps, the new tests,
the governor breaker tests (memtable_budget_trips_on_refresh,
build_from_default_config_trips_nothing), mutation-confirm x2 (the
lock-summing revert and a dropped mirror are both caught), and the
ES-compat YAML suite.
Closes xerj-org#872
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.
Summary
Fixes #872. The resource sampler's per-index
size_bytes()no longerread-locks all 16 memtable shards; it is now one
Relaxedload of ashared
Arc<AtomicUsize>aggregate that every shard maintainsincrementally at each
total_byteswrite.Root cause
Sampler thread B (
"xerj-memtable-sampler", spawned inengine.rs:3820spawn_resource_sampler, comment hunk atengine.rs:3878-3885) callstotal_memtable_bytes()everySAMPLE_INTERVAL_MS = 100ms. That summedShardedFtsMemtable::size_bytes()for every index, and the old body(
memtable.rs, pre-fix line ~841) wasself.shards.iter().map(|s| s.read().size_bytes()).sum(): a read lockon each of the 16 shards. At 464 idle indices that is
464 x 16 x 10 = ~74,240 lock acquisitions per second of pure idle
overhead, and a held shard write lock stalled the sampler outright.
Fix design
Arc<AtomicUsize>per index, created inwith_registry_and_shards(memtable.rs:789-805) and cloned intoevery
FtsMemtableshard.ShardedFtsMemtable::size_bytes()(
memtable.rs:854) is a singleRelaxedload, no locks.FtsMemtable, not in the shardedwrapper, because
with_shard_muthands callers a raw&mut FtsMemtable: turbo ingest callsinsert_raw_bytes_freshdirectly through it, so wrapper-level accounting would miss that
bypass entirely.
total_byteswrite sites mirror the observed delta(
insert_analyzed,insert_raw_bytes_with_seq,insert_raw_bytes_fresh,insert_pretokenized_inner,insert_pretokenized_arc, min-clampedremove, the four full-resetdrains, and
drain_shard_inner). Observed-delta mirroring means areset subtracts exactly what the shard still carries, so nested or
repeated resets contribute 0 and drains stay idempotent-safe.
AtomicUsize, not the issue'sAtomicU64:total_bytesisusize,the width is the same on 64-bit targets, and it avoids a cast at
every mirror site. Standalone
FtsMemtable::new()/with_registry()get a private counter;
FtsMemtable::size_bytes()is unchanged.Breaker unaffected
The governor admission path reads its own relaxed atomics
(
governor.rs:120-143check_ingest_admission) and is untouched.memtable_budget_trips_on_refreshandbuild_from_default_config_trips_nothingstay green in the gatebattery.
Tests
size_bytes_does_not_take_shard_locks: provessize_bytes()returnswhile a shard write lock is held. The holder self-releases on a 4 s
bound, so a lock-summing regression fails the 2 s receive timeout
cleanly instead of deadlocking the test.
aggregate_bytes_matches_full_recount_after_every_mutator: provesthe aggregate equals a full per-shard recount after every mutator
(inserts of all four kinds, the
with_shard_mutraw bypass, removeof an existing and a missing doc, every drain variant), ending at 0.
size_bytes()to the lock-summing body;the lock test kills it. M2 zeroes the
fetch_add(size, ...)mirrors;the recount test kills it. Both restores verified byte-clean against
the staged fix.
Verification
Verified: the gate battery results in
~/Documents/xerj-872-gate.log:cargo fmt --check, build,clippy --no-deps, the two new tests, the two governor tests,mutation-confirm x2 with clean restores, and the ES-compat YAML suite.
Assumed (not measured): the production sampler CPU reduction at 464
indices. The ~74,240 locks/s figure is arithmetic from the constants in
#872, not a live-cluster measurement.
Closes #872