From bbc7ad9c43c947ef9615ed0a0c8f24750b209099 Mon Sep 17 00:00:00 2001 From: Zack Jackson <25274700+ScriptedAlchemy@users.noreply.github.com> Date: Sat, 7 Nov 2026 23:18:29 -0800 Subject: [PATCH 01/23] fix(retention): queue maintenance after repeated writer refusals Retention fast probes never join the daemon writer queue. Under sustained store-scoped writers, even a fair RwLock cannot admit the maintenance pass. After two per-store nonqueued refusals, join the fair writer queue with cooperative shutdown cancellation; reset/prune the counters on admission. Exercise bounded deferrals, progress with competing store writers, and cancellation of the queued acquisition in native Rust tests. Refs #4616 --- crates/tracedecay/src/daemon/maintenance.rs | 190 +++++++++++++++++++- 1 file changed, 185 insertions(+), 5 deletions(-) diff --git a/crates/tracedecay/src/daemon/maintenance.rs b/crates/tracedecay/src/daemon/maintenance.rs index ea2070195f9..4db847bc33f 100644 --- a/crates/tracedecay/src/daemon/maintenance.rs +++ b/crates/tracedecay/src/daemon/maintenance.rs @@ -1,4 +1,5 @@ -use std::collections::BTreeSet; +use std::collections::{BTreeMap, BTreeSet}; +use std::future::Future; use std::path::{Path, PathBuf}; use std::sync::Arc; use std::time::{Duration, Instant}; @@ -32,6 +33,32 @@ use tracedecay_runtime_core::logging::log_daemon_event; use tracedecay_runtime_core::native_runtime::{self, NativeTask, TimeoutError}; const MAINTENANCE_STORE_PAGE_LIMIT: usize = 8; const REGISTERED_PROJECT_PAGE_LIMIT: usize = 64; +/// Fast probes preserve low-latency work. After two refusals, join Tokio's +/// fair writer queue so a stream of store-scoped readers cannot starve GC. +const MAINTENANCE_WRITER_FAST_DEFERRALS: u8 = 2; + +async fn admit_retention_writer( + administration: &StoreAdministration, + cancellation: &tracedecay_runtime_core::cancellation::CancellationToken, + prior_deferrals: u8, + operation: Operation, +) -> Option +where + Operation: FnOnce() -> OperationFuture, + OperationFuture: Future, +{ + if prior_deferrals < MAINTENANCE_WRITER_FAST_DEFERRALS { + administration.try_with_writer(operation).await + } else { + // A queued writer blocks later store readers behind it. Cancellation + // drops the acquisition future and removes the queue entry on shutdown. + tokio::select! { + biased; + () = cancellation.cancelled() => None, + outcome = administration.with_writer(operation) => Some(outcome), + } + } +} /// Every registered project whose profile shard exists on disk, keyed for the /// tick's work list. @@ -618,6 +645,8 @@ pub(super) struct MaintenanceCoordinator { /// store is starved when the mounted set exceeds `MAINTENANCE_STORE_PAGE_LIMIT`. store_cursor: Arc>>, store_continuations: Arc>, + /// Per-store writer admission refusals, bounded and reset on a real admission. + writer_deferrals: Arc>>, /// Instant of the last branch-store GC pass that succeeded for every /// mounted project. `None` keeps the daily cadence retry-eligible. last_branch_gc: Arc>>, @@ -639,6 +668,7 @@ impl Default for MaintenanceCoordinator { metrics: Arc::new(Mutex::new(MaintenanceMetricsV1::default())), store_cursor: Arc::new(Mutex::new(None)), store_continuations: Arc::new(Mutex::new(StoreMaintenanceContinuations::default())), + writer_deferrals: Arc::new(Mutex::new(BTreeMap::new())), last_branch_gc: Arc::new(Mutex::new(None)), resident_memory_sampler: Arc::new(Mutex::new(None)), resident_memory_log: Arc::new(std::sync::Mutex::new( @@ -1060,6 +1090,10 @@ impl MaintenanceCoordinator { work.sort_by(|left, right| left.0.cmp(&right.0)); let keys = work.iter().map(|(key, _)| key.clone()).collect::>(); self.store_continuations.lock().await.retain_stores(&keys); + self.writer_deferrals + .lock() + .await + .retain(|key, _| keys.binary_search(key).is_ok()); let after = self.store_cursor.lock().await.clone(); let (window, _) = select_store_window(&keys, after.as_deref(), MAINTENANCE_STORE_PAGE_LIMIT); @@ -1124,8 +1158,18 @@ impl MaintenanceCoordinator { outcome = MaintenanceTickOutcome::Retry; break; } - let admitted = administration - .try_with_writer(|| async { + let prior_deferrals = self + .writer_deferrals + .lock() + .await + .get(&keys[index]) + .copied() + .unwrap_or(0); + let admitted = admit_retention_writer( + administration, + &self.cancellation, + prior_deferrals, + || async { let Ok(_clients) = administration.maintenance_store_clients().try_lock() else { return MaintenanceTickOutcome::Retry; }; @@ -1177,8 +1221,17 @@ impl MaintenanceCoordinator { .await } } - }) - .await; + }, + ) + .await; + { + let mut deferrals = self.writer_deferrals.lock().await; + if admitted.is_some() { + deferrals.remove(&keys[index]); + } else if !self.cancellation.is_cancelled() { + deferrals.insert(keys[index].clone(), prior_deferrals.saturating_add(1)); + } + } attempted = attempted.saturating_add(1); match admitted { Some(unit_outcome) => { @@ -1541,6 +1594,133 @@ mod tests { use tracedecay_maintenance::telemetry::StoreTelemetrySamplingRegistry; use tracedecay_maintenance::tick::{MaintenanceContinuation, MaintenanceTickOutcome}; + /// Hold an unrelated store writer across two failed fast probes. The third + /// retention attempt must queue behind it and complete even as more store + /// writers arrive, rather than repeatedly taking non-queued snapshots. + #[tokio::test] + async fn retention_queues_after_bounded_writer_deferrals() { + use crate::daemon::branch_admin::{StoreAdministration, StoreWriterClass, WriterScope}; + use tracedecay_runtime_core::cancellation::CancellationToken; + + let administration = StoreAdministration::default(); + let (entered_tx, entered_rx) = tokio::sync::oneshot::channel(); + let (release_tx, release_rx) = tokio::sync::oneshot::channel(); + let holding_admin = administration.clone(); + let holder = tokio::spawn(async move { + holding_admin + .with_writer_in( + WriterScope::store("/unrelated-store", StoreWriterClass::Content), + move || async move { + entered_tx.send(()).expect("store writer entered"); + release_rx.await.expect("store writer released"); + }, + ) + .await; + }); + entered_rx.await.expect("writer acquired its shared daemon gate"); + let cancellation = CancellationToken::new(); + for refused in 0..super::MAINTENANCE_WRITER_FAST_DEFERRALS { + assert_eq!( + super::admit_retention_writer(&administration, &cancellation, refused, || async { + 41_u8 + }) + .await, + None, + "a store writer blocks the non-queued daemon probe" + ); + } + + let queued = super::admit_retention_writer( + &administration, + &cancellation, + super::MAINTENANCE_WRITER_FAST_DEFERRALS, + || async { 42_u8 }, + ); + tokio::pin!(queued); + assert!( + tokio::time::timeout(Duration::from_millis(30), &mut queued) + .await + .is_err(), + "the maintenance operation waits behind an existing writer" + ); + + let competing_admin = administration.clone(); + let completed_writers = Arc::new(AtomicUsize::new(0)); + let completed = Arc::clone(&completed_writers); + let competitors = tokio::spawn(async move { + for _ in 0..64 { + competing_admin + .with_writer_in( + WriterScope::store("/another-store", StoreWriterClass::Content), + || { + let completed = Arc::clone(&completed); + async move { + completed.fetch_add(1, Ordering::SeqCst); + tokio::task::yield_now().await; + } + }, + ) + .await; + } + }); + release_tx.send(()).expect("release the original writer"); + assert_eq!( + tokio::time::timeout(Duration::from_secs(5), &mut queued) + .await + .expect("queued retention is admitted despite competing writers"), + Some(42) + ); + holder.await.expect("first writer finishes"); + competitors.await.expect("subsequent store writers finish"); + assert_eq!(completed_writers.load(Ordering::SeqCst), 64); + } + + #[tokio::test] + async fn cancelled_queued_retention_releases_its_writer_place() { + use crate::daemon::branch_admin::{StoreAdministration, StoreWriterClass, WriterScope}; + use tracedecay_runtime_core::cancellation::CancellationToken; + + let administration = StoreAdministration::default(); + let (entered_tx, entered_rx) = tokio::sync::oneshot::channel(); + let (release_tx, release_rx) = tokio::sync::oneshot::channel(); + let holding_admin = administration.clone(); + let writer = tokio::spawn(async move { + holding_admin + .with_writer_in( + WriterScope::store("/held-store", StoreWriterClass::Owner), + move || async move { + entered_tx.send(()).expect("entered writer"); + release_rx.await.expect("released writer"); + }, + ) + .await; + }); + entered_rx.await.expect("writer holds shared daemon admission"); + let cancellation = CancellationToken::new(); + let waiting = super::admit_retention_writer( + &administration, + &cancellation, + super::MAINTENANCE_WRITER_FAST_DEFERRALS, + || async { 7_u8 }, + ); + tokio::pin!(waiting); + assert!( + tokio::time::timeout(Duration::from_millis(30), &mut waiting) + .await + .is_err() + ); + cancellation.cancel(); + assert_eq!( + tokio::time::timeout(Duration::from_secs(2), &mut waiting) + .await + .expect("cancelled acquisition completes"), + None + ); + release_tx.send(()).expect("release held writer"); + writer.await.expect("held writer exits"); + assert_eq!(administration.try_with_writer(|| async { 9_u8 }).await, Some(9)); + } + #[tokio::test] async fn mounted_store_identity_tracks_native_removal_outcomes() { let temporary = tempfile::tempdir().expect("store identity fixture root"); From c13a88f0a72f9b2e1db3b3ab7ae102e716ce4ab8 Mon Sep 17 00:00:00 2001 From: Zack Jackson <25274700+ScriptedAlchemy@users.noreply.github.com> Date: Sat, 7 Nov 2026 23:24:14 -0800 Subject: [PATCH 02/23] test(retention): reclaim six-generation backlog after queued writer Drive the existing registered-store production maintenance tick through two real writer refusals and then a queued admission, with six durable code-index generations and a concurrent unrelated store writer. Verify that the writer release enables the same tick to collect five superseded generations. Refs #4616 --- crates/tracedecay/src/daemon/maintenance.rs | 54 ++++++++++++++++++++- 1 file changed, 53 insertions(+), 1 deletion(-) diff --git a/crates/tracedecay/src/daemon/maintenance.rs b/crates/tracedecay/src/daemon/maintenance.rs index 4db847bc33f..70b51efe068 100644 --- a/crates/tracedecay/src/daemon/maintenance.rs +++ b/crates/tracedecay/src/daemon/maintenance.rs @@ -1986,6 +1986,49 @@ mod tests { ); drop(release); + // A long-lived, unrelated content writer holds the daemon read gate. + // Two real retention ticks must defer, and the next must queue rather + // than missing the same writer again. The fixture's six on-disk + // generations make forward progress observable through actual GC. + let (writer_started, writer_entered) = tokio::sync::oneshot::channel(); + let (release_writer, writer_released) = tokio::sync::oneshot::channel(); + let holding_admin = administration.clone(); + let writer = NativeTask::spawn(async move { + holding_admin + .with_writer_in( + crate::daemon::branch_admin::WriterScope::store( + "/unrelated-project", + crate::daemon::branch_admin::StoreWriterClass::Content, + ), + move || async move { + writer_started.send(()).expect("writer admitted"); + writer_released.await.expect("writer released"); + }, + ) + .await; + }) + .expect("start competing store writer"); + writer_entered.await.expect("unrelated store writer owns shared gate"); + for _ in 0..super::MAINTENANCE_WRITER_FAST_DEFERRALS { + let deferred = coordinator + .run_tick( + &profile_root, + &profile_database, + &administration, + &schedulers, + &retention, + branch_gc, + None, + ) + .await; + assert_eq!(deferred, MaintenanceTickOutcome::Retry); + assert_eq!(generation_files(), 6, "fast probe must not collect under a writer"); + } + + let release_after_queue = std::thread::spawn(move || { + std::thread::sleep(Duration::from_millis(100)); + release_writer.send(()).expect("release competing writer"); + }); coordinator .run_tick( &profile_root, @@ -1997,7 +2040,16 @@ mod tests { None, ) .await; - assert_eq!(generation_files(), 1, "the full tick collects the backlog"); + release_after_queue.join().expect("writer release thread"); + assert!( + matches!(writer.wait().await, native_runtime::NativeTaskOutcome::Completed(())), + "competing writer must complete normally" + ); + assert_eq!( + generation_files(), + 1, + "queued retention reclaims five verified superseded generations after writer release" + ); let continuation = coordinator .run_tick( From 61fd4684c48fb553485a7c29b4d1a20d005ed25b Mon Sep 17 00:00:00 2001 From: Zack Jackson <25274700+ScriptedAlchemy@users.noreply.github.com> Date: Sat, 7 Nov 2026 23:26:29 -0800 Subject: [PATCH 03/23] fix(retention): cancel only queued writer admission A shutdown cancellation during an already-admitted retention operation must not drop journaled work that can still own blocking store clients. Separate the cancellable writer wait from the critical section, preserving cooperative operation completion and the existing writer lease after admission. Add a native Rust test that cancels after admission and requires the critical section to complete and release the gate. Refs #4616 --- crates/tracedecay/src/daemon/branch_admin.rs | 24 ++++++++++ crates/tracedecay/src/daemon/maintenance.rs | 49 +++++++++++++++++--- 2 files changed, 66 insertions(+), 7 deletions(-) diff --git a/crates/tracedecay/src/daemon/branch_admin.rs b/crates/tracedecay/src/daemon/branch_admin.rs index d0cae8b43ec..e521ed4b89b 100644 --- a/crates/tracedecay/src/daemon/branch_admin.rs +++ b/crates/tracedecay/src/daemon/branch_admin.rs @@ -1923,6 +1923,30 @@ impl StoreAdministration { operation().await } + /// Cancellation applies only while queued for daemon-wide writer admission. + /// Once the guard is acquired, the operation must finish cooperatively: + /// dropping an in-flight journal/retention future would abandon physical + /// blocking work whose client survives the daemon's shutdown join. + pub(super) async fn with_writer_cancellable_acquisition( + &self, + cancellation: &tracedecay_runtime_core::cancellation::CancellationToken, + operation: Operation, + ) -> Option + where + Operation: FnOnce() -> OperationFuture, + OperationFuture: Future, + { + let scope = WriterScope::Daemon; + let _writer = tokio::select! { + biased; + () = cancellation.cancelled() => return None, + writer = crate::daemon::core_admission::park_admission( + self.gate.acquire(&scope) + ) => writer, + }; + Some(operation().await) + } + pub(super) async fn try_with_writer( &self, operation: Operation, diff --git a/crates/tracedecay/src/daemon/maintenance.rs b/crates/tracedecay/src/daemon/maintenance.rs index 70b51efe068..284564fef3c 100644 --- a/crates/tracedecay/src/daemon/maintenance.rs +++ b/crates/tracedecay/src/daemon/maintenance.rs @@ -50,13 +50,11 @@ where if prior_deferrals < MAINTENANCE_WRITER_FAST_DEFERRALS { administration.try_with_writer(operation).await } else { - // A queued writer blocks later store readers behind it. Cancellation - // drops the acquisition future and removes the queue entry on shutdown. - tokio::select! { - biased; - () = cancellation.cancelled() => None, - outcome = administration.with_writer(operation) => Some(outcome), - } + // Only cancel the wait: once admitted, journal/retention work must + // finish cooperatively rather than be dropped at the writer boundary. + administration + .with_writer_cancellable_acquisition(cancellation, operation) + .await } } @@ -1721,6 +1719,43 @@ mod tests { assert_eq!(administration.try_with_writer(|| async { 9_u8 }).await, Some(9)); } + /// Cancellation after writer admission must not drop a running retention + /// operation: its blocking/journaled work is responsible for cooperating. + #[tokio::test] + async fn cancellation_after_writer_admission_preserves_running_work() { + use crate::daemon::branch_admin::StoreAdministration; + use tracedecay_runtime_core::cancellation::CancellationToken; + + let administration = StoreAdministration::default(); + let cancellation = CancellationToken::new(); + let working = administration.clone(); + let stopping = cancellation.clone(); + let (entered_tx, entered_rx) = tokio::sync::oneshot::channel(); + let (release_tx, release_rx) = tokio::sync::oneshot::channel(); + let work = tokio::spawn(async move { + super::admit_retention_writer( + &working, + &stopping, + super::MAINTENANCE_WRITER_FAST_DEFERRALS, + move || async move { + entered_tx.send(()).expect("retention entered its critical section"); + release_rx.await.expect("retention work released"); + 43_u8 + }, + ) + .await + }); + entered_rx.await.expect("writer admission succeeded"); + cancellation.cancel(); + release_tx.send(()).expect("finish admitted work"); + assert_eq!( + work.await.expect("retention task joins"), + Some(43), + "shutdown cancellation cannot drop journaled work already admitted" + ); + assert_eq!(administration.try_with_writer(|| async { 44_u8 }).await, Some(44)); + } + #[tokio::test] async fn mounted_store_identity_tracks_native_removal_outcomes() { let temporary = tempfile::tempdir().expect("store identity fixture root"); From c41076a0fdf3aa290d40ec32e7b7532e7be32a1c Mon Sep 17 00:00:00 2001 From: Zack Jackson <25274700+ScriptedAlchemy@users.noreply.github.com> Date: Sat, 7 Nov 2026 23:59:46 -0800 Subject: [PATCH 04/23] test(retention): prove fair admission during ongoing writer churn The old finite competitor burst could finish before maintenance acquired the daemon gate. Keep unrelated store writers active through the queued retention admission, asserting churn both before and afterward and only stopping writers after the admission proof. This remains a focused gate regression; sealed-artifact N>3 acceptance requires its own native test. Refs #4616 --- crates/tracedecay/src/daemon/maintenance.rs | 29 ++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/crates/tracedecay/src/daemon/maintenance.rs b/crates/tracedecay/src/daemon/maintenance.rs index 284564fef3c..be2f9635772 100644 --- a/crates/tracedecay/src/daemon/maintenance.rs +++ b/crates/tracedecay/src/daemon/maintenance.rs @@ -1642,11 +1642,16 @@ mod tests { "the maintenance operation waits behind an existing writer" ); + // Keep unrelated content writers churning until *after* the queued + // retention operation completes. A finite burst that happens to stop + // before retention runs would not prove progress under sustained load. let competing_admin = administration.clone(); let completed_writers = Arc::new(AtomicUsize::new(0)); let completed = Arc::clone(&completed_writers); + let stop_churn = Arc::new(std::sync::atomic::AtomicBool::new(false)); + let stop = Arc::clone(&stop_churn); let competitors = tokio::spawn(async move { - for _ in 0..64 { + while !stop.load(Ordering::SeqCst) { competing_admin .with_writer_in( WriterScope::store("/another-store", StoreWriterClass::Content), @@ -1661,6 +1666,14 @@ mod tests { .await; } }); + tokio::time::timeout(Duration::from_secs(5), async { + while completed_writers.load(Ordering::SeqCst) < 4 { + tokio::task::yield_now().await; + } + }) + .await + .expect("unrelated writers churn before original writer releases"); + let before_release = completed_writers.load(Ordering::SeqCst); release_tx.send(()).expect("release the original writer"); assert_eq!( tokio::time::timeout(Duration::from_secs(5), &mut queued) @@ -1668,9 +1681,19 @@ mod tests { .expect("queued retention is admitted despite competing writers"), Some(42) ); + // A succeeding write proves the churn remained live *through* the + // retention admission; it is stopped only after that observation. + tokio::time::timeout(Duration::from_secs(5), async { + while completed_writers.load(Ordering::SeqCst) <= before_release { + tokio::task::yield_now().await; + } + }) + .await + .expect("unrelated writers resume after maintenance admission"); + stop_churn.store(true, Ordering::SeqCst); holder.await.expect("first writer finishes"); - competitors.await.expect("subsequent store writers finish"); - assert_eq!(completed_writers.load(Ordering::SeqCst), 64); + competitors.await.expect("unrelated writer churn finishes"); + assert!(completed_writers.load(Ordering::SeqCst) > before_release); } #[tokio::test] From 58a76567f9bcbd3b7b6f9a28feb6fb91e8e2e5f5 Mon Sep 17 00:00:00 2001 From: Zack Jackson <25274700+ScriptedAlchemy@users.noreply.github.com> Date: Sun, 8 Nov 2026 00:27:01 -0800 Subject: [PATCH 05/23] fix(retention): honor shutdown and repair admission regressions Refuse already-cancelled retention before either fast or queued admission, without interrupting operations that already own the writer gate. Start ongoing writer churn before queueing exclusive retention. Explicitly poll queue admission and a later store writer instead of waiting for later readers to make progress behind the exclusive waiter. Check that cancelling the waiter admits its follower while the original shared holder is alive. Add cancellation-before-admission coverage and restore the native runtime import required by the real maintenance-tick regression. Refs #4616 Bazel/native execution remains required; no Cargo or CI changes. --- crates/tracedecay/src/daemon/maintenance.rs | 213 ++++++++++++++------ 1 file changed, 155 insertions(+), 58 deletions(-) diff --git a/crates/tracedecay/src/daemon/maintenance.rs b/crates/tracedecay/src/daemon/maintenance.rs index be2f9635772..fa2a8c4965f 100644 --- a/crates/tracedecay/src/daemon/maintenance.rs +++ b/crates/tracedecay/src/daemon/maintenance.rs @@ -47,6 +47,11 @@ where Operation: FnOnce() -> OperationFuture, OperationFuture: Future, { + // Shutdown observed before admission must not start another retention + // unit, even when a fast probe could acquire the writer gate immediately. + if cancellation.is_cancelled() { + return None; + } if prior_deferrals < MAINTENANCE_WRITER_FAST_DEFERRALS { administration.try_with_writer(operation).await } else { @@ -1581,25 +1586,27 @@ pub(super) fn retention_maintenance_enabled( #[cfg(test)] mod tests { + use std::future::{Future, poll_fn}; use std::path::Path; use std::sync::atomic::{AtomicUsize, Ordering}; use std::sync::{Arc, Condvar, Mutex as StdMutex}; + use std::task::Poll; use std::time::Duration; use tokio::sync::Notify; use super::{MaintenanceCoordinator, NativeTask, run_resident_memory_sampler_loop}; + use crate::daemon::branch_admin::{StoreAdministration, StoreWriterClass, WriterScope}; use tracedecay_maintenance::telemetry::StoreTelemetrySamplingRegistry; use tracedecay_maintenance::tick::{MaintenanceContinuation, MaintenanceTickOutcome}; + use tracedecay_runtime_core::cancellation::CancellationToken; + use tracedecay_runtime_core::native_runtime; /// Hold an unrelated store writer across two failed fast probes. The third /// retention attempt must queue behind it and complete even as more store /// writers arrive, rather than repeatedly taking non-queued snapshots. #[tokio::test] async fn retention_queues_after_bounded_writer_deferrals() { - use crate::daemon::branch_admin::{StoreAdministration, StoreWriterClass, WriterScope}; - use tracedecay_runtime_core::cancellation::CancellationToken; - let administration = StoreAdministration::default(); let (entered_tx, entered_rx) = tokio::sync::oneshot::channel(); let (release_tx, release_rx) = tokio::sync::oneshot::channel(); @@ -1615,7 +1622,9 @@ mod tests { ) .await; }); - entered_rx.await.expect("writer acquired its shared daemon gate"); + entered_rx + .await + .expect("writer acquired its shared daemon gate"); let cancellation = CancellationToken::new(); for refused in 0..super::MAINTENANCE_WRITER_FAST_DEFERRALS { assert_eq!( @@ -1628,52 +1637,65 @@ mod tests { ); } - let queued = super::admit_retention_writer( - &administration, - &cancellation, - super::MAINTENANCE_WRITER_FAST_DEFERRALS, - || async { 42_u8 }, - ); - tokio::pin!(queued); - assert!( - tokio::time::timeout(Duration::from_millis(30), &mut queued) - .await - .is_err(), - "the maintenance operation waits behind an existing writer" - ); - - // Keep unrelated content writers churning until *after* the queued - // retention operation completes. A finite burst that happens to stop - // before retention runs would not prove progress under sustained load. + // Establish ongoing writer activity before queueing the exclusive + // maintenance request. A fair queued writer blocks later readers; + // waiting for their progress before releasing the holder deadlocks. let competing_admin = administration.clone(); - let completed_writers = Arc::new(AtomicUsize::new(0)); - let completed = Arc::clone(&completed_writers); - let stop_churn = Arc::new(std::sync::atomic::AtomicBool::new(false)); - let stop = Arc::clone(&stop_churn); + let writer_entries = Arc::new(AtomicUsize::new(0)); + let entries = Arc::clone(&writer_entries); + let stop_churn = CancellationToken::new(); + let stop = stop_churn.clone(); let competitors = tokio::spawn(async move { - while !stop.load(Ordering::SeqCst) { + while !stop.is_cancelled() { competing_admin .with_writer_in( WriterScope::store("/another-store", StoreWriterClass::Content), - || { - let completed = Arc::clone(&completed); - async move { - completed.fetch_add(1, Ordering::SeqCst); - tokio::task::yield_now().await; - } + || async { + entries.fetch_add(1, Ordering::SeqCst); + tokio::task::yield_now().await; }, ) .await; } }); tokio::time::timeout(Duration::from_secs(5), async { - while completed_writers.load(Ordering::SeqCst) < 4 { + while writer_entries.load(Ordering::SeqCst) < 4 { tokio::task::yield_now().await; } }) .await - .expect("unrelated writers churn before original writer releases"); - let before_release = completed_writers.load(Ordering::SeqCst); + .expect("unrelated writers churn before retention queues"); + + let queued = super::admit_retention_writer( + &administration, + &cancellation, + super::MAINTENANCE_WRITER_FAST_DEFERRALS, + || async { 42_u8 }, + ); + tokio::pin!(queued); + poll_fn(|cx| { + assert!( + queued.as_mut().poll(cx).is_pending(), + "the retention request must queue behind the held writer" + ); + Poll::Ready(()) + }) + .await; + let later_writer = administration.with_writer_in( + WriterScope::store("/later-store", StoreWriterClass::Content), + || async { 43_u8 }, + ); + tokio::pin!(later_writer); + poll_fn(|cx| { + assert!( + later_writer.as_mut().poll(cx).is_pending(), + "a later store writer must not overtake queued retention" + ); + Poll::Ready(()) + }) + .await; + + let before_release = writer_entries.load(Ordering::SeqCst); release_tx.send(()).expect("release the original writer"); assert_eq!( tokio::time::timeout(Duration::from_secs(5), &mut queued) @@ -1681,26 +1703,59 @@ mod tests { .expect("queued retention is admitted despite competing writers"), Some(42) ); - // A succeeding write proves the churn remained live *through* the - // retention admission; it is stopped only after that observation. + assert_eq!( + tokio::time::timeout(Duration::from_secs(5), &mut later_writer) + .await + .expect("later writer resumes after retention"), + 43 + ); + // Keep the churn alive through retention and observe a subsequent + // acquisition before stopping it: a finite burst is not this proof. tokio::time::timeout(Duration::from_secs(5), async { - while completed_writers.load(Ordering::SeqCst) <= before_release { + while writer_entries.load(Ordering::SeqCst) <= before_release { tokio::task::yield_now().await; } }) .await .expect("unrelated writers resume after maintenance admission"); - stop_churn.store(true, Ordering::SeqCst); + stop_churn.cancel(); holder.await.expect("first writer finishes"); competitors.await.expect("unrelated writer churn finishes"); - assert!(completed_writers.load(Ordering::SeqCst) > before_release); + assert!(writer_entries.load(Ordering::SeqCst) > before_release); } #[tokio::test] - async fn cancelled_queued_retention_releases_its_writer_place() { - use crate::daemon::branch_admin::{StoreAdministration, StoreWriterClass, WriterScope}; - use tracedecay_runtime_core::cancellation::CancellationToken; + async fn cancelled_retention_never_constructs_work_before_admission() { + let administration = StoreAdministration::default(); + let cancellation = CancellationToken::new(); + cancellation.cancel(); + let constructed = AtomicUsize::new(0); + for prior_deferrals in 0..=super::MAINTENANCE_WRITER_FAST_DEFERRALS { + assert_eq!( + super::admit_retention_writer( + &administration, + &cancellation, + prior_deferrals, + || { + constructed.fetch_add(1, Ordering::SeqCst); + std::future::ready(7_u8) + }, + ) + .await, + None, + "cancelled retention must refuse even an uncontended fast probe" + ); + } + assert_eq!(constructed.load(Ordering::SeqCst), 0); + assert_eq!( + administration.try_with_writer(|| async { 9_u8 }).await, + Some(9), + "refused work leaves the writer gate available" + ); + } + #[tokio::test] + async fn cancelled_queued_retention_releases_its_writer_place() { let administration = StoreAdministration::default(); let (entered_tx, entered_rx) = tokio::sync::oneshot::channel(); let (release_tx, release_rx) = tokio::sync::oneshot::channel(); @@ -1716,20 +1771,36 @@ mod tests { ) .await; }); - entered_rx.await.expect("writer holds shared daemon admission"); + entered_rx + .await + .expect("writer holds shared daemon admission"); let cancellation = CancellationToken::new(); + let constructed = AtomicUsize::new(0); let waiting = super::admit_retention_writer( &administration, &cancellation, super::MAINTENANCE_WRITER_FAST_DEFERRALS, - || async { 7_u8 }, + || { + constructed.fetch_add(1, Ordering::SeqCst); + std::future::ready(7_u8) + }, ); tokio::pin!(waiting); - assert!( - tokio::time::timeout(Duration::from_millis(30), &mut waiting) - .await - .is_err() + poll_fn(|cx| { + assert!(waiting.as_mut().poll(cx).is_pending()); + Poll::Ready(()) + }) + .await; + let follower = administration.with_writer_in( + WriterScope::store("/follower-store", StoreWriterClass::Content), + || async { 8_u8 }, ); + tokio::pin!(follower); + poll_fn(|cx| { + assert!(follower.as_mut().poll(cx).is_pending()); + Poll::Ready(()) + }) + .await; cancellation.cancel(); assert_eq!( tokio::time::timeout(Duration::from_secs(2), &mut waiting) @@ -1737,18 +1808,25 @@ mod tests { .expect("cancelled acquisition completes"), None ); + assert_eq!(constructed.load(Ordering::SeqCst), 0); + assert_eq!( + tokio::time::timeout(Duration::from_secs(2), &mut follower) + .await + .expect("cancellation removes the queued writer before the holder exits"), + 8 + ); release_tx.send(()).expect("release held writer"); writer.await.expect("held writer exits"); - assert_eq!(administration.try_with_writer(|| async { 9_u8 }).await, Some(9)); + assert_eq!( + administration.try_with_writer(|| async { 9_u8 }).await, + Some(9) + ); } /// Cancellation after writer admission must not drop a running retention /// operation: its blocking/journaled work is responsible for cooperating. #[tokio::test] async fn cancellation_after_writer_admission_preserves_running_work() { - use crate::daemon::branch_admin::StoreAdministration; - use tracedecay_runtime_core::cancellation::CancellationToken; - let administration = StoreAdministration::default(); let cancellation = CancellationToken::new(); let working = administration.clone(); @@ -1761,7 +1839,9 @@ mod tests { &stopping, super::MAINTENANCE_WRITER_FAST_DEFERRALS, move || async move { - entered_tx.send(()).expect("retention entered its critical section"); + entered_tx + .send(()) + .expect("retention entered its critical section"); release_rx.await.expect("retention work released"); 43_u8 }, @@ -1770,13 +1850,21 @@ mod tests { }); entered_rx.await.expect("writer admission succeeded"); cancellation.cancel(); + assert_eq!( + administration.try_with_writer(|| async { 44_u8 }).await, + None, + "admitted work retains exclusion until it finishes" + ); release_tx.send(()).expect("finish admitted work"); assert_eq!( work.await.expect("retention task joins"), Some(43), "shutdown cancellation cannot drop journaled work already admitted" ); - assert_eq!(administration.try_with_writer(|| async { 44_u8 }).await, Some(44)); + assert_eq!( + administration.try_with_writer(|| async { 44_u8 }).await, + Some(44) + ); } #[tokio::test] @@ -2066,7 +2154,9 @@ mod tests { .await; }) .expect("start competing store writer"); - writer_entered.await.expect("unrelated store writer owns shared gate"); + writer_entered + .await + .expect("unrelated store writer owns shared gate"); for _ in 0..super::MAINTENANCE_WRITER_FAST_DEFERRALS { let deferred = coordinator .run_tick( @@ -2080,7 +2170,11 @@ mod tests { ) .await; assert_eq!(deferred, MaintenanceTickOutcome::Retry); - assert_eq!(generation_files(), 6, "fast probe must not collect under a writer"); + assert_eq!( + generation_files(), + 6, + "fast probe must not collect under a writer" + ); } let release_after_queue = std::thread::spawn(move || { @@ -2100,7 +2194,10 @@ mod tests { .await; release_after_queue.join().expect("writer release thread"); assert!( - matches!(writer.wait().await, native_runtime::NativeTaskOutcome::Completed(())), + matches!( + writer.wait().await, + native_runtime::NativeTaskOutcome::Completed(()) + ), "competing writer must complete normally" ); assert_eq!( From 41362a6527bdfea1e8609f370c25f56340fbe743 Mon Sep 17 00:00:00 2001 From: Zack Jackson <25274700+ScriptedAlchemy@users.noreply.github.com> Date: Sun, 8 Nov 2026 02:59:01 -0800 Subject: [PATCH 06/23] fix(retention): queue maintenance per project without global stalls Use a fair store-local maintenance lane shared by owner/content writers, while retaining daemon-wide exclusivity for truly profile-wide work. Queue verified project retention only against that project's writers and preserve independent project opens during a long running sync. Retain nonqueued daemon-wide probes for session work that lacks proven per-store identity. Exercise same-store fairness, independent writer progress, cancellation and a real registered-project GC tick. Refs #4616 --- .../src/writer_gate.rs | 117 ++++++++++++++++- crates/tracedecay/src/daemon/branch_admin.rs | 32 ++++- crates/tracedecay/src/daemon/maintenance.rs | 118 +++++++++++++----- 3 files changed, 224 insertions(+), 43 deletions(-) diff --git a/crates/tracedecay-store-runtime/src/writer_gate.rs b/crates/tracedecay-store-runtime/src/writer_gate.rs index 668af592408..a03ffa5cdf6 100644 --- a/crates/tracedecay-store-runtime/src/writer_gate.rs +++ b/crates/tracedecay-store-runtime/src/writer_gate.rs @@ -15,9 +15,10 @@ //! # The lock hierarchy //! //! ```text -//! daemon: RwLock Daemon scope = write, Store scope = read -//! ├── owner: Mutex -//! └── content: Mutex +//! daemon: RwLock Daemon scope = write, store scopes = read +//! └── store: RwLock Maintenance = write, owner/content = read +//! ├── owner: Mutex +//! └── content: Mutex //! ``` //! //! Every acquisition takes these in exactly that order, so no lock-order @@ -66,6 +67,8 @@ pub enum WriterScope { data_root: PathBuf, class: StoreWriterClass, }, + /// Fair exclusive maintenance for one store, without blocking unrelated stores. + StoreMaintenance { data_root: PathBuf }, } impl WriterScope { @@ -79,6 +82,15 @@ impl WriterScope { class, } } + + /// Same canonical store identity as [`Self::store`], with exclusive admission + /// only against this store's owner and content operations. + #[must_use] + pub fn maintenance(data_root: impl Into) -> Self { + Self::StoreMaintenance { + data_root: data_root.into(), + } + } } /// The logical lanes of one store family. Held by `Arc` from the registry and by @@ -86,6 +98,7 @@ impl WriterScope { /// its weak entry. #[derive(Default)] struct StoreGate { + maintenance: Arc>, owner: Arc>, content: Arc>, } @@ -105,6 +118,7 @@ impl StoreGate { /// exist to pin the guards. pub struct WriterAdmissionGuard { _class: Option>, + _maintenance: Option, _daemon: DaemonGuard, /// Keeps the store's gate alive for as long as it is held, so a registry /// prune between acquisition and release cannot hand a second acquirer a @@ -120,6 +134,13 @@ enum DaemonGuard { Exclusive(OwnedRwLockWriteGuard<()>), } +/// Store-local readers and the queued exclusive retention guard. +#[allow(dead_code)] +enum StoreMaintenanceGuard { + Shared(OwnedRwLockReadGuard<()>), + Exclusive(OwnedRwLockWriteGuard<()>), +} + /// Writer-gate registry: one process-wide lane plus one lane set per store family. pub struct StoreWriterGates { daemon: Arc>, @@ -173,6 +194,7 @@ impl StoreWriterGates { match scope { WriterScope::Daemon => WriterAdmissionGuard { _class: None, + _maintenance: None, _daemon: DaemonGuard::Exclusive( tracing::Instrument::instrument( Arc::clone(&self.daemon).write_owned(), @@ -189,6 +211,11 @@ impl StoreWriterGates { ) .await; let gate = self.store_gate(data_root); + let maintenance = tracing::Instrument::instrument( + Arc::clone(&gate.maintenance).read_owned(), + tracing::trace_span!("daemon.writer_gate.acquire.store_read"), + ) + .await; let class_guard = tracing::Instrument::instrument( gate.class_mutex(*class).lock_owned(), tracing::trace_span!("daemon.writer_gate.acquire.class"), @@ -196,6 +223,26 @@ impl StoreWriterGates { .await; WriterAdmissionGuard { _class: Some(class_guard), + _maintenance: Some(StoreMaintenanceGuard::Shared(maintenance)), + _daemon: DaemonGuard::Shared(daemon), + _gate: Some(gate), + } + } + WriterScope::StoreMaintenance { data_root } => { + let daemon = tracing::Instrument::instrument( + Arc::clone(&self.daemon).read_owned(), + tracing::trace_span!("daemon.writer_gate.acquire.store"), + ) + .await; + let gate = self.store_gate(data_root); + let maintenance = tracing::Instrument::instrument( + Arc::clone(&gate.maintenance).write_owned(), + tracing::trace_span!("daemon.writer_gate.acquire.store_maintenance"), + ) + .await; + WriterAdmissionGuard { + _class: None, + _maintenance: Some(StoreMaintenanceGuard::Exclusive(maintenance)), _daemon: DaemonGuard::Shared(daemon), _gate: Some(gate), } @@ -209,15 +256,29 @@ impl StoreWriterGates { match scope { WriterScope::Daemon => Some(WriterAdmissionGuard { _class: None, + _maintenance: None, _daemon: DaemonGuard::Exclusive(Arc::clone(&self.daemon).try_write_owned().ok()?), _gate: None, }), WriterScope::Store { data_root, class } => { let daemon = Arc::clone(&self.daemon).try_read_owned().ok()?; let gate = self.store_gate(data_root); + let maintenance = Arc::clone(&gate.maintenance).try_read_owned().ok()?; let class_guard = gate.class_mutex(*class).try_lock_owned().ok()?; Some(WriterAdmissionGuard { _class: Some(class_guard), + _maintenance: Some(StoreMaintenanceGuard::Shared(maintenance)), + _daemon: DaemonGuard::Shared(daemon), + _gate: Some(gate), + }) + } + WriterScope::StoreMaintenance { data_root } => { + let daemon = Arc::clone(&self.daemon).try_read_owned().ok()?; + let gate = self.store_gate(data_root); + let maintenance = Arc::clone(&gate.maintenance).try_write_owned().ok()?; + Some(WriterAdmissionGuard { + _class: None, + _maintenance: Some(StoreMaintenanceGuard::Exclusive(maintenance)), _daemon: DaemonGuard::Shared(daemon), _gate: Some(gate), }) @@ -228,6 +289,8 @@ impl StoreWriterGates { #[cfg(test)] mod tests { + use std::future::{Future, poll_fn}; + use std::task::Poll; use std::time::Duration; use super::*; @@ -322,6 +385,54 @@ mod tests { ); } + #[tokio::test] + async fn queued_store_maintenance_preserves_other_project_admission() { + let gates = StoreWriterGates::default(); + let held = gates.acquire(&store("/a", StoreWriterClass::Content)).await; + let maintenance_scope = WriterScope::maintenance("/a"); + let queued = gates.acquire(&maintenance_scope); + tokio::pin!(queued); + poll_fn(|cx| { + assert!(queued.as_mut().poll(cx).is_pending()); + Poll::Ready(()) + }) + .await; + + let following = gates.acquire(&store("/a", StoreWriterClass::Owner)); + tokio::pin!(following); + poll_fn(|cx| { + assert!( + following.as_mut().poll(cx).is_pending(), + "same-store operations wait behind queued maintenance" + ); + Poll::Ready(()) + }) + .await; + + assert!( + gates + .try_acquire(&store("/b", StoreWriterClass::Owner)) + .is_some(), + "an unrelated project can open while maintenance queues behind a long sync" + ); + drop(held); + let admitted = tokio::time::timeout(Duration::from_secs(5), &mut queued) + .await + .expect("queued maintenance acquires when its own store is free"); + assert!( + gates + .try_acquire(&store("/a", StoreWriterClass::Content)) + .is_none(), + "admitted maintenance excludes writes to its own store" + ); + drop(admitted); + let follower = tokio::time::timeout(Duration::from_secs(5), &mut following) + .await + .expect("same-store owner resumes after maintenance"); + drop(follower); + assert!(gates.try_acquire(&maintenance_scope).is_some()); + } + #[tokio::test] async fn retired_store_gates_are_reclaimed() { let gates = StoreWriterGates::default(); diff --git a/crates/tracedecay/src/daemon/branch_admin.rs b/crates/tracedecay/src/daemon/branch_admin.rs index e521ed4b89b..43ae742c7c6 100644 --- a/crates/tracedecay/src/daemon/branch_admin.rs +++ b/crates/tracedecay/src/daemon/branch_admin.rs @@ -125,6 +125,16 @@ pub(super) fn store_writer_scope(data_root: &Path, class: StoreWriterClass) -> W } } +/// Resolves maintenance to the same canonical store gate as logical writers. +/// A missing or unresolvable store refuses isolation by falling back to daemon +/// administration rather than silently splitting the writer identity. +pub(super) fn store_maintenance_writer_scope(data_root: &Path) -> WriterScope { + match authority::canonical_identity_path(data_root) { + Ok(canonical) => WriterScope::maintenance(canonical), + Err(_) => WriterScope::Daemon, + } +} + /// Owner-lane scope for one registered database owner. /// /// [`StoreOwnerKey::store_root`](tracedecay_session_runtime::StoreOwnerKey) is already the @@ -1923,20 +1933,20 @@ impl StoreAdministration { operation().await } - /// Cancellation applies only while queued for daemon-wide writer admission. + /// Cancellation applies only while queued for writer admission. /// Once the guard is acquired, the operation must finish cooperatively: /// dropping an in-flight journal/retention future would abandon physical /// blocking work whose client survives the daemon's shutdown join. pub(super) async fn with_writer_cancellable_acquisition( &self, cancellation: &tracedecay_runtime_core::cancellation::CancellationToken, + scope: WriterScope, operation: Operation, ) -> Option where Operation: FnOnce() -> OperationFuture, OperationFuture: Future, { - let scope = WriterScope::Daemon; let _writer = tokio::select! { biased; () = cancellation.cancelled() => return None, @@ -1955,10 +1965,20 @@ impl StoreAdministration { Operation: FnOnce() -> OperationFuture, OperationFuture: Future, { - let writer = self.gate.try_acquire(&WriterScope::Daemon)?; - let output = operation().await; - drop(writer); - Some(output) + self.try_with_writer_in(WriterScope::Daemon, operation).await + } + + pub(super) async fn try_with_writer_in( + &self, + scope: WriterScope, + operation: Operation, + ) -> Option + where + Operation: FnOnce() -> OperationFuture, + OperationFuture: Future, + { + let _writer = self.gate.try_acquire(&scope)?; + Some(operation().await) } /// Resolves the authenticated client's project layout and runs destructive diff --git a/crates/tracedecay/src/daemon/maintenance.rs b/crates/tracedecay/src/daemon/maintenance.rs index fa2a8c4965f..015842f186f 100644 --- a/crates/tracedecay/src/daemon/maintenance.rs +++ b/crates/tracedecay/src/daemon/maintenance.rs @@ -26,20 +26,22 @@ use tracedecay_maintenance::tick::{ }; use tracedecay_runtime_core::storage::{RESPONSE_HANDLES_DIRECTORY, profile_sharded_data_root}; -use super::branch_admin::StoreAdministration; +use super::branch_admin::{StoreAdministration, store_maintenance_writer_scope}; use tracedecay_daemon_service::shutdown::DAEMON_TASK_ABORT_DEADLINE; use tracedecay_domain::process_heap::request_idle_thread_collection_v1; use tracedecay_runtime_core::logging::log_daemon_event; use tracedecay_runtime_core::native_runtime::{self, NativeTask, TimeoutError}; +use tracedecay_store_runtime::WriterScope; const MAINTENANCE_STORE_PAGE_LIMIT: usize = 8; const REGISTERED_PROJECT_PAGE_LIMIT: usize = 64; -/// Fast probes preserve low-latency work. After two refusals, join Tokio's -/// fair writer queue so a stream of store-scoped readers cannot starve GC. +/// Fast probes preserve low-latency work. After two store-local refusals, +/// queue behind that store's writers without stalling independent projects. const MAINTENANCE_WRITER_FAST_DEFERRALS: u8 = 2; async fn admit_retention_writer( administration: &StoreAdministration, cancellation: &tracedecay_runtime_core::cancellation::CancellationToken, + scope: WriterScope, prior_deferrals: u8, operation: Operation, ) -> Option @@ -53,12 +55,12 @@ where return None; } if prior_deferrals < MAINTENANCE_WRITER_FAST_DEFERRALS { - administration.try_with_writer(operation).await + administration.try_with_writer_in(scope, operation).await } else { // Only cancel the wait: once admitted, journal/retention work must // finish cooperatively rather than be dropped at the writer boundary. administration - .with_writer_cancellable_acquisition(cancellation, operation) + .with_writer_cancellable_acquisition(cancellation, scope, operation) .await } } @@ -690,7 +692,10 @@ impl Default for MaintenanceCoordinator { /// retirements. A store unmounted since the listing is skipped. enum MaintenanceStoreWork { Session(PathBuf), - Graph(PathBuf), + Graph { + database_path: PathBuf, + data_root: PathBuf, + }, /// The code-index scopes of a registered project that no mounted graph /// owns; its database is never opened by this unit. Registered(RegisteredProjectStoreV1), @@ -699,10 +704,21 @@ enum MaintenanceStoreWork { impl MaintenanceStoreWork { fn database_path(&self) -> Option<&Path> { match self { - Self::Session(database_path) | Self::Graph(database_path) => Some(database_path), + Self::Session(database_path) | Self::Graph { database_path, .. } => Some(database_path), Self::Registered(_) => None, } } + + fn writer_scope(&self) -> WriterScope { + match self { + // Session-wide administration has no independently proven canonical + // store gate. Keep its global probe nonqueued rather than blocking + // unrelated project opens behind a potentially long session pass. + Self::Session(_) => WriterScope::Daemon, + Self::Graph { data_root, .. } => store_maintenance_writer_scope(data_root), + Self::Registered(store) => store_maintenance_writer_scope(&store.data_root), + } + } } /// What a tick reads from one mounted project graph, copied out at listing. @@ -1081,7 +1097,10 @@ impl MaintenanceCoordinator { for graph in &project_graphs { work.push(( graph.key.clone(), - MaintenanceStoreWork::Graph(graph.database_path.clone()), + MaintenanceStoreWork::Graph { + database_path: graph.database_path.clone(), + data_root: graph.data_root.clone(), + }, )); } for (project_id, store) in registered_projects { @@ -1122,7 +1141,7 @@ impl MaintenanceCoordinator { .collect::>(); scan_data_roots.extend(work.iter().filter_map(|(_, unit)| match unit { MaintenanceStoreWork::Registered(store) => Some(store.data_root.clone()), - MaintenanceStoreWork::Session(_) | MaintenanceStoreWork::Graph(_) => None, + MaintenanceStoreWork::Session(_) | MaintenanceStoreWork::Graph { .. } => None, })); maintenance_observations.retain_text_artifact_scan_projects(&scan_data_roots); } @@ -1161,16 +1180,24 @@ impl MaintenanceCoordinator { outcome = MaintenanceTickOutcome::Retry; break; } - let prior_deferrals = self - .writer_deferrals - .lock() - .await - .get(&keys[index]) - .copied() - .unwrap_or(0); + let scope = work[index].1.writer_scope(); + // Only store-local scopes may queue. A queued global writer would + // block every unrelated project while one long sync holds a read gate. + let local = !matches!(&scope, WriterScope::Daemon); + let prior_deferrals = if local { + self.writer_deferrals + .lock() + .await + .get(&keys[index]) + .copied() + .unwrap_or(0) + } else { + 0 + }; let admitted = admit_retention_writer( administration, &self.cancellation, + scope, prior_deferrals, || async { let Ok(_clients) = administration.maintenance_store_clients().try_lock() else { @@ -1193,7 +1220,7 @@ impl MaintenanceCoordinator { run_registered_store_retention(&database, retention).await } } - MaintenanceStoreWork::Graph(_) => { + MaintenanceStoreWork::Graph { .. } => { let Some(graph) = administration .mounted_project_graphs() .await @@ -1231,7 +1258,7 @@ impl MaintenanceCoordinator { let mut deferrals = self.writer_deferrals.lock().await; if admitted.is_some() { deferrals.remove(&keys[index]); - } else if !self.cancellation.is_cancelled() { + } else if local && !self.cancellation.is_cancelled() { deferrals.insert(keys[index].clone(), prior_deferrals.saturating_add(1)); } } @@ -1602,9 +1629,9 @@ mod tests { use tracedecay_runtime_core::cancellation::CancellationToken; use tracedecay_runtime_core::native_runtime; - /// Hold an unrelated store writer across two failed fast probes. The third - /// retention attempt must queue behind it and complete even as more store - /// writers arrive, rather than repeatedly taking non-queued snapshots. + /// Hold a writer for the target store across two failed fast probes. + /// The third retention attempt joins its local queue; independent project + /// opens must remain admitted while that writer is still active. #[tokio::test] async fn retention_queues_after_bounded_writer_deferrals() { let administration = StoreAdministration::default(); @@ -1614,7 +1641,7 @@ mod tests { let holder = tokio::spawn(async move { holding_admin .with_writer_in( - WriterScope::store("/unrelated-store", StoreWriterClass::Content), + WriterScope::store("/retained-store", StoreWriterClass::Content), move || async move { entered_tx.send(()).expect("store writer entered"); release_rx.await.expect("store writer released"); @@ -1624,21 +1651,25 @@ mod tests { }); entered_rx .await - .expect("writer acquired its shared daemon gate"); + .expect("writer acquired its store gate"); let cancellation = CancellationToken::new(); for refused in 0..super::MAINTENANCE_WRITER_FAST_DEFERRALS { assert_eq!( - super::admit_retention_writer(&administration, &cancellation, refused, || async { - 41_u8 - }) + super::admit_retention_writer( + &administration, + &cancellation, + WriterScope::maintenance("/retained-store"), + refused, + || async { 41_u8 }, + ) .await, None, - "a store writer blocks the non-queued daemon probe" + "a store writer blocks the nonqueued store-local probe" ); } // Establish ongoing writer activity before queueing the exclusive - // maintenance request. A fair queued writer blocks later readers; + // maintenance request. A fair queued maintenance writer blocks later readers of that store; // waiting for their progress before releasing the holder deadlocks. let competing_admin = administration.clone(); let writer_entries = Arc::new(AtomicUsize::new(0)); @@ -1669,6 +1700,7 @@ mod tests { let queued = super::admit_retention_writer( &administration, &cancellation, + WriterScope::maintenance("/retained-store"), super::MAINTENANCE_WRITER_FAST_DEFERRALS, || async { 42_u8 }, ); @@ -1682,7 +1714,7 @@ mod tests { }) .await; let later_writer = administration.with_writer_in( - WriterScope::store("/later-store", StoreWriterClass::Content), + WriterScope::store("/retained-store", StoreWriterClass::Owner), || async { 43_u8 }, ); tokio::pin!(later_writer); @@ -1695,6 +1727,17 @@ mod tests { }) .await; + // An independent store must still open while maintenance is queued. + let independent = administration.with_writer_in( + WriterScope::store("/independent-store", StoreWriterClass::Owner), + || async { 44_u8 }, + ); + assert_eq!( + tokio::time::timeout(Duration::from_secs(2), independent) + .await + .expect("unrelated project open must not stall behind queued maintenance"), + 44 + ); let before_release = writer_entries.load(Ordering::SeqCst); release_tx.send(()).expect("release the original writer"); assert_eq!( @@ -1735,6 +1778,7 @@ mod tests { super::admit_retention_writer( &administration, &cancellation, + WriterScope::maintenance("/held-store"), prior_deferrals, || { constructed.fetch_add(1, Ordering::SeqCst); @@ -1779,6 +1823,7 @@ mod tests { let waiting = super::admit_retention_writer( &administration, &cancellation, + WriterScope::maintenance("/held-store"), super::MAINTENANCE_WRITER_FAST_DEFERRALS, || { constructed.fetch_add(1, Ordering::SeqCst); @@ -1837,6 +1882,7 @@ mod tests { super::admit_retention_writer( &working, &stopping, + WriterScope::maintenance("/retention-store"), super::MAINTENANCE_WRITER_FAST_DEFERRALS, move || async move { entered_tx @@ -2132,19 +2178,23 @@ mod tests { ); drop(release); - // A long-lived, unrelated content writer holds the daemon read gate. + // A long-lived writer for this project holds the store-local read gate. // Two real retention ticks must defer, and the next must queue rather // than missing the same writer again. The fixture's six on-disk // generations make forward progress observable through actual GC. let (writer_started, writer_entered) = tokio::sync::oneshot::channel(); let (release_writer, writer_released) = tokio::sync::oneshot::channel(); let holding_admin = administration.clone(); + let retained_data_root = tracedecay_runtime_core::storage::profile_sharded_data_root( + &profile_root, + &project.project_id, + ); let writer = NativeTask::spawn(async move { holding_admin .with_writer_in( - crate::daemon::branch_admin::WriterScope::store( - "/unrelated-project", - crate::daemon::branch_admin::StoreWriterClass::Content, + crate::daemon::branch_admin::store_writer_scope( + &retained_data_root, + StoreWriterClass::Content, ), move || async move { writer_started.send(()).expect("writer admitted"); @@ -2156,7 +2206,7 @@ mod tests { .expect("start competing store writer"); writer_entered .await - .expect("unrelated store writer owns shared gate"); + .expect("project content writer owns shared store gate"); for _ in 0..super::MAINTENANCE_WRITER_FAST_DEFERRALS { let deferred = coordinator .run_tick( From 2bd73a4fac5be20797837e421ba976478ffb7912 Mon Sep 17 00:00:00 2001 From: Zack Jackson <25274700+ScriptedAlchemy@users.noreply.github.com> Date: Sun, 8 Nov 2026 05:24:10 -0800 Subject: [PATCH 07/23] style(retention): match Bazel rustfmt on admission paths --- crates/tracedecay/src/daemon/branch_admin.rs | 3 ++- crates/tracedecay/src/daemon/maintenance.rs | 4 +--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/crates/tracedecay/src/daemon/branch_admin.rs b/crates/tracedecay/src/daemon/branch_admin.rs index 43ae742c7c6..5c891316169 100644 --- a/crates/tracedecay/src/daemon/branch_admin.rs +++ b/crates/tracedecay/src/daemon/branch_admin.rs @@ -1965,7 +1965,8 @@ impl StoreAdministration { Operation: FnOnce() -> OperationFuture, OperationFuture: Future, { - self.try_with_writer_in(WriterScope::Daemon, operation).await + self.try_with_writer_in(WriterScope::Daemon, operation) + .await } pub(super) async fn try_with_writer_in( diff --git a/crates/tracedecay/src/daemon/maintenance.rs b/crates/tracedecay/src/daemon/maintenance.rs index 015842f186f..8e7e541def5 100644 --- a/crates/tracedecay/src/daemon/maintenance.rs +++ b/crates/tracedecay/src/daemon/maintenance.rs @@ -1649,9 +1649,7 @@ mod tests { ) .await; }); - entered_rx - .await - .expect("writer acquired its store gate"); + entered_rx.await.expect("writer acquired its store gate"); let cancellation = CancellationToken::new(); for refused in 0..super::MAINTENANCE_WRITER_FAST_DEFERRALS { assert_eq!( From 5c37cdedf9b8bea37e61961257c3a65502740853 Mon Sep 17 00:00:00 2001 From: Zack Jackson <25274700+ScriptedAlchemy@users.noreply.github.com> Date: Sun, 8 Nov 2026 07:24:51 -0800 Subject: [PATCH 08/23] fix(retention): keep queued test scope alive for Bazel The store-local writer admission regression borrowed a temporary WriterScope across a pinned asynchronous acquisition. Native Bazel Rust compilation fails with E0716 at writer_gate.rs:401; retain the exact scope in a binding until that future completes. Existing behavioral regressions continue proving fairness and cross-project isolation. Refs #4616 --- crates/tracedecay-store-runtime/src/writer_gate.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/tracedecay-store-runtime/src/writer_gate.rs b/crates/tracedecay-store-runtime/src/writer_gate.rs index a03ffa5cdf6..e2d572d8e20 100644 --- a/crates/tracedecay-store-runtime/src/writer_gate.rs +++ b/crates/tracedecay-store-runtime/src/writer_gate.rs @@ -398,7 +398,8 @@ mod tests { }) .await; - let following = gates.acquire(&store("/a", StoreWriterClass::Owner)); + let following_scope = store("/a", StoreWriterClass::Owner); + let following = gates.acquire(&following_scope); tokio::pin!(following); poll_fn(|cx| { assert!( From 0b2a0621ca2616a9f7fe0cba95508048cae1d169 Mon Sep 17 00:00:00 2001 From: Zack Jackson <25274700+ScriptedAlchemy@users.noreply.github.com> Date: Sun, 8 Nov 2026 13:31:34 -0800 Subject: [PATCH 09/23] test(retention): queue the cancellation follower on the held store --- crates/tracedecay/src/daemon/maintenance.rs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/crates/tracedecay/src/daemon/maintenance.rs b/crates/tracedecay/src/daemon/maintenance.rs index 8e7e541def5..68eb3a5026c 100644 --- a/crates/tracedecay/src/daemon/maintenance.rs +++ b/crates/tracedecay/src/daemon/maintenance.rs @@ -1835,7 +1835,7 @@ mod tests { }) .await; let follower = administration.with_writer_in( - WriterScope::store("/follower-store", StoreWriterClass::Content), + WriterScope::store("/held-store", StoreWriterClass::Content), || async { 8_u8 }, ); tokio::pin!(follower); @@ -1852,14 +1852,19 @@ mod tests { None ); assert_eq!(constructed.load(Ordering::SeqCst), 0); + poll_fn(|cx| { + assert!(follower.as_mut().poll(cx).is_pending()); + Poll::Ready(()) + }) + .await; + release_tx.send(()).expect("release held writer"); + writer.await.expect("held writer exits"); assert_eq!( tokio::time::timeout(Duration::from_secs(2), &mut follower) .await - .expect("cancellation removes the queued writer before the holder exits"), + .expect("next store writer proceeds after the holder exits"), 8 ); - release_tx.send(()).expect("release held writer"); - writer.await.expect("held writer exits"); assert_eq!( administration.try_with_writer(|| async { 9_u8 }).await, Some(9) From 0dbda7ea574b8576b5a8c5ec4822001b45bd09b2 Mon Sep 17 00:00:00 2001 From: Zack Jackson <25274700+ScriptedAlchemy@users.noreply.github.com> Date: Sun, 8 Nov 2026 01:52:34 -0800 Subject: [PATCH 10/23] test(automation): bind scheduler fixtures to backend identities Repair three hosted automation_runner_test failures by supplying the configured executable identity required by the production prefilter. Use the same identity for the prefilter and retained runner, preserving manual-schedule, exact-ledger, live-lock and lock-race assertions. Add a production-prefilter regression for unconfigured refusal, deduplication and same-occurrence recovery after binding, with zero backend calls and no effectful terminal. Do not relax backend admission, change timeouts, or alter the graph publication changes. Refs #4532 --- .../tests/automation_runner_test/jobs.rs | 124 ++++++++++++++++-- 1 file changed, 110 insertions(+), 14 deletions(-) diff --git a/crates/tracedecay/tests/automation_runner_test/jobs.rs b/crates/tracedecay/tests/automation_runner_test/jobs.rs index 740283d56e8..7342786381b 100644 --- a/crates/tracedecay/tests/automation_runner_test/jobs.rs +++ b/crates/tracedecay/tests/automation_runner_test/jobs.rs @@ -86,6 +86,7 @@ fn enabled_job_config() -> AutomationSettingsV1 { struct ContentBackend { calls: AtomicUsize, content: &'static str, + executable: Option, } impl ContentBackend { @@ -93,9 +94,15 @@ impl ContentBackend { Self { calls: AtomicUsize::new(0), content, + executable: None, } } + fn with_executable(mut self, executable: &Path) -> Self { + self.executable = Some(executable.to_path_buf()); + self + } + fn calls(&self) -> usize { self.calls.load(Ordering::SeqCst) } @@ -126,7 +133,7 @@ impl AgentTaskBackend for ContentBackend { } fn executable(&self) -> Option<&std::path::Path> { - None + self.executable.as_deref() } } @@ -988,6 +995,10 @@ async fn scheduler_trigger_skips_repeat_and_respects_lock_discipline() { let mut job = sample_job("sched-job"); job.schedule = Some("manual".to_string()); let config = enabled_job_config(); + // Reach the schedule gate with a bound identity, not the earlier + // backend-unconfigured refusal. This gate never invokes the backend. + let executable = tempfile::NamedTempFile::new().unwrap(); + let backend = ContentBackend::new("unused").with_executable(executable.path()); // Manual schedule: every scheduler tick of the same occurrence reaches // the prefilter, which records the skip once. @@ -996,7 +1007,7 @@ async fn scheduler_trigger_skips_repeat_and_respects_lock_discipline() { let run = evaluate_and_record_scheduler_skip( &dashboard_root, &config, - None, + backend.executable(), &job, "sched-occurrence", None, @@ -1019,7 +1030,7 @@ async fn scheduler_trigger_skips_repeat_and_respects_lock_discipline() { let repeated = evaluate_and_record_scheduler_skip( &dashboard_root, &config, - None, + backend.executable(), &job, "sched-occurrence", None, @@ -1031,6 +1042,76 @@ async fn scheduler_trigger_skips_repeat_and_respects_lock_discipline() { repeated.ledger_record, records[0], "an immediate repeat skip must return the exact durable prior row" ); + assert_eq!(backend.calls(), 0); +} + +#[tokio::test] +async fn scheduler_prefilter_rejects_unconfigured_backend_and_recovers_without_effect() { + let temp = tempdir().unwrap(); + let dashboard_root = temp.path().join("dashboard"); + let job = sample_job("backend-recovery"); + let config = enabled_job_config(); + let occurrence = "backend-recovery-occurrence"; + let backend = ContentBackend::new("unused"); + + let first = evaluate_and_record_scheduler_skip( + &dashboard_root, + &config, + backend.executable(), + &job, + occurrence, + None, + ) + .await + .unwrap() + .expect("a due job still requires a configured backend"); + assert_eq!(first.report["reason"], json!("backend_unconfigured")); + assert_eq!(first.ledger_record.status, AutomationRunStatus::Skipped); + assert_ne!(first.run_id, occurrence); + let repeated = evaluate_and_record_scheduler_skip( + &dashboard_root, + &config, + backend.executable(), + &job, + occurrence, + None, + ) + .await + .unwrap() + .unwrap(); + assert_eq!(repeated.ledger_record, first.ledger_record); + + let executable = tempfile::NamedTempFile::new().unwrap(); + let backend = backend.with_executable(executable.path()); + assert!( + evaluate_and_record_scheduler_skip( + &dashboard_root, + &config, + backend.executable(), + &job, + occurrence, + None, + ) + .await + .unwrap() + .is_none(), + "binding the backend must admit the same due occurrence without cooldown" + ); + assert_eq!(backend.calls(), 0); + assert_eq!( + load_run_records(&dashboard_root, 10).await.unwrap(), + vec![first.ledger_record], + "advisory admission must not append an effectful terminal" + ); + assert!( + tracedecay_automation_runtime::automation::run_ledger::find_run_record_exact_bounded( + &dashboard_root, + occurrence, + ) + .await + .unwrap() + .is_none() + ); } #[tokio::test] @@ -1206,15 +1287,22 @@ async fn scheduler_prefilter_live_lock_wins_over_not_due_summary() { let config = enabled_job_config(); let occurrence = "locked-prefilter-occurrence"; - let prefilter = - evaluate_and_record_scheduler_skip(&dashboard_root, &config, None, &job, occurrence, None) - .await - .unwrap() - .expect("the live job lock must produce a diagnostic"); + let executable = tempfile::NamedTempFile::new().unwrap(); + let backend = ContentBackend::new("unused").with_executable(executable.path()); + let prefilter = evaluate_and_record_scheduler_skip( + &dashboard_root, + &config, + backend.executable(), + &job, + occurrence, + None, + ) + .await + .unwrap() + .expect("the live job lock must produce a diagnostic"); assert_eq!(prefilter.report["reason"], json!("scheduler_lock_active")); assert_ne!(prefilter.run_id, occurrence); - let backend = ContentBackend::new("unused"); let admitted_job = tracedecay_automation_runtime::automation::jobs::admit_job(&job).unwrap(); let retained = run_user_job_with_backend_for_retained_settlement( &dashboard_root, @@ -1262,12 +1350,21 @@ async fn retained_scheduler_runner_reacquires_after_due_prefilter() { job.schedule = Some("every 1h".to_owned()); let config = enabled_job_config(); let occurrence = "prefilter-gap-occurrence"; + let executable = tempfile::NamedTempFile::new().unwrap(); + let backend = ContentBackend::new("unused").with_executable(executable.path()); assert!( - evaluate_and_record_scheduler_skip(&dashboard_root, &config, None, &job, occurrence, None) - .await - .unwrap() - .is_none(), + evaluate_and_record_scheduler_skip( + &dashboard_root, + &config, + backend.executable(), + &job, + occurrence, + None, + ) + .await + .unwrap() + .is_none(), "a new interval job is advisory-due before effect admission" ); let _raced_in = @@ -1276,7 +1373,6 @@ async fn retained_scheduler_runner_reacquires_after_due_prefilter() { .unwrap() .expect("the advisory prefilter must release its reservation lock"); - let backend = ContentBackend::new("unused"); let admitted_job = tracedecay_automation_runtime::automation::jobs::admit_job(&job).unwrap(); let retained = run_user_job_with_backend_for_retained_settlement( &dashboard_root, From 0b520f3c26ba551c61b8913cd9702eff87dd5300 Mon Sep 17 00:00:00 2001 From: Zack Jackson <25274700+ScriptedAlchemy@users.noreply.github.com> Date: Sun, 8 Nov 2026 13:51:16 -0800 Subject: [PATCH 11/23] test(retention): verify content resumes before owner release --- crates/tracedecay/src/daemon/maintenance.rs | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/crates/tracedecay/src/daemon/maintenance.rs b/crates/tracedecay/src/daemon/maintenance.rs index 68eb3a5026c..a62aa22d878 100644 --- a/crates/tracedecay/src/daemon/maintenance.rs +++ b/crates/tracedecay/src/daemon/maintenance.rs @@ -1852,19 +1852,16 @@ mod tests { None ); assert_eq!(constructed.load(Ordering::SeqCst), 0); - poll_fn(|cx| { - assert!(follower.as_mut().poll(cx).is_pending()); - Poll::Ready(()) - }) - .await; - release_tx.send(()).expect("release held writer"); - writer.await.expect("held writer exits"); + // Owner and content otherwise run concurrently. Only queued exclusive + // maintenance blocked this content writer on the same store. assert_eq!( tokio::time::timeout(Duration::from_secs(2), &mut follower) .await - .expect("next store writer proceeds after the holder exits"), + .expect("content writer resumes while the owner is still active"), 8 ); + release_tx.send(()).expect("release held writer"); + writer.await.expect("held writer exits"); assert_eq!( administration.try_with_writer(|| async { 9_u8 }).await, Some(9) From b299f0047691424fee71962344c0a2cb337cc8c8 Mon Sep 17 00:00:00 2001 From: Zack Jackson <25274700+ScriptedAlchemy@users.noreply.github.com> Date: Sun, 8 Nov 2026 15:06:56 -0800 Subject: [PATCH 12/23] test(cli): time held responses after request admission --- .../tool_client_transport.rs | 55 +++++++++---------- 1 file changed, 25 insertions(+), 30 deletions(-) diff --git a/crates/tracedecay/tests/runtime_acceptance_suite/tool_client_transport.rs b/crates/tracedecay/tests/runtime_acceptance_suite/tool_client_transport.rs index b0f714cdffb..e45871856b9 100644 --- a/crates/tracedecay/tests/runtime_acceptance_suite/tool_client_transport.rs +++ b/crates/tracedecay/tests/runtime_acceptance_suite/tool_client_transport.rs @@ -545,34 +545,12 @@ fn hold_stream_after_files( let _ = BufReader::new(stream).read_line(&mut extra); } -fn assert_complete_json_document(result: &ChildResult) -> Value { - assert!( - !result.killed_by_harness, - "CLI --json hung after writing files: elapsed={:?} stdout_len={} stderr={}", - result.elapsed, - result.output.stdout.len(), - String::from_utf8_lossy(&result.output.stderr) - ); - assert!( - result.output.status.success(), - "{}", - String::from_utf8_lossy(&result.output.stderr) - ); - serde_json::from_slice(&result.output.stdout).unwrap_or_else(|error| { - panic!( - "--json must be one complete parseable document ({error}); bytes={} head={}", - result.output.stdout.len(), - String::from_utf8_lossy(&result.output.stdout[..result.output.stdout.len().min(200)]) - ) - }) -} - #[test] fn generic_tool_json_preserves_large_result_while_daemon_waits() { let (_home, _project, _socket_dir, home, project, socket) = fixture(); let files = oversized_files(); let expected = files.count; - let (_requests, server) = spawn_scripted_daemon( + let (requests, server) = spawn_scripted_daemon( socket.clone(), &home, &project, @@ -581,12 +559,29 @@ fn generic_tool_json_preserves_large_result_while_daemon_waits() { hold_stream_after_files(stream, request, scope, files.clone()); }, ); - let result = run_command_with_timeout( - tool_command(&home, &project, &socket, "large"), - CHILD_TIMEOUT, - ); + let child = tool_command(&home, &project, &socket, "large") + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .expect("spawn CLI for held response"); + let mut child = common::TestChildProcess::new(child); + // Measure completion while the admitted request's connection remains open. + // The daemon has its own bounded acceptance window for cold CLI startup. + requests + .recv_timeout(ACCEPT_HANG_GUARD) + .expect("daemon received the large-result request"); + let output = child + .wait_with_output(CHILD_TIMEOUT) + .expect("CLI must finish without waiting for daemon EOF"); server.join().expect("join scripted daemon"); - let envelope = assert_complete_json_document(&result); + assert!( + output.status.success(), + "{}", + String::from_utf8_lossy(&output.stderr) + ); + let envelope: Value = + serde_json::from_slice(&output.stdout).expect("--json must contain one complete document"); assert_eq!(envelope["isError"], false, "{envelope}"); assert_eq!(envelope["structuredContent"]["count"], expected); let files = envelope["structuredContent"]["files"] @@ -594,9 +589,9 @@ fn generic_tool_json_preserves_large_result_while_daemon_waits() { .unwrap_or_else(|| panic!("held-stream --json must keep the full listing: {envelope}")); assert_eq!(files.len(), expected); assert!( - result.output.stdout.len() > 65_536, + output.stdout.len() > 65_536, "held-stream --json must keep the full listing above 64KiB: {}", - result.output.stdout.len() + output.stdout.len() ); } From fedc68eb803da5f2d88e0af151450ee09e7980dd Mon Sep 17 00:00:00 2001 From: Zack Jackson <25274700+ScriptedAlchemy@users.noreply.github.com> Date: Sun, 8 Nov 2026 15:45:00 -0800 Subject: [PATCH 13/23] test(runtime): bound CLI startup before scripted daemon request Refs #4616 --- .../tool_client_transport.rs | 29 +++++++++++++++---- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/crates/tracedecay/tests/runtime_acceptance_suite/tool_client_transport.rs b/crates/tracedecay/tests/runtime_acceptance_suite/tool_client_transport.rs index e45871856b9..020a5d17f4d 100644 --- a/crates/tracedecay/tests/runtime_acceptance_suite/tool_client_transport.rs +++ b/crates/tracedecay/tests/runtime_acceptance_suite/tool_client_transport.rs @@ -566,11 +566,30 @@ fn generic_tool_json_preserves_large_result_while_daemon_waits() { .spawn() .expect("spawn CLI for held response"); let mut child = common::TestChildProcess::new(child); - // Measure completion while the admitted request's connection remains open. - // The daemon has its own bounded acceptance window for cold CLI startup. - requests - .recv_timeout(ACCEPT_HANG_GUARD) - .expect("daemon received the large-result request"); + // Request admission and CLI startup share the original child deadline. + // Poll the child as well as the receipt: waiting only for the daemon's + // 120-second accept leak guard would mask an early CLI exit or failure. + let request_deadline = Instant::now() + CHILD_TIMEOUT; + loop { + match requests.recv_timeout(Duration::from_millis(50)) { + Ok(()) => break, + Err(RecvTimeoutError::Timeout) => { + assert!( + child.try_wait().expect("poll CLI before daemon request").is_none(), + "CLI exited before the fake daemon received its request" + ); + assert!( + Instant::now() < request_deadline, + "CLI did not send a daemon request within the child startup deadline" + ); + } + Err(RecvTimeoutError::Disconnected) => { + panic!("fake daemon stopped before the CLI sent its request") + } + } + } + // Once admitted, prove the full response is written while the daemon + // deliberately holds the connection open, not after an EOF or stall. let output = child .wait_with_output(CHILD_TIMEOUT) .expect("CLI must finish without waiting for daemon EOF"); From d70ad107cabc2ce357f11a9c547cb1d1dd659330 Mon Sep 17 00:00:00 2001 From: Zack Jackson <25274700+ScriptedAlchemy@users.noreply.github.com> Date: Sun, 8 Nov 2026 16:29:42 -0800 Subject: [PATCH 14/23] style(cli): format the pre-admission process assertion --- .../tests/runtime_acceptance_suite/tool_client_transport.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/tracedecay/tests/runtime_acceptance_suite/tool_client_transport.rs b/crates/tracedecay/tests/runtime_acceptance_suite/tool_client_transport.rs index 020a5d17f4d..49501498fa0 100644 --- a/crates/tracedecay/tests/runtime_acceptance_suite/tool_client_transport.rs +++ b/crates/tracedecay/tests/runtime_acceptance_suite/tool_client_transport.rs @@ -575,7 +575,10 @@ fn generic_tool_json_preserves_large_result_while_daemon_waits() { Ok(()) => break, Err(RecvTimeoutError::Timeout) => { assert!( - child.try_wait().expect("poll CLI before daemon request").is_none(), + child + .try_wait() + .expect("poll CLI before daemon request") + .is_none(), "CLI exited before the fake daemon received its request" ); assert!( From 58f6ee32bb8a42d5a734e7dea3005850ef336dbd Mon Sep 17 00:00:00 2001 From: Zack Jackson <25274700+ScriptedAlchemy@users.noreply.github.com> Date: Sun, 8 Nov 2026 13:58:03 -0800 Subject: [PATCH 15/23] test(cli): observe daemon readiness before checking socket mode --- .../tests/core_cli_suite/tool_daemon_test.rs | 33 ++----------------- 1 file changed, 3 insertions(+), 30 deletions(-) diff --git a/crates/tracedecay-cli/tests/core_cli_suite/tool_daemon_test.rs b/crates/tracedecay-cli/tests/core_cli_suite/tool_daemon_test.rs index 0c65098e48c..429644ba3c3 100644 --- a/crates/tracedecay-cli/tests/core_cli_suite/tool_daemon_test.rs +++ b/crates/tracedecay-cli/tests/core_cli_suite/tool_daemon_test.rs @@ -543,21 +543,6 @@ fn configuration_tool_success( } } -fn wait_for_daemon_socket(socket_path: &Path) { - let deadline = Instant::now() + Duration::from_secs(10); - common::poll_until( - deadline, - Duration::from_millis(25), - || UnixStream::connect(socket_path).is_ok().then_some(()), - || { - format!( - "timed out waiting for daemon socket at {}", - socket_path.display() - ) - }, - ); -} - /// Enroll a project for the native hook capture contract: an enrollment /// marker binds the project root to a profile shard, and a daemon-issued /// hook configuration binding is published under that shard's data root so @@ -1718,21 +1703,9 @@ fn daemon_socket_is_owner_only() { let home = TempDir::new().unwrap(); let home_path = canonical_existing_path(home.path()); let socket_path = common::daemon_socket_path(&home_path); - let _ = std::fs::remove_file(&socket_path); - let mut daemon = common::DaemonProcess::new( - tracedecay_command_with_home(&home_path) - .arg("daemon") - .arg("run") - .arg("--socket") - .arg(&socket_path) - .stdin(Stdio::null()) - .stdout(Stdio::null()) - .stderr(Stdio::null()) - .spawn() - .expect("tracedecay daemon should start"), - ); - - wait_for_daemon_socket(&socket_path); + let mut daemon = common::spawn_tracedecay_daemon_with(&home_path, |command| { + command.arg("--socket").arg(&socket_path); + }); let mode = std::fs::metadata(&socket_path) .expect("socket metadata") .permissions() From 7264f0d6c28deae48804bddbea2d9abad0d2f400 Mon Sep 17 00:00:00 2001 From: Zack Jackson <25274700+ScriptedAlchemy@users.noreply.github.com> Date: Sun, 8 Nov 2026 19:10:13 -0800 Subject: [PATCH 16/23] test(cli): measure oversized responses from request admission --- .../tool_client_transport.rs | 127 ++++++++++-------- 1 file changed, 74 insertions(+), 53 deletions(-) diff --git a/crates/tracedecay/tests/runtime_acceptance_suite/tool_client_transport.rs b/crates/tracedecay/tests/runtime_acceptance_suite/tool_client_transport.rs index b0f714cdffb..43a6df1fb54 100644 --- a/crates/tracedecay/tests/runtime_acceptance_suite/tool_client_transport.rs +++ b/crates/tracedecay/tests/runtime_acceptance_suite/tool_client_transport.rs @@ -79,6 +79,55 @@ fn run_command_with_timeout(mut command: Command, timeout: Duration) -> ChildRes } } +fn run_tool_after_admission(mut command: Command, requests: mpsc::Receiver) -> Output { + let started = Instant::now(); + let child = command + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .expect("spawn CLI for scripted daemon"); + let mut child = common::TestChildProcess::new(child); + let startup_deadline = started + CHILD_TIMEOUT; + let admitted = loop { + let remaining = startup_deadline.saturating_duration_since(Instant::now()); + match requests.recv_timeout(remaining.min(Duration::from_millis(50))) { + Ok(admitted) => break admitted, + Err(RecvTimeoutError::Timeout) => { + assert!( + child + .try_wait() + .expect("poll CLI before admission") + .is_none(), + "CLI exited before the daemon received its request" + ); + assert!( + Instant::now() < startup_deadline, + "CLI request admission timed out" + ); + } + Err(RecvTimeoutError::Disconnected) => { + panic!("scripted daemon stopped before receiving the CLI request") + } + } + }; + assert!( + admitted <= startup_deadline, + "CLI request admission timed out" + ); + // The daemon timestamps the authenticated request. Scheduling this test + // thread later must not extend the response deadline. + let output = child + .wait_with_output(CHILD_TIMEOUT.saturating_sub(admitted.elapsed())) + .expect("CLI must finish delivering the admitted response within its deadline"); + assert!( + output.status.success(), + "{}", + String::from_utf8_lossy(&output.stderr) + ); + output +} + fn init_project(home: &Path, project: &Path) { std::fs::create_dir_all(project.join("src")).expect("create project source"); std::fs::write(project.join("src/lib.rs"), "pub fn marker() {}\n") @@ -161,11 +210,11 @@ fn response_bytes(request: &DaemonInvocationRequest, scope: ResolvedScope, text: files_response(request, scope, files_result([text.to_owned()])) } -fn output_payload(result: &ChildResult) -> Value { - let envelope = serde_json::from_slice(&result.output.stdout).unwrap_or_else(|error| { +fn output_payload(output: &Output) -> Value { + let envelope = serde_json::from_slice(&output.stdout).unwrap_or_else(|error| { panic!( "invalid CLI JSON ({error}); stderr: {}", - String::from_utf8_lossy(&result.output.stderr) + String::from_utf8_lossy(&output.stderr) ) }); tracedecay::daemon::tool_json_payload(&envelope, "transport fixture") @@ -190,7 +239,7 @@ fn spawn_scripted_daemon( project: &Path, connections: usize, script: F, -) -> (mpsc::Receiver<()>, JoinHandle<()>) +) -> (mpsc::Receiver, JoinHandle<()>) where F: Fn(UnixStream, DaemonInvocationRequest, ResolvedScope) + Send + Sync + 'static, { @@ -275,7 +324,9 @@ where .. } )); - request_tx.send(()).expect("publish request receipt"); + request_tx + .send(Instant::now()) + .expect("publish request receipt"); let script = Arc::clone(&script); let scope = scope.clone(); workers.push(std::thread::spawn(move || script(stream, request, scope))); @@ -423,7 +474,7 @@ fn generic_tool_reports_unavailable_truncation_storage() { "{}", String::from_utf8_lossy(&result.output.stderr) ); - let payload = output_payload(&result); + let payload = output_payload(&result.output); assert_eq!(payload["truncated"], true, "{payload}"); assert_eq!(payload["handle_available"], false, "{payload}"); assert!( @@ -448,30 +499,23 @@ fn generic_tool_reports_unavailable_truncation_storage() { #[test] fn generic_tool_retrieves_oversized_typed_result() { let (_home, _project, _socket_dir, home, project, socket) = fixture(); - let expected = serde_json::to_value(oversized_files()).expect("expected file listing"); - let (_requests, server) = spawn_scripted_daemon( + let files = oversized_files(); + let expected = serde_json::to_value(&files).expect("expected file listing"); + let (requests, server) = spawn_scripted_daemon( socket.clone(), &home, &project, 1, - |mut stream, request, scope| { + move |mut stream, request, scope| { stream - .write_all(&files_response(&request, scope, oversized_files())) + .write_all(&files_response(&request, scope, files.clone())) .expect("write oversized result"); }, ); - let result = run_command_with_timeout( - tool_command(&home, &project, &socket, "large"), - CHILD_TIMEOUT, - ); + let output = + run_tool_after_admission(tool_command(&home, &project, &socket, "large"), requests); server.join().expect("join scripted daemon"); - assert!(!result.killed_by_harness, "oversized response hung"); - assert!( - result.output.status.success(), - "{}", - String::from_utf8_lossy(&result.output.stderr) - ); - let preview = output_payload(&result); + let preview = output_payload(&output); assert_eq!(preview["truncated"], true, "{preview}"); let handle = preview["handle"].as_str().expect("stored response handle"); assert_eq!(preview["retrieve_tool"], "tracedecay_retrieve"); @@ -501,7 +545,7 @@ fn generic_tool_retrieves_oversized_typed_result() { "{}", String::from_utf8_lossy(&retrieved.output.stderr) ); - let page = output_payload(&retrieved); + let page = output_payload(&retrieved.output); pages += 1; assert_eq!(page["handle"], handle); assert_eq!(page["offset"], offset); @@ -545,34 +589,12 @@ fn hold_stream_after_files( let _ = BufReader::new(stream).read_line(&mut extra); } -fn assert_complete_json_document(result: &ChildResult) -> Value { - assert!( - !result.killed_by_harness, - "CLI --json hung after writing files: elapsed={:?} stdout_len={} stderr={}", - result.elapsed, - result.output.stdout.len(), - String::from_utf8_lossy(&result.output.stderr) - ); - assert!( - result.output.status.success(), - "{}", - String::from_utf8_lossy(&result.output.stderr) - ); - serde_json::from_slice(&result.output.stdout).unwrap_or_else(|error| { - panic!( - "--json must be one complete parseable document ({error}); bytes={} head={}", - result.output.stdout.len(), - String::from_utf8_lossy(&result.output.stdout[..result.output.stdout.len().min(200)]) - ) - }) -} - #[test] fn generic_tool_json_preserves_large_result_while_daemon_waits() { let (_home, _project, _socket_dir, home, project, socket) = fixture(); let files = oversized_files(); let expected = files.count; - let (_requests, server) = spawn_scripted_daemon( + let (requests, server) = spawn_scripted_daemon( socket.clone(), &home, &project, @@ -581,12 +603,11 @@ fn generic_tool_json_preserves_large_result_while_daemon_waits() { hold_stream_after_files(stream, request, scope, files.clone()); }, ); - let result = run_command_with_timeout( - tool_command(&home, &project, &socket, "large"), - CHILD_TIMEOUT, - ); + let output = + run_tool_after_admission(tool_command(&home, &project, &socket, "large"), requests); server.join().expect("join scripted daemon"); - let envelope = assert_complete_json_document(&result); + let envelope: Value = + serde_json::from_slice(&output.stdout).expect("--json must contain one complete document"); assert_eq!(envelope["isError"], false, "{envelope}"); assert_eq!(envelope["structuredContent"]["count"], expected); let files = envelope["structuredContent"]["files"] @@ -594,9 +615,9 @@ fn generic_tool_json_preserves_large_result_while_daemon_waits() { .unwrap_or_else(|| panic!("held-stream --json must keep the full listing: {envelope}")); assert_eq!(files.len(), expected); assert!( - result.output.stdout.len() > 65_536, + output.stdout.len() > 65_536, "held-stream --json must keep the full listing above 64KiB: {}", - result.output.stdout.len() + output.stdout.len() ); } @@ -732,7 +753,7 @@ fn cancelling_generic_tool_reaps_child_and_closes_request() { // load, and fail fast if the CLI dies or the daemon stops first. loop { match requests.recv_timeout(Duration::from_millis(50)) { - Ok(()) => break, + Ok(_) => break, Err(RecvTimeoutError::Timeout) => assert!( child.try_wait().expect("poll cancellable CLI").is_none(), "CLI exited before the fake daemon received its request" From ec0b338d8d41c836f97dd74bad69a22f42a4f23b Mon Sep 17 00:00:00 2001 From: Zack Jackson <25274700+ScriptedAlchemy@users.noreply.github.com> Date: Sun, 8 Nov 2026 21:34:55 -0800 Subject: [PATCH 17/23] test(cli): share one watchdog across request admission Keep admission receipts for phase diagnostics without granting another response timeout. Both oversized-result journeys use one absolute eight-second deadline for startup, response delivery and process exit. Add a real subprocess regression proving a queued admission receipt cannot restart an already expired watchdog. Preserve all response, retrieval, permission and cancellation assertions. --- .../tool_client_transport.rs | 59 +++++++++++++------ 1 file changed, 40 insertions(+), 19 deletions(-) diff --git a/crates/tracedecay/tests/runtime_acceptance_suite/tool_client_transport.rs b/crates/tracedecay/tests/runtime_acceptance_suite/tool_client_transport.rs index 43a6df1fb54..e17ef06fd5b 100644 --- a/crates/tracedecay/tests/runtime_acceptance_suite/tool_client_transport.rs +++ b/crates/tracedecay/tests/runtime_acceptance_suite/tool_client_transport.rs @@ -79,8 +79,11 @@ fn run_command_with_timeout(mut command: Command, timeout: Duration) -> ChildRes } } -fn run_tool_after_admission(mut command: Command, requests: mpsc::Receiver) -> Output { - let started = Instant::now(); +fn run_tool_after_admission( + mut command: Command, + requests: mpsc::Receiver, + deadline: Instant, +) -> Output { let child = command .stdin(Stdio::null()) .stdout(Stdio::piped()) @@ -88,9 +91,8 @@ fn run_tool_after_admission(mut command: Command, requests: mpsc::Receiver break admitted, Err(RecvTimeoutError::Timeout) => { @@ -101,24 +103,23 @@ fn run_tool_after_admission(mut command: Command, requests: mpsc::Receiver { panic!("scripted daemon stopped before receiving the CLI request") } } }; - assert!( - admitted <= startup_deadline, - "CLI request admission timed out" - ); - // The daemon timestamps the authenticated request. Scheduling this test - // thread later must not extend the response deadline. + assert!(admitted <= deadline, "CLI request admission timed out"); + // Admission diagnoses which phase stalled; it does not grant a second + // watchdog. Startup, response delivery and process exit share the same + // absolute deadline, including delayed observation of a queued receipt. + let remaining = deadline + .checked_duration_since(Instant::now()) + .filter(|remaining| !remaining.is_zero()) + .expect("CLI response deadline expired after admission"); let output = child - .wait_with_output(CHILD_TIMEOUT.saturating_sub(admitted.elapsed())) + .wait_with_output(remaining) .expect("CLI must finish delivering the admitted response within its deadline"); assert!( output.status.success(), @@ -128,6 +129,20 @@ fn run_tool_after_admission(mut command: Command, requests: mpsc::Receiver Date: Mon, 9 Nov 2026 02:02:29 -0800 Subject: [PATCH 18/23] test(cli): use sandbox-writable roots for temporary path policy --- .../cli_non_interactive_test.rs | 45 +++++++++---------- 1 file changed, 22 insertions(+), 23 deletions(-) diff --git a/crates/tracedecay-cli/tests/core_cli_suite/cli_non_interactive_test.rs b/crates/tracedecay-cli/tests/core_cli_suite/cli_non_interactive_test.rs index 3391fe3c5d6..4f3a712b793 100644 --- a/crates/tracedecay-cli/tests/core_cli_suite/cli_non_interactive_test.rs +++ b/crates/tracedecay-cli/tests/core_cli_suite/cli_non_interactive_test.rs @@ -42,18 +42,20 @@ fn ephemeral_safe_fixture_base(test_path: &str) -> Option { assert!(!tracedecay_global_db::is_ephemeral_path(&root)); return Some(root); } - // Worker scratch paths and macOS's per-user temporary directory can leave - // too little room for the nested child home and its Unix daemon socket. - // Use a short, private root on Unix. The private child TMPDIR - // still defines the ephemeral boundary, with durable fixtures beside it. - let scratch = if cfg!(unix) { - TempDir::new_in("/tmp") + // Linux fleet scratch can exceed the Unix socket path limit. Darwin's + // sandbox grants writes to the user's temporary directory, not /tmp. + let parent = if cfg!(target_os = "linux") { + PathBuf::from("/tmp") } else { - TempDir::new() - } - .expect("isolated temp-policy fixture"); - let temporary = scratch.path().join("temporary"); - let durable = scratch.path().join("durable"); + std::env::temp_dir() + }; + let scratch = tempfile::Builder::new() + .prefix("f") + .tempdir_in(parent) + .expect("isolated temp-policy fixture"); + // These components also prefix the child's 104-byte Darwin socket address. + let temporary = scratch.path().join("t"); + let durable = scratch.path().join("d"); std::fs::create_dir(&temporary).unwrap(); std::fs::create_dir(&durable).unwrap(); crate::common::rerun_test_in_child( @@ -1259,15 +1261,12 @@ fn init_wait_holds_until_first_generation_is_ready() { ); } -/// Unix daemon endpoints must fit even when the runner's temporary directory -/// has a long sandbox prefix. The profile remains private to the fixture. -fn daemon_contract_home() -> TempDir { - if cfg!(unix) { - TempDir::new_in("/tmp") - } else { - TempDir::new() - } - .expect("private daemon fixture home") +/// Keep the socket path short and outside the child process's temporary root. +fn daemon_contract_home(durable: &Path) -> TempDir { + tempfile::Builder::new() + .prefix("h") + .tempdir_in(durable) + .expect("private daemon fixture home") } fn assert_project_storage_absent(home: &Path, project: &Path) { @@ -1309,7 +1308,7 @@ fn init_without_a_running_daemon_refuses_before_any_index_write() { ) else { return; }; - let home = daemon_contract_home(); + let home = daemon_contract_home(&durable); let project = tempfile::Builder::new() .prefix("no-daemon-init-project-") .tempdir_in(durable) @@ -1351,7 +1350,7 @@ fn status_and_query_reads_refuse_typed_daemon_unavailable_without_one() { ) else { return; }; - let home = daemon_contract_home(); + let home = daemon_contract_home(&durable); let project = tempfile::Builder::new() .prefix("no-daemon-read-project-") .tempdir_in(durable) @@ -1519,7 +1518,7 @@ fn pricing_and_usage_reads_make_no_http_request() { ) else { return; }; - let home = daemon_contract_home(); + let home = daemon_contract_home(&durable); let project = tempfile::Builder::new() .prefix("local-usage-project-") .tempdir_in(durable) From 05f4a292a6fe09d37ca739842b3df61ce4d0f2e3 Mon Sep 17 00:00:00 2001 From: Zack Jackson <25274700+ScriptedAlchemy@users.noreply.github.com> Date: Mon, 9 Nov 2026 02:01:54 -0800 Subject: [PATCH 19/23] test(transport): pace slow response bytes on one clock --- .../tests/runtime_acceptance_suite/tool_client_transport.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/tracedecay/tests/runtime_acceptance_suite/tool_client_transport.rs b/crates/tracedecay/tests/runtime_acceptance_suite/tool_client_transport.rs index e17ef06fd5b..c94b3bc22ef 100644 --- a/crates/tracedecay/tests/runtime_acceptance_suite/tool_client_transport.rs +++ b/crates/tracedecay/tests/runtime_acceptance_suite/tool_client_transport.rs @@ -390,10 +390,14 @@ fn generic_tool_accepts_slow_byte_stream() { &project, 1, |mut stream, request, scope| { + // Pace each byte against one clock so scheduler delays do not + // accumulate across the entire response frame. + let mut send_at = Instant::now(); for byte in response_bytes(&request, scope, "slow-ok") { + std::thread::sleep(send_at.saturating_duration_since(Instant::now())); stream.write_all(&[byte]).expect("write slow byte"); stream.flush().expect("flush slow byte"); - std::thread::sleep(Duration::from_millis(2)); + send_at += Duration::from_millis(2); } }, ); From eb11bce2b44d2b0f0e204f782f0b964890d83958 Mon Sep 17 00:00:00 2001 From: Zack Jackson <25274700+ScriptedAlchemy@users.noreply.github.com> Date: Mon, 9 Nov 2026 03:02:33 -0800 Subject: [PATCH 20/23] test(cli): keep orphan store sockets within Darwin worker limits --- .../tests/core_cli_suite/cli_non_interactive_test.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/crates/tracedecay-cli/tests/core_cli_suite/cli_non_interactive_test.rs b/crates/tracedecay-cli/tests/core_cli_suite/cli_non_interactive_test.rs index 4f3a712b793..9c309d9b570 100644 --- a/crates/tracedecay-cli/tests/core_cli_suite/cli_non_interactive_test.rs +++ b/crates/tracedecay-cli/tests/core_cli_suite/cli_non_interactive_test.rs @@ -1261,11 +1261,14 @@ fn init_wait_holds_until_first_generation_is_ready() { ); } -/// Keep the socket path short and outside the child process's temporary root. -fn daemon_contract_home(durable: &Path) -> TempDir { +/// Allocate a short socket path within the caller's private fixture root. +fn daemon_contract_home(root: &Path) -> TempDir { + // The outer scratch already isolates this home. Keep its component one byte + // so the default socket fits even in the native worker scratch directory. tempfile::Builder::new() .prefix("h") - .tempdir_in(durable) + .rand_bytes(0) + .tempdir_in(root) .expect("private daemon fixture home") } @@ -2835,7 +2838,7 @@ fn list_all_reports_orphan_manifest_reconstructable_store() { ) else { return; }; - let home = TempDir::new().unwrap(); + let home = daemon_contract_home(&std::env::temp_dir()); let project = tempfile::Builder::new() .prefix("list-orphan-project-") .tempdir_in(durable) From 9cb2657d0b3986d616c5815c66844a4986791034 Mon Sep 17 00:00:00 2001 From: Zack Jackson <25274700+ScriptedAlchemy@users.noreply.github.com> Date: Mon, 9 Nov 2026 03:00:10 -0800 Subject: [PATCH 21/23] test(hooks): initialize and ingest on the same ready daemon --- .../advisory_runtime_acceptance.rs | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/crates/tracedecay/tests/runtime_acceptance_suite/advisory_runtime_acceptance.rs b/crates/tracedecay/tests/runtime_acceptance_suite/advisory_runtime_acceptance.rs index b6dd4a3e4f5..647e85d13cb 100644 --- a/crates/tracedecay/tests/runtime_acceptance_suite/advisory_runtime_acceptance.rs +++ b/crates/tracedecay/tests/runtime_acceptance_suite/advisory_runtime_acceptance.rs @@ -2,7 +2,7 @@ use std::collections::{BTreeSet, VecDeque}; use std::io::SeekFrom; -use std::process::{Command, Stdio}; +use std::process::Command; use std::sync::{Arc, Mutex}; use std::time::Duration; @@ -978,18 +978,13 @@ async fn packaged_host_ingest_delivers_a_registered_advisory_cycle() { String::from_utf8_lossy(&output.stderr) ); } - common::initialize_tracedecay_cli_project(environment.home(), &project); let daemon_log = environment.home().join("advisory-daemon.log"); - let _daemon = common::spawn_tracedecay_daemon_with(environment.home(), |command| { - // `.cargo/config.toml` sets TRACEDECAY_DISABLE_GLOBAL_DB=1 so cargo - // children never touch the operator ledger. This journey depends on - // the registered profile accounting owner for hint-outcome settlement. - command - .env_remove("TRACEDECAY_DISABLE_GLOBAL_DB") - .stderr(Stdio::from( - std::fs::File::create(&daemon_log).expect("create isolated advisory daemon log"), - )); - }); + let _daemon = + common::spawn_tracedecay_daemon_logged(environment.home(), &daemon_log, |command| { + // Hint-outcome settlement requires the isolated profile's accounting owner. + command.env_remove("TRACEDECAY_DISABLE_GLOBAL_DB"); + }); + common::initialize_tracedecay_cli_project(environment.home(), &project); let transcript = project.join("cursor-proximity.jsonl"); std::fs::write( &transcript, From 4339b7c2578fd7ade36331f9c6e7716736fc664d Mon Sep 17 00:00:00 2001 From: Zack Jackson <25274700+ScriptedAlchemy@users.noreply.github.com> Date: Mon, 9 Nov 2026 22:19:27 -0800 Subject: [PATCH 22/23] fix(retention): preserve progress across contended stores --- .../src/writer_gate.rs | 261 +++++++-- crates/tracedecay/src/daemon/branch_admin.rs | 29 +- crates/tracedecay/src/daemon/maintenance.rs | 516 +++++++++++++----- 3 files changed, 592 insertions(+), 214 deletions(-) diff --git a/crates/tracedecay-store-runtime/src/writer_gate.rs b/crates/tracedecay-store-runtime/src/writer_gate.rs index e2d572d8e20..f3dd1c0ebe7 100644 --- a/crates/tracedecay-store-runtime/src/writer_gate.rs +++ b/crates/tracedecay-store-runtime/src/writer_gate.rs @@ -21,8 +21,12 @@ //! └── content: Mutex //! ``` //! -//! Every acquisition takes these in exactly that order, so no lock-order -//! inversion is possible. +//! Admitted operations hold this hierarchy. A queued store operation first +//! reserves its local resources without holding daemon admission. It only +//! *tries* the daemon read gate with that reservation held; if the daemon is +//! busy, it releases every local resource before awaiting the daemon. Thus +//! no local waiter pins daemon admission, and no task waits for the daemon +//! while holding a resource another daemon reader could need. //! //! # Exclusivity argument //! @@ -110,6 +114,28 @@ impl StoreGate { StoreWriterClass::Content => Arc::clone(&self.content), } } + + fn try_reserve( + &self, + class: Option, + ) -> Option<(StoreMaintenanceGuard, Option>)> { + match class { + Some(class) => { + let maintenance = Arc::clone(&self.maintenance).try_read_owned().ok()?; + let class_guard = self.class_mutex(class).try_lock_owned().ok()?; + Some(( + StoreMaintenanceGuard::Shared(maintenance), + Some(class_guard), + )) + } + None => Some(( + StoreMaintenanceGuard::Exclusive( + Arc::clone(&self.maintenance).try_write_owned().ok()?, + ), + None, + )), + } + } } /// Admission held for the duration of one writer operation. @@ -205,48 +231,78 @@ impl StoreWriterGates { _gate: None, }, WriterScope::Store { data_root, class } => { - let daemon = tracing::Instrument::instrument( - Arc::clone(&self.daemon).read_owned(), - tracing::trace_span!("daemon.writer_gate.acquire.store"), - ) - .await; - let gate = self.store_gate(data_root); - let maintenance = tracing::Instrument::instrument( - Arc::clone(&gate.maintenance).read_owned(), - tracing::trace_span!("daemon.writer_gate.acquire.store_read"), - ) - .await; - let class_guard = tracing::Instrument::instrument( - gate.class_mutex(*class).lock_owned(), - tracing::trace_span!("daemon.writer_gate.acquire.class"), - ) - .await; - WriterAdmissionGuard { - _class: Some(class_guard), - _maintenance: Some(StoreMaintenanceGuard::Shared(maintenance)), + self.acquire_store(data_root, Some(*class)).await + } + WriterScope::StoreMaintenance { data_root } => { + self.acquire_store(data_root, None).await + } + } + } + + /// Reserve the store without retaining daemon admission while queued. + /// `None` is exclusive maintenance; owner/content reserve a shared store + /// guard plus their class mutex. + async fn acquire_store( + &self, + data_root: &Path, + class: Option, + ) -> WriterAdmissionGuard { + let gate = self.store_gate(data_root); + loop { + let (maintenance, class_guard) = match class { + Some(class) => { + let maintenance = tracing::Instrument::instrument( + Arc::clone(&gate.maintenance).read_owned(), + tracing::trace_span!("daemon.writer_gate.acquire.store_read"), + ) + .await; + let class_guard = tracing::Instrument::instrument( + gate.class_mutex(class).lock_owned(), + tracing::trace_span!("daemon.writer_gate.acquire.class"), + ) + .await; + ( + StoreMaintenanceGuard::Shared(maintenance), + Some(class_guard), + ) + } + None => { + let maintenance = tracing::Instrument::instrument( + Arc::clone(&gate.maintenance).write_owned(), + tracing::trace_span!("daemon.writer_gate.acquire.store_maintenance"), + ) + .await; + (StoreMaintenanceGuard::Exclusive(maintenance), None) + } + }; + if let Ok(daemon) = Arc::clone(&self.daemon).try_read_owned() { + return WriterAdmissionGuard { + _class: class_guard, + _maintenance: Some(maintenance), _daemon: DaemonGuard::Shared(daemon), _gate: Some(gate), - } + }; } - WriterScope::StoreMaintenance { data_root } => { - let daemon = tracing::Instrument::instrument( - Arc::clone(&self.daemon).read_owned(), - tracing::trace_span!("daemon.writer_gate.acquire.store"), - ) - .await; - let gate = self.store_gate(data_root); - let maintenance = tracing::Instrument::instrument( - Arc::clone(&gate.maintenance).write_owned(), - tracing::trace_span!("daemon.writer_gate.acquire.store_maintenance"), - ) - .await; - WriterAdmissionGuard { - _class: None, - _maintenance: Some(StoreMaintenanceGuard::Exclusive(maintenance)), + // Take our fair daemon turn without awaiting any local resource + // under it. An immediate local probe can use this turn even when + // another daemon writer is queued; dropping the turn first could + // starve a store behind continuous daemon-wide work. + drop(class_guard); + drop(maintenance); + let daemon = tracing::Instrument::instrument( + Arc::clone(&self.daemon).read_owned(), + tracing::trace_span!("daemon.writer_gate.acquire.store"), + ) + .await; + if let Some((maintenance, class_guard)) = gate.try_reserve(class) { + return WriterAdmissionGuard { + _class: class_guard, + _maintenance: Some(maintenance), _daemon: DaemonGuard::Shared(daemon), _gate: Some(gate), - } + }; } + drop(daemon); } } @@ -261,30 +317,27 @@ impl StoreWriterGates { _gate: None, }), WriterScope::Store { data_root, class } => { - let daemon = Arc::clone(&self.daemon).try_read_owned().ok()?; - let gate = self.store_gate(data_root); - let maintenance = Arc::clone(&gate.maintenance).try_read_owned().ok()?; - let class_guard = gate.class_mutex(*class).try_lock_owned().ok()?; - Some(WriterAdmissionGuard { - _class: Some(class_guard), - _maintenance: Some(StoreMaintenanceGuard::Shared(maintenance)), - _daemon: DaemonGuard::Shared(daemon), - _gate: Some(gate), - }) - } - WriterScope::StoreMaintenance { data_root } => { - let daemon = Arc::clone(&self.daemon).try_read_owned().ok()?; - let gate = self.store_gate(data_root); - let maintenance = Arc::clone(&gate.maintenance).try_write_owned().ok()?; - Some(WriterAdmissionGuard { - _class: None, - _maintenance: Some(StoreMaintenanceGuard::Exclusive(maintenance)), - _daemon: DaemonGuard::Shared(daemon), - _gate: Some(gate), - }) + self.try_acquire_store(data_root, Some(*class)) } + WriterScope::StoreMaintenance { data_root } => self.try_acquire_store(data_root, None), } } + + fn try_acquire_store( + &self, + data_root: &Path, + class: Option, + ) -> Option { + let daemon = Arc::clone(&self.daemon).try_read_owned().ok()?; + let gate = self.store_gate(data_root); + let (maintenance, class_guard) = gate.try_reserve(class)?; + Some(WriterAdmissionGuard { + _class: class_guard, + _maintenance: Some(maintenance), + _daemon: DaemonGuard::Shared(daemon), + _gate: Some(gate), + }) + } } #[cfg(test)] @@ -434,6 +487,96 @@ mod tests { assert!(gates.try_acquire(&maintenance_scope).is_some()); } + #[tokio::test] + async fn queued_maintenance_yields_to_a_daemon_writer_without_a_lock_cycle() { + let gates = StoreWriterGates::default(); + let held = gates.acquire(&store("/a", StoreWriterClass::Content)).await; + let scope = WriterScope::maintenance("/a"); + let maintenance = gates.acquire(&scope); + tokio::pin!(maintenance); + poll_fn(|cx| { + assert!(maintenance.as_mut().poll(cx).is_pending()); + Poll::Ready(()) + }) + .await; + let follower_scope = store("/a", StoreWriterClass::Owner); + let follower = gates.acquire(&follower_scope); + tokio::pin!(follower); + poll_fn(|cx| { + assert!(follower.as_mut().poll(cx).is_pending()); + Poll::Ready(()) + }) + .await; + let daemon_scope = WriterScope::Daemon; + let daemon = gates.acquire(&daemon_scope); + tokio::pin!(daemon); + poll_fn(|cx| { + assert!(daemon.as_mut().poll(cx).is_pending()); + Poll::Ready(()) + }) + .await; + drop(held); + // Neither queued local future is repolled. Their reservations must + // not retain daemon admission, even after their store becomes free. + let exclusive = tokio::time::timeout(Duration::from_secs(5), &mut daemon) + .await + .expect("daemon writer progresses independently of queued store futures"); + assert!(gates.try_acquire(&scope).is_none()); + poll_fn(|cx| { + assert!(maintenance.as_mut().poll(cx).is_pending()); + assert!(follower.as_mut().poll(cx).is_pending()); + Poll::Ready(()) + }) + .await; + drop(exclusive); + let retained = tokio::time::timeout(Duration::from_secs(5), &mut maintenance) + .await + .expect("maintenance resumes after daemon writer"); + drop(retained); + let following = tokio::time::timeout(Duration::from_secs(5), &mut follower) + .await + .expect("store follower resumes after maintenance"); + drop(following); + } + + #[tokio::test] + async fn a_queued_store_uses_its_fair_daemon_turn() { + for scope in [ + WriterScope::maintenance("/a"), + store("/a", StoreWriterClass::Content), + ] { + let gates = StoreWriterGates::default(); + let held = gates.acquire(&WriterScope::Daemon).await; + let store = gates.acquire(&scope); + tokio::pin!(store); + poll_fn(|cx| { + assert!(store.as_mut().poll(cx).is_pending()); + Poll::Ready(()) + }) + .await; + let daemon_scope = WriterScope::Daemon; + let following = gates.acquire(&daemon_scope); + tokio::pin!(following); + poll_fn(|cx| { + assert!(following.as_mut().poll(cx).is_pending()); + Poll::Ready(()) + }) + .await; + drop(held); + // The later daemon writer stays queued and unpolled. The store + // must consume its earlier shared turn rather than discard it. + let admitted = tokio::time::timeout(Duration::from_secs(5), &mut store) + .await + .expect("store uses its fair daemon turn before the next daemon writer"); + assert!(gates.try_acquire(&WriterScope::Daemon).is_none()); + drop(admitted); + let next = tokio::time::timeout(Duration::from_secs(5), &mut following) + .await + .expect("following daemon writer progresses after the store"); + drop(next); + } + } + #[tokio::test] async fn retired_store_gates_are_reclaimed() { let gates = StoreWriterGates::default(); diff --git a/crates/tracedecay/src/daemon/branch_admin.rs b/crates/tracedecay/src/daemon/branch_admin.rs index 5c891316169..83a41efa7f4 100644 --- a/crates/tracedecay/src/daemon/branch_admin.rs +++ b/crates/tracedecay/src/daemon/branch_admin.rs @@ -1933,28 +1933,15 @@ impl StoreAdministration { operation().await } - /// Cancellation applies only while queued for writer admission. - /// Once the guard is acquired, the operation must finish cooperatively: - /// dropping an in-flight journal/retention future would abandon physical - /// blocking work whose client survives the daemon's shutdown join. - pub(super) async fn with_writer_cancellable_acquisition( + /// Owns only the canonical gate while retention waits across ticks. The + /// caller polls this acquisition without waiting in the serial tick and + /// drops it on cancellation or when the exact store leaves the work list. + pub(super) fn retention_writer_acquisition( &self, - cancellation: &tracedecay_runtime_core::cancellation::CancellationToken, - scope: WriterScope, - operation: Operation, - ) -> Option - where - Operation: FnOnce() -> OperationFuture, - OperationFuture: Future, - { - let _writer = tokio::select! { - biased; - () = cancellation.cancelled() => return None, - writer = crate::daemon::core_admission::park_admission( - self.gate.acquire(&scope) - ) => writer, - }; - Some(operation().await) + data_root: PathBuf, + ) -> impl Future + Send + 'static { + let gate = Arc::clone(&self.gate); + async move { gate.acquire(&WriterScope::maintenance(data_root)).await } } pub(super) async fn try_with_writer( diff --git a/crates/tracedecay/src/daemon/maintenance.rs b/crates/tracedecay/src/daemon/maintenance.rs index a62aa22d878..661e28983e9 100644 --- a/crates/tracedecay/src/daemon/maintenance.rs +++ b/crates/tracedecay/src/daemon/maintenance.rs @@ -4,6 +4,7 @@ use std::path::{Path, PathBuf}; use std::sync::Arc; use std::time::{Duration, Instant}; +use futures_util::{FutureExt, future::BoxFuture}; use tokio::sync::Mutex; use tracedecay_code_index::parallelism::{ collect_idle_installed_worker_heaps, collect_installed_worker_heaps_when_idle, @@ -31,38 +32,83 @@ use tracedecay_daemon_service::shutdown::DAEMON_TASK_ABORT_DEADLINE; use tracedecay_domain::process_heap::request_idle_thread_collection_v1; use tracedecay_runtime_core::logging::log_daemon_event; use tracedecay_runtime_core::native_runtime::{self, NativeTask, TimeoutError}; -use tracedecay_store_runtime::WriterScope; +use tracedecay_store_runtime::{WriterAdmissionGuard, WriterScope}; const MAINTENANCE_STORE_PAGE_LIMIT: usize = 8; const REGISTERED_PROJECT_PAGE_LIMIT: usize = 64; -/// Fast probes preserve low-latency work. After two store-local refusals, -/// queue behind that store's writers without stalling independent projects. -const MAINTENANCE_WRITER_FAST_DEFERRALS: u8 = 2; +/// Fair store-local acquisitions survive retry ticks without parking the tick +/// or retaining a database client. Only the current tick may consume admission +/// and run work; cancellation and removal drop the pending queue positions. +/// Work keys distinguish mounted branches sharing a store; pairing the key +/// with its canonical root prevents a same-key replacement reusing admission. +#[derive(Default)] +struct RetentionWriterReservations { + pending: + std::sync::Mutex>>, +} + +impl RetentionWriterReservations { + fn clear(&self) { + self.pending + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .clear(); + } + + fn retain(&self, active: &BTreeSet<(String, PathBuf)>) { + self.pending + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .retain(|root, _| active.contains(root)); + } +} async fn admit_retention_writer( administration: &StoreAdministration, cancellation: &tracedecay_runtime_core::cancellation::CancellationToken, scope: WriterScope, - prior_deferrals: u8, + reservations: &RetentionWriterReservations, + work_key: &str, operation: Operation, ) -> Option where Operation: FnOnce() -> OperationFuture, OperationFuture: Future, { - // Shutdown observed before admission must not start another retention - // unit, even when a fast probe could acquire the writer gate immediately. if cancellation.is_cancelled() { + reservations.clear(); return None; } - if prior_deferrals < MAINTENANCE_WRITER_FAST_DEFERRALS { - administration.try_with_writer_in(scope, operation).await - } else { - // Only cancel the wait: once admitted, journal/retention work must - // finish cooperatively rather than be dropped at the writer boundary. - administration - .with_writer_cancellable_acquisition(cancellation, scope, operation) - .await - } + let WriterScope::StoreMaintenance { data_root } = scope else { + // Global maintenance remains a nonqueued probe: it must never put + // unrelated store writers behind an unbounded daemon-wide wait. + return administration.try_with_writer_in(scope, operation).await; + }; + let _writer = { + let mut pending = reservations + .pending + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + // Cancellation can race the lock. Do not recreate a reservation after + // shutdown prepare has synchronously cleared this table. + if cancellation.is_cancelled() { + pending.clear(); + return None; + } + let key = (work_key.to_owned(), data_root.clone()); + let acquisition = pending.entry(key.clone()).or_insert_with(|| { + administration + .retention_writer_acquisition(data_root.clone()) + .boxed() + }); + // Poll once, retaining the exact same fair queue position on refusal. + // Retry cadence owns the next poll; there is no independent job owner. + let admitted = acquisition.as_mut().now_or_never()?; + pending.remove(&key); + admitted + }; + // Once admitted, journal/retention work must finish cooperatively rather + // than be dropped at the writer boundary. + Some(operation().await) } /// Every registered project whose profile shard exists on disk, keyed for the @@ -650,8 +696,8 @@ pub(super) struct MaintenanceCoordinator { /// store is starved when the mounted set exceeds `MAINTENANCE_STORE_PAGE_LIMIT`. store_cursor: Arc>>, store_continuations: Arc>, - /// Per-store writer admission refusals, bounded and reset on a real admission. - writer_deferrals: Arc>>, + /// Pending canonical store acquisitions; no admitted work lives here. + writer_reservations: Arc, /// Instant of the last branch-store GC pass that succeeded for every /// mounted project. `None` keeps the daily cadence retry-eligible. last_branch_gc: Arc>>, @@ -673,7 +719,7 @@ impl Default for MaintenanceCoordinator { metrics: Arc::new(Mutex::new(MaintenanceMetricsV1::default())), store_cursor: Arc::new(Mutex::new(None)), store_continuations: Arc::new(Mutex::new(StoreMaintenanceContinuations::default())), - writer_deferrals: Arc::new(Mutex::new(BTreeMap::new())), + writer_reservations: Arc::new(RetentionWriterReservations::default()), last_branch_gc: Arc::new(Mutex::new(None)), resident_memory_sampler: Arc::new(Mutex::new(None)), resident_memory_log: Arc::new(std::sync::Mutex::new( @@ -873,6 +919,7 @@ impl MaintenanceCoordinator { /// join below stays correct whether or not it already ran. pub(super) fn cancel(&self) { self.cancellation.cancel(); + self.writer_reservations.clear(); } pub(super) async fn shutdown(&self) { @@ -955,6 +1002,9 @@ impl MaintenanceCoordinator { let profile_database = match administration.registered_profile_database().await { Ok(database) => database, Err(error) => { + // Without profile authority there is no current work + // census to justify retaining store queue positions. + self.writer_reservations.clear(); log_daemon_event( "retention_degraded", &[ @@ -1112,10 +1162,15 @@ impl MaintenanceCoordinator { work.sort_by(|left, right| left.0.cmp(&right.0)); let keys = work.iter().map(|(key, _)| key.clone()).collect::>(); self.store_continuations.lock().await.retain_stores(&keys); - self.writer_deferrals - .lock() - .await - .retain(|key, _| keys.binary_search(key).is_ok()); + self.writer_reservations.retain( + &work + .iter() + .filter_map(|(key, unit)| match unit.writer_scope() { + WriterScope::StoreMaintenance { data_root } => Some((key.clone(), data_root)), + WriterScope::Daemon | WriterScope::Store { .. } => None, + }) + .collect(), + ); let after = self.store_cursor.lock().await.clone(); let (window, _) = select_store_window(&keys, after.as_deref(), MAINTENANCE_STORE_PAGE_LIMIT); @@ -1181,24 +1236,12 @@ impl MaintenanceCoordinator { break; } let scope = work[index].1.writer_scope(); - // Only store-local scopes may queue. A queued global writer would - // block every unrelated project while one long sync holds a read gate. - let local = !matches!(&scope, WriterScope::Daemon); - let prior_deferrals = if local { - self.writer_deferrals - .lock() - .await - .get(&keys[index]) - .copied() - .unwrap_or(0) - } else { - 0 - }; let admitted = admit_retention_writer( administration, &self.cancellation, scope, - prior_deferrals, + &self.writer_reservations, + &keys[index], || async { let Ok(_clients) = administration.maintenance_store_clients().try_lock() else { return MaintenanceTickOutcome::Retry; @@ -1220,7 +1263,10 @@ impl MaintenanceCoordinator { run_registered_store_retention(&database, retention).await } } - MaintenanceStoreWork::Graph { .. } => { + MaintenanceStoreWork::Graph { + database_path, + data_root, + } => { let Some(graph) = administration .mounted_project_graphs() .await @@ -1229,6 +1275,14 @@ impl MaintenanceCoordinator { else { return MaintenanceTickOutcome::Complete; }; + // A same-key remount can replace its store between + // listing and admission. The old gate cannot + // authorize maintenance against the replacement. + if graph.db().database_path() != database_path + || graph.store_layout().data_root != *data_root + { + return MaintenanceTickOutcome::Retry; + } run_project_generation_maintenance( &project_store_maintenance_lease(&graph), code_index_schedulers, @@ -1254,14 +1308,6 @@ impl MaintenanceCoordinator { }, ) .await; - { - let mut deferrals = self.writer_deferrals.lock().await; - if admitted.is_some() { - deferrals.remove(&keys[index]); - } else if local && !self.cancellation.is_cancelled() { - deferrals.insert(keys[index].clone(), prior_deferrals.saturating_add(1)); - } - } attempted = attempted.saturating_add(1); match admitted { Some(unit_outcome) => { @@ -1629,11 +1675,10 @@ mod tests { use tracedecay_runtime_core::cancellation::CancellationToken; use tracedecay_runtime_core::native_runtime; - /// Hold a writer for the target store across two failed fast probes. - /// The third retention attempt joins its local queue; independent project - /// opens must remain admitted while that writer is still active. + /// Retry ticks retain one fair queue position, with no maintenance task + /// owning a daemon read guard while the target store is busy. #[tokio::test] - async fn retention_queues_after_bounded_writer_deferrals() { + async fn retention_retains_one_fair_acquisition_across_retry_ticks() { let administration = StoreAdministration::default(); let (entered_tx, entered_rx) = tokio::sync::oneshot::channel(); let (release_tx, release_rx) = tokio::sync::oneshot::channel(); @@ -1651,24 +1696,44 @@ mod tests { }); entered_rx.await.expect("writer acquired its store gate"); let cancellation = CancellationToken::new(); - for refused in 0..super::MAINTENANCE_WRITER_FAST_DEFERRALS { + let reservations = super::RetentionWriterReservations::default(); + for _ in 0..4 { assert_eq!( super::admit_retention_writer( &administration, &cancellation, WriterScope::maintenance("/retained-store"), - refused, + &reservations, + "retention", || async { 41_u8 }, ) .await, None, - "a store writer blocks the nonqueued store-local probe" + "a held store defers only its own maintenance unit" ); + assert_eq!(reservations.pending.lock().unwrap().len(), 1); } - // Establish ongoing writer activity before queueing the exclusive - // maintenance request. A fair queued maintenance writer blocks later readers of that store; - // waiting for their progress before releasing the holder deadlocks. + assert_eq!( + super::admit_retention_writer( + &administration, + &cancellation, + WriterScope::maintenance("/retained-store"), + &reservations, + "another-branch", + || async { 45_u8 }, + ) + .await, + None, + ); + assert_eq!( + reservations.pending.lock().unwrap().len(), + 2, + "distinct work units in one store retain distinct fair queue positions" + ); + + // Keep unrelated writer activity running through retention admission. + // Same-store readers must wait behind the retained fair acquisitions. let competing_admin = administration.clone(); let writer_entries = Arc::new(AtomicUsize::new(0)); let entries = Arc::clone(&writer_entries); @@ -1695,22 +1760,6 @@ mod tests { .await .expect("unrelated writers churn before retention queues"); - let queued = super::admit_retention_writer( - &administration, - &cancellation, - WriterScope::maintenance("/retained-store"), - super::MAINTENANCE_WRITER_FAST_DEFERRALS, - || async { 42_u8 }, - ); - tokio::pin!(queued); - poll_fn(|cx| { - assert!( - queued.as_mut().poll(cx).is_pending(), - "the retention request must queue behind the held writer" - ); - Poll::Ready(()) - }) - .await; let later_writer = administration.with_writer_in( WriterScope::store("/retained-store", StoreWriterClass::Owner), || async { 43_u8 }, @@ -1738,12 +1787,32 @@ mod tests { ); let before_release = writer_entries.load(Ordering::SeqCst); release_tx.send(()).expect("release the original writer"); + holder.await.expect("first writer finishes"); assert_eq!( - tokio::time::timeout(Duration::from_secs(5), &mut queued) - .await - .expect("queued retention is admitted despite competing writers"), + super::admit_retention_writer( + &administration, + &cancellation, + WriterScope::maintenance("/retained-store"), + &reservations, + "retention", + || async { 42_u8 }, + ) + .await, Some(42) ); + assert_eq!( + super::admit_retention_writer( + &administration, + &cancellation, + WriterScope::maintenance("/retained-store"), + &reservations, + "another-branch", + || async { 45_u8 }, + ) + .await, + Some(45), + "the first work unit cannot consume another branch's queued acquisition", + ); assert_eq!( tokio::time::timeout(Duration::from_secs(5), &mut later_writer) .await @@ -1760,7 +1829,7 @@ mod tests { .await .expect("unrelated writers resume after maintenance admission"); stop_churn.cancel(); - holder.await.expect("first writer finishes"); + assert!(reservations.pending.lock().unwrap().is_empty()); competitors.await.expect("unrelated writer churn finishes"); assert!(writer_entries.load(Ordering::SeqCst) > before_release); } @@ -1771,13 +1840,15 @@ mod tests { let cancellation = CancellationToken::new(); cancellation.cancel(); let constructed = AtomicUsize::new(0); - for prior_deferrals in 0..=super::MAINTENANCE_WRITER_FAST_DEFERRALS { + let reservations = super::RetentionWriterReservations::default(); + for scope in [WriterScope::maintenance("/held-store"), WriterScope::Daemon] { assert_eq!( super::admit_retention_writer( &administration, &cancellation, - WriterScope::maintenance("/held-store"), - prior_deferrals, + scope, + &reservations, + "retention", || { constructed.fetch_add(1, Ordering::SeqCst); std::future::ready(7_u8) @@ -1785,7 +1856,7 @@ mod tests { ) .await, None, - "cancelled retention must refuse even an uncontended fast probe" + "cancelled retention refuses even an uncontended acquisition" ); } assert_eq!(constructed.load(Ordering::SeqCst), 0); @@ -1816,24 +1887,23 @@ mod tests { entered_rx .await .expect("writer holds shared daemon admission"); - let cancellation = CancellationToken::new(); + let coordinator = MaintenanceCoordinator::default(); let constructed = AtomicUsize::new(0); - let waiting = super::admit_retention_writer( - &administration, - &cancellation, - WriterScope::maintenance("/held-store"), - super::MAINTENANCE_WRITER_FAST_DEFERRALS, - || { - constructed.fetch_add(1, Ordering::SeqCst); - std::future::ready(7_u8) - }, + assert_eq!( + super::admit_retention_writer( + &administration, + &coordinator.cancellation, + WriterScope::maintenance("/held-store"), + &coordinator.writer_reservations, + "retention", + || { + constructed.fetch_add(1, Ordering::SeqCst); + std::future::ready(7_u8) + }, + ) + .await, + None ); - tokio::pin!(waiting); - poll_fn(|cx| { - assert!(waiting.as_mut().poll(cx).is_pending()); - Poll::Ready(()) - }) - .await; let follower = administration.with_writer_in( WriterScope::store("/held-store", StoreWriterClass::Content), || async { 8_u8 }, @@ -1844,12 +1914,14 @@ mod tests { Poll::Ready(()) }) .await; - cancellation.cancel(); - assert_eq!( - tokio::time::timeout(Duration::from_secs(2), &mut waiting) - .await - .expect("cancelled acquisition completes"), - None + coordinator.shutdown().await; + assert!( + coordinator + .writer_reservations + .pending + .lock() + .unwrap() + .is_empty() ); assert_eq!(constructed.load(Ordering::SeqCst), 0); // Owner and content otherwise run concurrently. Only queued exclusive @@ -1868,6 +1940,111 @@ mod tests { ); } + #[tokio::test] + async fn retired_store_reservations_release_followers_and_replacement_uses_its_own_gate() { + let temp = tempfile::tempdir().expect("store roots"); + let retired = temp.path().join("retired"); + let replacement = temp.path().join("replacement"); + std::fs::create_dir_all(&retired).expect("retired root"); + std::fs::create_dir_all(&replacement).expect("replacement root"); + let administration = StoreAdministration::default(); + let coordinator = MaintenanceCoordinator::default(); + let (entered_tx, entered_rx) = tokio::sync::oneshot::channel(); + let (release_tx, release_rx) = tokio::sync::oneshot::channel(); + let holding_admin = administration.clone(); + let owner_scope = + crate::daemon::branch_admin::store_writer_scope(&retired, StoreWriterClass::Owner); + let owner = tokio::spawn(async move { + holding_admin + .with_writer_in(owner_scope, || async { + entered_tx.send(()).expect("owner admitted"); + release_rx.await.expect("owner released"); + }) + .await; + }); + entered_rx.await.expect("held retired store"); + let work = AtomicUsize::new(0); + // Different spellings of the same canonical store share one pending + // acquisition, rather than competing maintenance jobs. + for root in [&retired, &retired.join(".")] { + assert_eq!( + super::admit_retention_writer( + &administration, + &coordinator.cancellation, + super::store_maintenance_writer_scope(root), + &coordinator.writer_reservations, + "retention", + || async { work.fetch_add(1, Ordering::SeqCst) }, + ) + .await, + None + ); + } + assert_eq!( + coordinator + .writer_reservations + .pending + .lock() + .unwrap() + .len(), + 1 + ); + let following = administration.with_writer_in( + crate::daemon::branch_admin::store_writer_scope(&retired, StoreWriterClass::Content), + || async { 8_u8 }, + ); + tokio::pin!(following); + poll_fn(|cx| { + assert!(following.as_mut().poll(cx).is_pending()); + Poll::Ready(()) + }) + .await; + let replacement_scope = super::store_maintenance_writer_scope(&replacement); + let WriterScope::StoreMaintenance { data_root } = &replacement_scope else { + panic!("replacement has canonical identity"); + }; + // The current mounted-work census keeps only the replacement identity. + coordinator + .writer_reservations + .retain(&std::collections::BTreeSet::from([( + "retention".to_owned(), + data_root.clone(), + )])); + assert_eq!( + tokio::time::timeout(Duration::from_secs(2), &mut following) + .await + .expect("retirement drops the old queue position while its owner is still active"), + 8 + ); + assert_eq!( + super::admit_retention_writer( + &administration, + &coordinator.cancellation, + replacement_scope, + &coordinator.writer_reservations, + "retention", + || async { work.fetch_add(1, Ordering::SeqCst) }, + ) + .await, + Some(0) + ); + assert_eq!( + work.load(Ordering::SeqCst), + 1, + "only the replacement executes maintenance" + ); + assert!( + coordinator + .writer_reservations + .pending + .lock() + .unwrap() + .is_empty() + ); + release_tx.send(()).expect("release retired owner"); + owner.await.expect("retired owner settles"); + } + /// Cancellation after writer admission must not drop a running retention /// operation: its blocking/journaled work is responsible for cooperating. #[tokio::test] @@ -1883,7 +2060,8 @@ mod tests { &working, &stopping, WriterScope::maintenance("/retention-store"), - super::MAINTENANCE_WRITER_FAST_DEFERRALS, + &super::RetentionWriterReservations::default(), + "retention", move || async move { entered_tx .send(()) @@ -2089,21 +2267,20 @@ mod tests { .expect("sampler joins after cancellation"); } - /// A registered project nothing has mounted since the daemon started is - /// collected by the first full tick and converges on its continuation, - /// instead of waiting for a client to mount it. A tick that runs while a - /// capacity release holds the maintenance store clients defers it. + /// Busy registered store A cannot park free store B or the profile sweep. + /// After release, the same acquisition lets A converge without a separate + /// maintenance job. Capacity release still excludes store clients. #[test] - fn a_full_tick_collects_an_unmounted_registered_project_without_a_tokio_runtime() { + fn full_ticks_preserve_independent_store_progress_without_a_tokio_runtime() { let hazards = crate::test_support::native_runtime_hazards(); tracedecay_runtime_core::native_runtime::block_on( - a_full_tick_collects_an_unmounted_registered_project(), + full_ticks_preserve_independent_store_progress(), ) .expect("native runtime"); assert_eq!(crate::test_support::native_runtime_hazards(), hazards); } - async fn a_full_tick_collects_an_unmounted_registered_project() { + async fn full_ticks_preserve_independent_store_progress() { assert!(tokio::runtime::Handle::try_current().is_err()); let temp = tempfile::TempDir::new().expect("temp root"); let profile_root = temp.path().join("profile"); @@ -2178,10 +2355,8 @@ mod tests { ); drop(release); - // A long-lived writer for this project holds the store-local read gate. - // Two real retention ticks must defer, and the next must queue rather - // than missing the same writer again. The fixture's six on-disk - // generations make forward progress observable through actual GC. + // A stays busy across more than two complete retry ticks. New work + // in B and the profile must still finish while A's gate remains held. let (writer_started, writer_entered) = tokio::sync::oneshot::channel(); let (release_writer, writer_released) = tokio::sync::oneshot::channel(); let holding_admin = administration.clone(); @@ -2189,11 +2364,12 @@ mod tests { &profile_root, &project.project_id, ); + let writer_data_root = retained_data_root.clone(); let writer = NativeTask::spawn(async move { holding_admin .with_writer_in( crate::daemon::branch_admin::store_writer_scope( - &retained_data_root, + &writer_data_root, StoreWriterClass::Content, ), move || async move { @@ -2207,9 +2383,10 @@ mod tests { writer_entered .await .expect("project content writer owns shared store gate"); - for _ in 0..super::MAINTENANCE_WRITER_FAST_DEFERRALS { - let deferred = coordinator - .run_tick( + for _ in 0..2 { + let deferred = native_runtime::timeout( + Duration::from_secs(10), + coordinator.run_tick( &profile_root, &profile_database, &administration, @@ -2217,20 +2394,91 @@ mod tests { &retention, branch_gc, None, - ) - .await; + ), + ) + .await + .expect("busy A cannot park a complete tick"); + assert_eq!(deferred, MaintenanceTickOutcome::Retry); + assert_eq!(generation_files(), 6, "A cannot collect under its writer"); + } + let other_checkout = temp.path().join("other-checkout"); + std::fs::create_dir_all(&other_checkout).expect("other checkout"); + let other = profile_database + .upsert_code_project("proj_z_independent", &other_checkout, None, None, None) + .await + .expect("register independent project B"); + assert!( + project.project_id < other.project_id, + "A precedes B in the work window" + ); + let other_store = + tracedecay_code_index_retention::code_index_generations::code_index_store_root( + &tracedecay_runtime_core::storage::profile_sharded_data_root( + &profile_root, + &other.project_id, + ), + Path::new(&other.canonical_root), + ); + tracedecay_code_index_retention::code_index_generations::fixture::write_generation_store_fixture( + &other_store, 6, + ); + let handles = profile_root.join(super::RESPONSE_HANDLES_DIRECTORY); + tracedecay_session_memory::response_handles::store_response_handle(&handles, "expired", 1) + .expect("expired profile handle"); + tracedecay_session_memory::response_handles::store_response_handle( + &handles, + "fresh", + tracedecay_runtime_core::tracedecay::current_timestamp(), + ) + .expect("fresh profile handle"); + for _ in 0..2 { + let deferred = native_runtime::timeout( + Duration::from_secs(10), + coordinator.run_tick( + &profile_root, + &profile_database, + &administration, + &schedulers, + &retention, + branch_gc, + None, + ), + ) + .await + .expect("B and profile maintenance finish while A stays busy beyond two ticks"); assert_eq!(deferred, MaintenanceTickOutcome::Retry); + assert_eq!(generation_files(), 6, "A's writer is still held"); + assert_eq!( + std::fs::read_dir(other_store.join("code-generations-v1")) + .expect("B generations") + .count(), + 1, + "B reclaims its five superseded generations before A releases", + ); assert_eq!( - generation_files(), - 6, - "fast probe must not collect under a writer" + tracedecay_session_memory::response_handles::inventory_response_handles(&handles) + .expect("profile handle inventory") + .file_count, + 1, + "the profile sweep removes the expired handle before A releases", ); + let pending = coordinator.writer_reservations.pending.lock().unwrap(); + assert_eq!(pending.len(), 1, "only one acquisition, for busy A"); + let WriterScope::StoreMaintenance { data_root } = + super::store_maintenance_writer_scope(&retained_data_root) + else { + panic!("fixture A has canonical store identity"); + }; + assert!(pending.contains_key(&(format!("r:{}", project.project_id), data_root))); } - - let release_after_queue = std::thread::spawn(move || { - std::thread::sleep(Duration::from_millis(100)); - release_writer.send(()).expect("release competing writer"); - }); + release_writer.send(()).expect("release competing writer"); + assert!( + matches!( + writer.wait().await, + native_runtime::NativeTaskOutcome::Completed(()) + ), + "competing writer must complete normally" + ); coordinator .run_tick( &profile_root, @@ -2242,13 +2490,13 @@ mod tests { None, ) .await; - release_after_queue.join().expect("writer release thread"); assert!( - matches!( - writer.wait().await, - native_runtime::NativeTaskOutcome::Completed(()) - ), - "competing writer must complete normally" + coordinator + .writer_reservations + .pending + .lock() + .unwrap() + .is_empty() ); assert_eq!( generation_files(), From 5618857159c87674533f8d11b407741d70de4763 Mon Sep 17 00:00:00 2001 From: Zack Jackson <25274700+ScriptedAlchemy@users.noreply.github.com> Date: Mon, 9 Nov 2026 23:00:03 -0800 Subject: [PATCH 23/23] fix(retention): actively own queued maintenance work Transfer contended fair acquisitions and their work to supervised native jobs so store permits are consumed and released between ticks. Keep CPU admission with running work, retain exact work identities, cancel queued acquisitions and join admitted work during shutdown. Reproduce the ordinary-writer stall on the prior implementation, then verify native between-tick completion, cross-store generation and profile progress, cancellation, replacement gates and cooperative shutdown. Focused maintenance and writer-gate tests, the daemon capacity-retirement journey, Clippy, formatting and generated BUILD checks pass. --- crates/tracedecay/src/daemon/branch_admin.rs | 5 +- crates/tracedecay/src/daemon/maintenance.rs | 626 +++++++++++++++---- 2 files changed, 492 insertions(+), 139 deletions(-) diff --git a/crates/tracedecay/src/daemon/branch_admin.rs b/crates/tracedecay/src/daemon/branch_admin.rs index 83a41efa7f4..497ff5db3e3 100644 --- a/crates/tracedecay/src/daemon/branch_admin.rs +++ b/crates/tracedecay/src/daemon/branch_admin.rs @@ -1933,9 +1933,8 @@ impl StoreAdministration { operation().await } - /// Owns only the canonical gate while retention waits across ticks. The - /// caller polls this acquisition without waiting in the serial tick and - /// drops it on cancellation or when the exact store leaves the work list. + /// Transfers only the canonical gate into the supervised retention job. + /// The acquisition retains no mounted graph or project database client. pub(super) fn retention_writer_acquisition( &self, data_root: PathBuf, diff --git a/crates/tracedecay/src/daemon/maintenance.rs b/crates/tracedecay/src/daemon/maintenance.rs index 661e28983e9..9cc365460f0 100644 --- a/crates/tracedecay/src/daemon/maintenance.rs +++ b/crates/tracedecay/src/daemon/maintenance.rs @@ -4,7 +4,7 @@ use std::path::{Path, PathBuf}; use std::sync::Arc; use std::time::{Duration, Instant}; -use futures_util::{FutureExt, future::BoxFuture}; +use futures_util::FutureExt; use tokio::sync::Mutex; use tracedecay_code_index::parallelism::{ collect_idle_installed_worker_heaps, collect_installed_worker_heaps_when_idle, @@ -32,33 +32,116 @@ use tracedecay_daemon_service::shutdown::DAEMON_TASK_ABORT_DEADLINE; use tracedecay_domain::process_heap::request_idle_thread_collection_v1; use tracedecay_runtime_core::logging::log_daemon_event; use tracedecay_runtime_core::native_runtime::{self, NativeTask, TimeoutError}; -use tracedecay_store_runtime::{WriterAdmissionGuard, WriterScope}; +use tracedecay_store_runtime::WriterScope; const MAINTENANCE_STORE_PAGE_LIMIT: usize = 8; const REGISTERED_PROJECT_PAGE_LIMIT: usize = 64; -/// Fair store-local acquisitions survive retry ticks without parking the tick -/// or retaining a database client. Only the current tick may consume admission -/// and run work; cancellation and removal drop the pending queue positions. -/// Work keys distinguish mounted branches sharing a store; pairing the key -/// with its canonical root prevents a same-key replacement reusing admission. -#[derive(Default)] -struct RetentionWriterReservations { - pending: - std::sync::Mutex>>, +/// One owned fair acquisition and its work. Cancellation only governs the +/// acquisition; admitted journaled work finishes cooperatively. +struct RetentionWriterJob { + cancellation: tracedecay_runtime_core::cancellation::CancellationToken, + task: NativeTask>, } -impl RetentionWriterReservations { - fn clear(&self) { - self.pending +/// The coordinator supervises queued store units independently of its cadence. +/// A job consumes its fair turn and runs/releases admission even when no tick +/// can run. Completed outcomes stay here until the next census observes them, +/// preventing duplicate jobs for the same canonical store and work key. +struct RetentionWriterJobs { + pending: std::sync::Mutex>>, +} + +impl Default for RetentionWriterJobs { + fn default() -> Self { + Self { + pending: std::sync::Mutex::new(BTreeMap::new()), + } + } +} + +impl RetentionWriterJobs { + fn cancel_all(&self) { + for job in self + .pending .lock() .unwrap_or_else(std::sync::PoisonError::into_inner) - .clear(); + .values() + { + job.cancellation.cancel(); + } } fn retain(&self, active: &BTreeSet<(String, PathBuf)>) { self.pending .lock() .unwrap_or_else(std::sync::PoisonError::into_inner) - .retain(|root, _| active.contains(root)); + .retain(|key, job| { + if active.contains(key) { + return true; + } + job.cancellation.cancel(); + // Keep admitted work supervised until it settles. Cancelling + // an acquisition releases its queued permit on the next poll. + job.task.try_finished().is_none_or(|outcome| { + record_retention_job_outcome(outcome); + false + }) + }); + } + + async fn shutdown(&self) { + let jobs = std::mem::take( + &mut *self + .pending + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner), + ); + for job in jobs.values() { + job.cancellation.cancel(); + } + for (_, job) in jobs { + join_cancelled_maintenance_task( + Some(job.task), + "retention_store", + DAEMON_TASK_ABORT_DEADLINE, + ) + .await; + } + } +} + +impl Drop for RetentionWriterJobs { + fn drop(&mut self) { + for job in self + .pending + .get_mut() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .values() + { + job.cancellation.cancel(); + } + } +} + +fn record_retention_job_outcome( + outcome: native_runtime::NativeTaskOutcome>, +) -> Option { + match outcome { + native_runtime::NativeTaskOutcome::Completed(output) => output, + native_runtime::NativeTaskOutcome::Cancelled => { + tracing::error!( + event = "retention_store_task_cancelled", + "retention store owner did not settle normally" + ); + None + } + native_runtime::NativeTaskOutcome::Panicked(error) => { + tracing::error!( + event = "retention_store_task_panicked", + ?error, + "retention store owner failed" + ); + None + } } } @@ -66,16 +149,17 @@ async fn admit_retention_writer( administration: &StoreAdministration, cancellation: &tracedecay_runtime_core::cancellation::CancellationToken, scope: WriterScope, - reservations: &RetentionWriterReservations, + jobs: &RetentionWriterJobs, work_key: &str, operation: Operation, ) -> Option where - Operation: FnOnce() -> OperationFuture, - OperationFuture: Future, + Operation: FnOnce() -> OperationFuture + Send + 'static, + OperationFuture: Future + Send, + Output: Clone + Send + Sync + 'static, { if cancellation.is_cancelled() { - reservations.clear(); + jobs.cancel_all(); return None; } let WriterScope::StoreMaintenance { data_root } = scope else { @@ -83,31 +167,61 @@ where // unrelated store writers behind an unbounded daemon-wide wait. return administration.try_with_writer_in(scope, operation).await; }; - let _writer = { - let mut pending = reservations + let writer = { + let mut pending = jobs .pending .lock() .unwrap_or_else(std::sync::PoisonError::into_inner); - // Cancellation can race the lock. Do not recreate a reservation after - // shutdown prepare has synchronously cleared this table. if cancellation.is_cancelled() { - pending.clear(); return None; } let key = (work_key.to_owned(), data_root.clone()); - let acquisition = pending.entry(key.clone()).or_insert_with(|| { - administration - .retention_writer_acquisition(data_root.clone()) - .boxed() - }); - // Poll once, retaining the exact same fair queue position on refusal. - // Retry cadence owns the next poll; there is no independent job owner. - let admitted = acquisition.as_mut().now_or_never()?; - pending.remove(&key); - admitted + if let Some(job) = pending.get(&key) { + let outcome = job.task.try_finished()?; + pending.remove(&key); + return record_retention_job_outcome(outcome); + } + let mut acquisition = administration + .retention_writer_acquisition(data_root) + .boxed(); + if let Some(writer) = acquisition.as_mut().now_or_never() { + writer + } else { + let job_cancellation = tracedecay_runtime_core::cancellation::CancellationToken::new(); + let stopped = job_cancellation.clone(); + let stopping = cancellation.clone(); + // Transfer the exact queued future, together with the operation, + // to an actively polled owner. Never retain an idle lock future. + let task = match NativeTask::spawn(async move { + let writer = tokio::select! { + biased; + () = stopping.cancelled() => return None, + () = stopped.cancelled() => return None, + writer = acquisition => writer, + }; + if stopping.is_cancelled() || stopped.is_cancelled() { + return None; + } + let _writer = writer; + Some(operation().await) + }) { + Ok(task) => task, + Err(error) => { + tracing::error!(event = "retention_store_task_unavailable", %error, "could not start retention store owner"); + return None; + } + }; + pending.insert( + key, + RetentionWriterJob { + cancellation: job_cancellation, + task, + }, + ); + return None; + } }; - // Once admitted, journal/retention work must finish cooperatively rather - // than be dropped at the writer boundary. + let _writer = writer; Some(operation().await) } @@ -240,8 +354,8 @@ fn spawn_maintenance_task( /// the blocking closure keeps that client past the terminal store close, which /// then leaves the store's WAL untruncated. The task therefore gets the abort /// deadline to finish cooperatively and is aborted only past it. -async fn join_cancelled_maintenance_task( - task: Option>, +async fn join_cancelled_maintenance_task( + task: Option>, owner: &'static str, abort_deadline: Duration, ) { @@ -249,7 +363,27 @@ async fn join_cancelled_maintenance_task( return; }; let reason = match native_runtime::timeout(abort_deadline, task.wait()).await { - Ok(_) => return, + Ok(native_runtime::NativeTaskOutcome::Completed(_)) => return, + Ok(native_runtime::NativeTaskOutcome::Cancelled) => { + log_daemon_event( + "maintenance_task_failed", + &[ + ("owner", owner.to_owned()), + ("reason", "task_cancelled".to_owned()), + ], + ); + return; + } + Ok(native_runtime::NativeTaskOutcome::Panicked(error)) => { + log_daemon_event( + "maintenance_task_failed", + &[ + ("owner", owner.to_owned()), + ("reason", format!("{error:?}")), + ], + ); + return; + } Err(TimeoutError::Elapsed) => "cooperative_join_exceeded_abort_deadline".to_string(), Err(error @ TimeoutError::TimerUnavailable(_)) => error.to_string(), }; @@ -696,8 +830,8 @@ pub(super) struct MaintenanceCoordinator { /// store is starved when the mounted set exceeds `MAINTENANCE_STORE_PAGE_LIMIT`. store_cursor: Arc>>, store_continuations: Arc>, - /// Pending canonical store acquisitions; no admitted work lives here. - writer_reservations: Arc, + /// Supervised canonical store jobs, including admitted work and its outcome. + writer_jobs: Arc, /// Instant of the last branch-store GC pass that succeeded for every /// mounted project. `None` keeps the daily cadence retry-eligible. last_branch_gc: Arc>>, @@ -719,7 +853,7 @@ impl Default for MaintenanceCoordinator { metrics: Arc::new(Mutex::new(MaintenanceMetricsV1::default())), store_cursor: Arc::new(Mutex::new(None)), store_continuations: Arc::new(Mutex::new(StoreMaintenanceContinuations::default())), - writer_reservations: Arc::new(RetentionWriterReservations::default()), + writer_jobs: Arc::new(RetentionWriterJobs::default()), last_branch_gc: Arc::new(Mutex::new(None)), resident_memory_sampler: Arc::new(Mutex::new(None)), resident_memory_log: Arc::new(std::sync::Mutex::new( @@ -736,6 +870,7 @@ impl Default for MaintenanceCoordinator { /// retirement of an idle project refuses while anything leases its stores, so /// a tick that kept every listed store for its whole run refused those /// retirements. A store unmounted since the listing is skipped. +#[derive(Clone)] enum MaintenanceStoreWork { Session(PathBuf), Graph { @@ -743,15 +878,19 @@ enum MaintenanceStoreWork { data_root: PathBuf, }, /// The code-index scopes of a registered project that no mounted graph - /// owns; its database is never opened by this unit. - Registered(RegisteredProjectStoreV1), + /// owns. The job retains profile authority, never a project-store client. + Registered { + project_id: String, + store: RegisteredProjectStoreV1, + profile_database: tracedecay_global_db::RegisteredGlobalDbLeaseV1, + }, } impl MaintenanceStoreWork { fn database_path(&self) -> Option<&Path> { match self { Self::Session(database_path) | Self::Graph { database_path, .. } => Some(database_path), - Self::Registered(_) => None, + Self::Registered { .. } => None, } } @@ -762,7 +901,7 @@ impl MaintenanceStoreWork { // unrelated project opens behind a potentially long session pass. Self::Session(_) => WriterScope::Daemon, Self::Graph { data_root, .. } => store_maintenance_writer_scope(data_root), - Self::Registered(store) => store_maintenance_writer_scope(&store.data_root), + Self::Registered { store, .. } => store_maintenance_writer_scope(&store.data_root), } } } @@ -773,7 +912,6 @@ struct MountedGraphListing { database_path: PathBuf, project_root: PathBuf, data_root: PathBuf, - code_index_store_root: PathBuf, } /// The work-list key of a mounted project graph: its root and serving branch. @@ -919,7 +1057,7 @@ impl MaintenanceCoordinator { /// join below stays correct whether or not it already ran. pub(super) fn cancel(&self) { self.cancellation.cancel(); - self.writer_reservations.clear(); + self.writer_jobs.cancel_all(); } pub(super) async fn shutdown(&self) { @@ -933,6 +1071,7 @@ impl MaintenanceCoordinator { DAEMON_TASK_ABORT_DEADLINE, ) .await; + self.writer_jobs.shutdown().await; join_cancelled_maintenance_task( self.resident_memory_sampler.lock().await.take(), "resident_memory_sampler", @@ -1004,7 +1143,7 @@ impl MaintenanceCoordinator { Err(error) => { // Without profile authority there is no current work // census to justify retaining store queue positions. - self.writer_reservations.clear(); + self.writer_jobs.cancel_all(); log_daemon_event( "retention_degraded", &[ @@ -1018,7 +1157,7 @@ impl MaintenanceCoordinator { }; self.run_tick( profile_root, - profile_database.as_ref(), + &profile_database, administration, code_index_schedulers, retention, @@ -1043,7 +1182,7 @@ impl MaintenanceCoordinator { async fn run_tick( &self, profile_root: &Path, - profile_database: &tracedecay_global_db::RegisteredGlobalDb, + profile_database: &tracedecay_global_db::RegisteredGlobalDbLeaseV1, administration: &StoreAdministration, code_index_schedulers: &tracedecay_code_index_runtime::code_index_scheduler::CodeIndexSchedulerRegistryV1, retention: &tracedecay_configuration::RetentionConfig, @@ -1078,10 +1217,6 @@ impl MaintenanceCoordinator { database_path: graph.db().database_path().to_path_buf(), project_root: graph.project_root().to_path_buf(), data_root: graph.store_layout().data_root.clone(), - code_index_store_root: code_index_store_root( - &graph.store_layout().data_root, - &graph.store_layout().project_root, - ), }) .collect::>(); #[cfg(any(test, feature = "test-helpers"))] @@ -1090,7 +1225,7 @@ impl MaintenanceCoordinator { } // The listing copies in-memory mounts and is not CPU work, so a tick // parked at the listing holds no background CPU unit. - let Some(_background_cpu) = background_cpu.try_acquire() else { + let Some(tick_cpu) = background_cpu.try_acquire() else { return MaintenanceTickOutcome::Retry; }; let (registered_projects, registry_listed) = @@ -1119,10 +1254,6 @@ impl MaintenanceCoordinator { .collect(); sweep_response_handles(handle_roots, self.cancellation.clone()).await; } - let mounted_store_roots = project_graphs - .iter() - .map(|graph| graph.code_index_store_root.clone()) - .collect::>(); let mut active_telemetry_paths = BTreeSet::from([profile_database.db_path().to_path_buf()]); active_telemetry_paths.extend(session_databases.iter().cloned()); active_telemetry_paths.extend( @@ -1156,13 +1287,17 @@ impl MaintenanceCoordinator { for (project_id, store) in registered_projects { work.push(( format!("r:{project_id}"), - MaintenanceStoreWork::Registered(store), + MaintenanceStoreWork::Registered { + project_id, + store, + profile_database: profile_database.clone(), + }, )); } work.sort_by(|left, right| left.0.cmp(&right.0)); let keys = work.iter().map(|(key, _)| key.clone()).collect::>(); self.store_continuations.lock().await.retain_stores(&keys); - self.writer_reservations.retain( + self.writer_jobs.retain( &work .iter() .filter_map(|(key, unit)| match unit.writer_scope() { @@ -1195,7 +1330,7 @@ impl MaintenanceCoordinator { .map(|graph| graph.data_root.clone()) .collect::>(); scan_data_roots.extend(work.iter().filter_map(|(_, unit)| match unit { - MaintenanceStoreWork::Registered(store) => Some(store.data_root.clone()), + MaintenanceStoreWork::Registered { store, .. } => Some(store.data_root.clone()), MaintenanceStoreWork::Session(_) | MaintenanceStoreWork::Graph { .. } => None, })); maintenance_observations.retain_text_artifact_scan_projects(&scan_data_roots); @@ -1227,6 +1362,8 @@ impl MaintenanceCoordinator { // advances past attempted units even on cancellation. A continuation // advances bounded lossless storage work and code-generation retention // over the same bounded store window rather than pinning one project. + // CPU capacity belongs to actual work, never a queued acquisition. + drop(tick_cpu); let mut attempted = 0usize; let mut deferred = 0u64; let mut outcome = MaintenanceTickOutcome::Complete; @@ -1236,17 +1373,33 @@ impl MaintenanceCoordinator { break; } let scope = work[index].1.writer_scope(); + let job_admin = administration.clone(); + let job_cancellation = self.cancellation.clone(); + let job_cpu = Arc::clone(background_cpu); + let job_schedulers = code_index_schedulers.clone(); + let job_retention = retention.clone(); + let job_observations = maintenance_observations.clone(); + let job_key = keys[index].clone(); + let job_unit = work[index].1.clone(); let admitted = admit_retention_writer( administration, &self.cancellation, scope, - &self.writer_reservations, + &self.writer_jobs, &keys[index], - || async { + move || async move { + let administration = &job_admin; + let cancellation = &job_cancellation; + let code_index_schedulers = &job_schedulers; + let retention = &job_retention; + let maintenance_observations = job_observations; + let Some(_background_cpu) = job_cpu.try_acquire() else { + return MaintenanceTickOutcome::Retry; + }; let Ok(_clients) = administration.maintenance_store_clients().try_lock() else { return MaintenanceTickOutcome::Retry; }; - let (key, unit) = &work[index]; + let (key, unit) = (&job_key, &job_unit); match unit { MaintenanceStoreWork::Session(database_path) => { let Some(database) = administration @@ -1287,20 +1440,31 @@ impl MaintenanceCoordinator { &project_store_maintenance_lease(&graph), code_index_schedulers, &maintenance_observations, - &self.cancellation, + cancellation, retention.compaction.as_ref(), continuation, ) .await } - MaintenanceStoreWork::Registered(store) => { + MaintenanceStoreWork::Registered { project_id, store, profile_database } => { + match profile_database.code_project_exists(project_id).await { + Ok(true) => {} + Ok(false) => return MaintenanceTickOutcome::Complete, + Err(error) => { + tracing::error!(event = "retention_registry_unavailable", %error); + return MaintenanceTickOutcome::Retry; + } + } + let mounted_store_roots = administration.mounted_project_graphs().await + .iter().map(|graph| code_index_store_root(&graph.store_layout().data_root, graph.project_root())) + .collect(); run_registered_project_generation_maintenance( store, &mounted_store_roots, code_index_schedulers, profile_database, &maintenance_observations, - &self.cancellation, + cancellation, ) .await } @@ -1334,6 +1498,10 @@ impl MaintenanceCoordinator { outcome = MaintenanceTickOutcome::Retry; } + let Some(_background_cpu) = background_cpu.try_acquire() else { + return outcome.combine(MaintenanceTickOutcome::Retry); + }; + // Profile-wide maintenance is intentionally excluded from a bounded // continuation: only the owning phase is eligible for the short // cadence. @@ -1675,10 +1843,157 @@ mod tests { use tracedecay_runtime_core::cancellation::CancellationToken; use tracedecay_runtime_core::native_runtime; - /// Retry ticks retain one fair queue position, with no maintenance task - /// owning a daemon read guard while the target store is busy. + #[test] + fn released_store_admits_ordinary_writer_without_another_retention_tick() { + native_runtime::block_on(async { + let administration = StoreAdministration::default(); + let (entered_tx, entered_rx) = tokio::sync::oneshot::channel(); + let (release_tx, release_rx) = tokio::sync::oneshot::channel(); + let holding = administration.clone(); + let writer = NativeTask::spawn(async move { + holding + .with_writer_in( + WriterScope::store("/between-ticks", StoreWriterClass::Content), + || async { + entered_tx.send(()).expect("writer entered"); + release_rx.await.expect("writer released"); + }, + ) + .await; + }) + .expect("native writer"); + entered_rx.await.expect("store held"); + let cancellation = CancellationToken::new(); + let reservations = super::RetentionWriterJobs::default(); + let retained = Arc::new(AtomicUsize::new(0)); + let completed = Arc::clone(&retained); + assert_eq!( + super::admit_retention_writer( + &administration, + &cancellation, + WriterScope::maintenance("/between-ticks"), + &reservations, + "retention", + move || async move { + completed.fetch_add(1, Ordering::SeqCst); + }, + ) + .await, + None + ); + // Queue a real later writer before releasing the current writer. + let following = administration.with_writer_in( + WriterScope::store("/between-ticks", StoreWriterClass::Owner), + || async { 43_u8 }, + ); + tokio::pin!(following); + poll_fn(|cx| { + assert!(following.as_mut().poll(cx).is_pending()); + Poll::Ready(()) + }) + .await; + release_tx.send(()).expect("release between ticks"); + assert!(matches!( + writer.wait().await, + native_runtime::NativeTaskOutcome::Completed(()) + )); + // No retention helper or tick is polled after release, including + // when the next tick cannot obtain background CPU admission. + assert_eq!( + native_runtime::timeout(Duration::from_secs(2), &mut following) + .await + .expect("ordinary writer must resume without a retention tick"), + 43 + ); + assert_eq!( + retained.load(Ordering::SeqCst), + 1, + "the earlier fair retention turn ran exactly once" + ); + reservations.shutdown().await; + }) + .expect("native runtime"); + } + + #[test] + fn shutdown_joins_queued_retention_that_has_started_between_ticks() { + native_runtime::block_on(async { + let administration = StoreAdministration::default(); + let coordinator = MaintenanceCoordinator::default(); + let (holding_tx, holding_rx) = tokio::sync::oneshot::channel(); + let (release_writer, released_writer) = tokio::sync::oneshot::channel(); + let holding = administration.clone(); + let writer = NativeTask::spawn(async move { + holding + .with_writer_in( + WriterScope::store("/shutdown-between-ticks", StoreWriterClass::Content), + || async { + holding_tx.send(()).expect("writer entered"); + released_writer.await.expect("writer released"); + }, + ) + .await; + }) + .expect("native writer"); + holding_rx.await.expect("store held"); + let (entered_tx, entered_rx) = tokio::sync::oneshot::channel(); + let (release_work, released_work) = tokio::sync::oneshot::channel(); + let finished = Arc::new(AtomicUsize::new(0)); + let completed = Arc::clone(&finished); + assert_eq!( + super::admit_retention_writer( + &administration, + &coordinator.cancellation, + WriterScope::maintenance("/shutdown-between-ticks"), + &coordinator.writer_jobs, + "retention", + move || async move { + entered_tx.send(()).expect("retention entered"); + released_work.await.expect("retention released"); + completed.fetch_add(1, Ordering::SeqCst); + MaintenanceTickOutcome::Complete + }, + ) + .await, + None + ); + release_writer.send(()).expect("release store writer"); + assert!(matches!( + writer.wait().await, + native_runtime::NativeTaskOutcome::Completed(()) + )); + entered_rx.await.expect("queued work starts without a tick"); + let shutdown = coordinator.shutdown(); + tokio::pin!(shutdown); + poll_fn(|cx| { + assert!( + shutdown.as_mut().poll(cx).is_pending(), + "shutdown joins admitted work" + ); + Poll::Ready(()) + }) + .await; + assert!(coordinator.cancellation.is_cancelled()); + assert_eq!(finished.load(Ordering::SeqCst), 0); + assert_eq!( + administration.try_with_writer(|| async {}).await, + None, + "admitted work retains writer admission through shutdown" + ); + release_work.send(()).expect("finish admitted work"); + native_runtime::timeout(Duration::from_secs(2), &mut shutdown) + .await + .expect("cooperative shutdown"); + assert_eq!(finished.load(Ordering::SeqCst), 1); + assert_eq!(administration.try_with_writer(|| async {}).await, Some(())); + }) + .expect("native runtime"); + } + + /// Retry ticks submit only one fair job, which owns no daemon read guard + /// while the target store is busy. #[tokio::test] - async fn retention_retains_one_fair_acquisition_across_retry_ticks() { + async fn retention_runs_one_fair_job_across_retry_ticks() { let administration = StoreAdministration::default(); let (entered_tx, entered_rx) = tokio::sync::oneshot::channel(); let (release_tx, release_rx) = tokio::sync::oneshot::channel(); @@ -1696,7 +2011,7 @@ mod tests { }); entered_rx.await.expect("writer acquired its store gate"); let cancellation = CancellationToken::new(); - let reservations = super::RetentionWriterReservations::default(); + let reservations = super::RetentionWriterJobs::default(); for _ in 0..4 { assert_eq!( super::admit_retention_writer( @@ -1705,7 +2020,7 @@ mod tests { WriterScope::maintenance("/retained-store"), &reservations, "retention", - || async { 41_u8 }, + || async { 42_u8 }, ) .await, None, @@ -1788,6 +2103,26 @@ mod tests { let before_release = writer_entries.load(Ordering::SeqCst); release_tx.send(()).expect("release the original writer"); holder.await.expect("first writer finishes"); + assert_eq!( + tokio::time::timeout(Duration::from_secs(5), &mut later_writer) + .await + .expect("later writer resumes after retention"), + 43 + ); + + tokio::time::timeout(Duration::from_secs(5), async { + while reservations + .pending + .lock() + .unwrap() + .values() + .any(|job| !job.task.is_finished()) + { + tokio::task::yield_now().await; + } + }) + .await + .expect("both fair jobs settle before their outcomes are observed"); assert_eq!( super::admit_retention_writer( &administration, @@ -1813,12 +2148,6 @@ mod tests { Some(45), "the first work unit cannot consume another branch's queued acquisition", ); - assert_eq!( - tokio::time::timeout(Duration::from_secs(5), &mut later_writer) - .await - .expect("later writer resumes after retention"), - 43 - ); // Keep the churn alive through retention and observe a subsequent // acquisition before stopping it: a finite burst is not this proof. tokio::time::timeout(Duration::from_secs(5), async { @@ -1839,8 +2168,8 @@ mod tests { let administration = StoreAdministration::default(); let cancellation = CancellationToken::new(); cancellation.cancel(); - let constructed = AtomicUsize::new(0); - let reservations = super::RetentionWriterReservations::default(); + let constructed = Arc::new(AtomicUsize::new(0)); + let reservations = super::RetentionWriterJobs::default(); for scope in [WriterScope::maintenance("/held-store"), WriterScope::Daemon] { assert_eq!( super::admit_retention_writer( @@ -1849,9 +2178,12 @@ mod tests { scope, &reservations, "retention", - || { - constructed.fetch_add(1, Ordering::SeqCst); - std::future::ready(7_u8) + { + let constructed = Arc::clone(&constructed); + move || { + constructed.fetch_add(1, Ordering::SeqCst); + std::future::ready(7_u8) + } }, ) .await, @@ -1888,17 +2220,20 @@ mod tests { .await .expect("writer holds shared daemon admission"); let coordinator = MaintenanceCoordinator::default(); - let constructed = AtomicUsize::new(0); + let constructed = Arc::new(AtomicUsize::new(0)); assert_eq!( super::admit_retention_writer( &administration, &coordinator.cancellation, WriterScope::maintenance("/held-store"), - &coordinator.writer_reservations, + &coordinator.writer_jobs, "retention", - || { - constructed.fetch_add(1, Ordering::SeqCst); - std::future::ready(7_u8) + { + let constructed = Arc::clone(&constructed); + move || { + constructed.fetch_add(1, Ordering::SeqCst); + std::future::ready(MaintenanceTickOutcome::Complete) + } }, ) .await, @@ -1915,14 +2250,7 @@ mod tests { }) .await; coordinator.shutdown().await; - assert!( - coordinator - .writer_reservations - .pending - .lock() - .unwrap() - .is_empty() - ); + assert!(coordinator.writer_jobs.pending.lock().unwrap().is_empty()); assert_eq!(constructed.load(Ordering::SeqCst), 0); // Owner and content otherwise run concurrently. Only queued exclusive // maintenance blocked this content writer on the same store. @@ -1941,7 +2269,7 @@ mod tests { } #[tokio::test] - async fn retired_store_reservations_release_followers_and_replacement_uses_its_own_gate() { + async fn retired_store_jobs_release_followers_and_replacement_uses_its_own_gate() { let temp = tempfile::tempdir().expect("store roots"); let retired = temp.path().join("retired"); let replacement = temp.path().join("replacement"); @@ -1963,7 +2291,7 @@ mod tests { .await; }); entered_rx.await.expect("held retired store"); - let work = AtomicUsize::new(0); + let work = Arc::new(AtomicUsize::new(0)); // Different spellings of the same canonical store share one pending // acquisition, rather than competing maintenance jobs. for root in [&retired, &retired.join(".")] { @@ -1972,23 +2300,21 @@ mod tests { &administration, &coordinator.cancellation, super::store_maintenance_writer_scope(root), - &coordinator.writer_reservations, + &coordinator.writer_jobs, "retention", - || async { work.fetch_add(1, Ordering::SeqCst) }, + { + let work = Arc::clone(&work); + move || async move { + work.fetch_add(1, Ordering::SeqCst); + MaintenanceTickOutcome::Complete + } + }, ) .await, None ); } - assert_eq!( - coordinator - .writer_reservations - .pending - .lock() - .unwrap() - .len(), - 1 - ); + assert_eq!(coordinator.writer_jobs.pending.lock().unwrap().len(), 1); let following = administration.with_writer_in( crate::daemon::branch_admin::store_writer_scope(&retired, StoreWriterClass::Content), || async { 8_u8 }, @@ -2005,7 +2331,7 @@ mod tests { }; // The current mounted-work census keeps only the replacement identity. coordinator - .writer_reservations + .writer_jobs .retain(&std::collections::BTreeSet::from([( "retention".to_owned(), data_root.clone(), @@ -2021,26 +2347,26 @@ mod tests { &administration, &coordinator.cancellation, replacement_scope, - &coordinator.writer_reservations, + &coordinator.writer_jobs, "retention", - || async { work.fetch_add(1, Ordering::SeqCst) }, + { + let work = Arc::clone(&work); + move || async move { + work.fetch_add(1, Ordering::SeqCst); + MaintenanceTickOutcome::Complete + } + }, ) .await, - Some(0) + Some(MaintenanceTickOutcome::Complete) ); assert_eq!( work.load(Ordering::SeqCst), 1, "only the replacement executes maintenance" ); - assert!( - coordinator - .writer_reservations - .pending - .lock() - .unwrap() - .is_empty() - ); + coordinator.shutdown().await; + assert!(coordinator.writer_jobs.pending.lock().unwrap().is_empty()); release_tx.send(()).expect("release retired owner"); owner.await.expect("retired owner settles"); } @@ -2060,7 +2386,7 @@ mod tests { &working, &stopping, WriterScope::maintenance("/retention-store"), - &super::RetentionWriterReservations::default(), + &super::RetentionWriterJobs::default(), "retention", move || async move { entered_tx @@ -2268,8 +2594,8 @@ mod tests { } /// Busy registered store A cannot park free store B or the profile sweep. - /// After release, the same acquisition lets A converge without a separate - /// maintenance job. Capacity release still excludes store clients. + /// After release, the supervised fair job lets A converge between ticks. + /// Capacity release still excludes store clients. #[test] fn full_ticks_preserve_independent_store_progress_without_a_tokio_runtime() { let hazards = crate::test_support::native_runtime_hazards(); @@ -2462,7 +2788,7 @@ mod tests { 1, "the profile sweep removes the expired handle before A releases", ); - let pending = coordinator.writer_reservations.pending.lock().unwrap(); + let pending = coordinator.writer_jobs.pending.lock().unwrap(); assert_eq!(pending.len(), 1, "only one acquisition, for busy A"); let WriterScope::StoreMaintenance { data_root } = super::store_maintenance_writer_scope(&retained_data_root) @@ -2471,6 +2797,19 @@ mod tests { }; assert!(pending.contains_key(&(format!("r:{}", project.project_id), data_root))); } + let following = administration.with_writer_in( + super::super::branch_admin::store_writer_scope( + &retained_data_root, + StoreWriterClass::Owner, + ), + || async {}, + ); + tokio::pin!(following); + poll_fn(|cx| { + assert!(following.as_mut().poll(cx).is_pending()); + Poll::Ready(()) + }) + .await; release_writer.send(()).expect("release competing writer"); assert!( matches!( @@ -2479,6 +2818,28 @@ mod tests { ), "competing writer must complete normally" ); + native_runtime::timeout(Duration::from_secs(10), &mut following) + .await + .expect("ordinary writer resumes after retention without another tick"); + assert_eq!( + generation_files(), + 1, + "A collects before any further tick is polled" + ); + native_runtime::timeout(Duration::from_secs(10), async { + while coordinator + .writer_jobs + .pending + .lock() + .unwrap() + .values() + .any(|job| !job.task.is_finished()) + { + tokio::task::yield_now().await; + } + }) + .await + .expect("A's supervised owner settles"); coordinator .run_tick( &profile_root, @@ -2490,14 +2851,7 @@ mod tests { None, ) .await; - assert!( - coordinator - .writer_reservations - .pending - .lock() - .unwrap() - .is_empty() - ); + assert!(coordinator.writer_jobs.pending.lock().unwrap().is_empty()); assert_eq!( generation_files(), 1,