Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 23 additions & 7 deletions src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -645,9 +645,19 @@ impl Entry {

/// The entry's write lock, or `None` if a delete beat the caller to
/// it — a handle that predates the removal must not touch the files
/// the delete just removed, let alone recreate them. Every
/// post-lookup lock acquisition goes through here so no path can
/// forget the tombstone.
/// the delete just removed, let alone recreate them. This and its
/// read-only sibling [`Entry::read_unless_deleted`] are the two
/// sanctioned ways to fence a lookup against a concurrent delete;
/// most post-lookup call sites go through one of them. The rest
/// take `self.inner` directly and are responsible for their own
/// tombstone-safety, which in every current caller comes from one
/// of: matching `Slot` inline and handling `Deleted` itself (e.g.
/// `read_context`, `describe_entry`, the eviction sweep), being the
/// side that plants the tombstone in the first place (`delete`,
/// replica deregistration), holding an entry the caller already
/// owns exclusively (a rename's freshly-inserted destination, a
/// drained rename source), or running at boot before the listener
/// binds (`preload_pinned`).
#[allow(clippy::readonly_write_lock)] // some callers lock purely for exclusion
fn lock_unless_deleted(&self) -> Option<parking_lot::RwLockWriteGuard<'_, EntryInner>> {
let guard = self.inner.write();
Expand Down Expand Up @@ -922,7 +932,8 @@ pub enum PutSchemaError {
/// [`schema::SCHEMA_TYPE_LABEL`] — ADR 0009 §6.3 guard 3's
/// migration-boundary counterpart. Carries the offending alias so
/// the caller can name it and instruct a rename, mirroring
/// `EMPTY_SOURCE`'s own collision wording (`src/export.rs:315`).
/// `EMPTY_SOURCE`'s own collision wording in [`crate::export::render`]'s
/// reserved-source-id refusals.
ReservedAlias(String),
/// Loading the context to inspect its live label-alias table
/// failed — mirrors `update_meta`'s own `ensure_hot` failure arm.
Expand Down Expand Up @@ -2141,9 +2152,14 @@ struct StateInner {

/// An LRU-bounded map of cue → embedding: an LLM client repeats query
/// wording, and recency (not insertion order) is what predicts the next
/// hit. At the cap it holds ~12 MB of vectors. Recency is tracked by a
/// counter dedicated to this cache rather than `AppState::clock`
/// (documented for a different purpose) to keep the two concerns apart.
/// hit. `CAP` bounds vector *count*, not bytes: at the cap it holds
/// `CAP` × dimensions × 4 bytes of vectors — ~12 MiB at 3072
/// dimensions (the largest first-party model), ~6 MiB at 1536 — plus
/// the cue strings themselves and `HashMap` overhead; dimension comes
/// from whatever the embedding provider returns, not a fixed constant.
/// Recency is tracked by a counter dedicated to this cache rather than
/// `AppState::clock` (documented for a different purpose) to keep the
/// two concerns apart.
#[derive(Default)]
struct CueCache {
vectors: HashMap<String, (Arc<Vec<f32>>, u64)>,
Expand Down
48 changes: 33 additions & 15 deletions src/registry/concurrency.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ use taguru::deadline::Deadline;
/// into the shared result once at the end, so contention is limited to
/// the queue itself; results come back in arrival order, not input
/// order — callers that need input order carry an index through `T`/`R`
/// and sort afterward.
/// and sort afterward. A panic inside `f` is not caught: `thread::scope`
/// re-raises it after every worker joins, so the caller gets the unwind
/// instead of the partial `results` this function was building.
pub(crate) fn parallel_map<T, R>(items: Vec<T>, workers: usize, f: impl Fn(T) -> R + Sync) -> Vec<R>
where
T: Send,
Expand Down Expand Up @@ -47,14 +49,17 @@ where
/// indices in order. Unlike `parallel_map` above — arrival-order
/// results, no notion of failure — this preserves input order and
/// stops claiming new work once a chunk's failure has been recorded.
/// Every caller (`extract_chunks_concurrently` in src/extract.rs, and
/// `embed_stale` / `refresh_passage_embeddings` below) needs both: an
/// input-order-preserving result to fold correctly, and best-effort
/// early termination once a failure surfaces, so a batch that is going
/// to fail stops enlisting new work. Fold-on-failure semantics differ
/// per caller (fail the whole batch vs. keep whatever succeeded), so
/// the fold itself is left to them — this returns the raw, unfolded
/// per-index outcome.
/// Every caller (`Run::extract_chunks_concurrently` in
/// `src/extract/run.rs`, and `embed_stale` /
/// `refresh_passage_embeddings` in `src/registry/embeddings.rs`) needs
/// both: an input-order-preserving result to fold correctly, and
/// best-effort early termination once a failure surfaces, so a batch
/// that is going to fail stops enlisting new work. Fold-on-failure
/// semantics differ per caller (fail the whole batch vs. keep whatever
/// succeeded), so the fold itself is left to them — this returns the
/// raw, unfolded per-index outcome. As with `parallel_map`, a panic
/// inside `f` unwinds through `thread::scope` after every worker joins;
/// no caller gets to see a partial `results` in that case.
///
/// `next` and `first_failure` are independent atomics; SeqCst on both
/// is required so a worker claiming an index past a just-recorded
Expand All @@ -66,7 +71,12 @@ where
/// recorded. Their count is NOT bounded by `workers` — a failure slow
/// to surface lets the other workers complete arbitrarily many later
/// indices first — so callers fold on the prefix, never on a count of
/// what landed past the failure.
/// what landed past the failure. The `Err(String)` inside a `Some` slot
/// does not distinguish a genuine per-item failure from a caller that
/// folded a deadline expiry or a slot-queue timeout into the same
/// `String` channel (both `embed_stale` and `refresh_passage_embeddings`
/// do exactly that) — callers that need to tell those apart must encode
/// it themselves before calling in, not after getting the result back.
pub(crate) fn dispatch_chunks_concurrently<C: Sync, R: Send + Sync>(
chunks: &[C],
workers: usize,
Expand Down Expand Up @@ -115,9 +125,13 @@ const SLOT_POLL: Duration = Duration::from_millis(50);
/// `dispatch_chunks_concurrently` fan-out inside one context's own
/// refresh — nested, those two ceilings would multiply into P × P
/// concurrent provider calls. Every refresh chunk instead acquires a
/// permit here around its provider call, so no matter how many
/// threads across how many contexts attempt one at once, at most
/// `embed_parallel` are ever in flight process-wide.
/// permit here before its provider call, so no matter how many threads
/// across how many contexts attempt one at once, at most `embed_parallel`
/// are ever in flight process-wide. The permit is taken before the
/// circuit breaker's own refusal check, so a chunk that the breaker
/// turns away still consumes — and promptly frees — a slot; the
/// ceiling this bounds is "concurrent attempts", not "concurrent
/// in-flight calls".
///
/// Two properties `Mutex<usize>` + `Condvar::notify_one` (the
/// original shape) did not have, per issue #563 item 4: a bound on
Expand Down Expand Up @@ -253,8 +267,12 @@ pub(crate) struct Acquisition<'a> {
pub(crate) queued: bool,
}

/// Returns its permit to [`Semaphore`] on drop — held across exactly
/// the provider call, never longer, so a panic mid-call still frees it.
/// Returns its permit to [`Semaphore`] on drop — a panic mid-call still
/// frees it. Held from just before the circuit-breaker refusal check
/// through the provider call (or, if the breaker is open, through the
/// refusal itself): a breaker-open chunk never reaches the provider,
/// but it still occupies — briefly — the slot that bounds "concurrent
/// attempts at this choke point", not "concurrent provider calls".
pub(crate) struct SemaphorePermit<'a> {
semaphore: &'a Semaphore,
}
Expand Down
8 changes: 7 additions & 1 deletion src/registry/engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -591,7 +591,13 @@ impl AppState {
/// resident estimate fits the budget. `except` (the context just
/// used) is never evicted, so a single oversized context cannot
/// thrash. Dirty contexts are persisted before eviction; if that
/// save fails they stay resident rather than losing writes.
/// save fails they stay resident rather than losing writes. When
/// `except` alone is bigger than the whole budget — or an eligible
/// dirty candidate's save keeps failing and it stays resident —
/// eviction runs out of candidates before the estimate ever fits:
/// the sweep marks itself saturated for that `except` (see the gate
/// comment below) rather than spinning, and a later call with a
/// different `except` still evicts promptly.
pub(crate) fn enforce_budget(&self, except: &str) {
// Cheap gate in front of the O(contexts) sweep below, off the
// every-64th forced-sweep beat: skip it when EITHER the atomic
Expand Down
14 changes: 9 additions & 5 deletions src/registry/lifecycle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,15 @@ impl AppState {
/// existence (and description) survives a crash from the moment the
/// create call returns. A persistence failure fails the create.
///
/// The registry lock is NOT held across the disk work (up to eight
/// unlinks plus save_files' fsyncs — seconds on slow storage,
/// behind which every operation on every context would otherwise
/// stall). The name is reserved in `pending.creates` under the
/// registry guard, the files are written unlocked, and the entry
/// The registry lock is NOT held across the disk work (an unlink
/// attempt for each candidate path `sweep_stale_stem_files` removes
/// — the stem's on-disk family minus `meta_path`, which
/// `save_files` overwrites instead — plus one per stale rename or
/// import marker it finds, plus save_files' fsyncs — seconds on
/// slow storage, behind which every operation on every context
/// would otherwise stall). The name is reserved in
/// `pending.creates` under the registry guard, the files are
/// written unlocked, and the entry
/// lands in a second critical section — the create twin of
/// delete's `pending.deletes` choreography.
pub fn create(&self, name: &str, meta: ContextMeta) -> Result<(), CreateError> {
Expand Down