diff --git a/.gitignore b/.gitignore index fb0d9ef..b01e019 100644 --- a/.gitignore +++ b/.gitignore @@ -103,3 +103,8 @@ $RECYCLE.BIN/ *.iml .fleet/ .anthropic/ +/obsd\BaseMyAI +obsd\BaseMyAI\.obsidian +obsd\BaseMyAI\BaseMyAI +obsd +opencode.jsonc diff --git a/crates/basemyai-engine/Cargo.toml b/crates/basemyai-engine/Cargo.toml index 5bed9c8..5c3909d 100644 --- a/crates/basemyai-engine/Cargo.toml +++ b/crates/basemyai-engine/Cargo.toml @@ -18,6 +18,13 @@ test-util = [] # Témoin négatif D-6 d'ADR-053 : réintroduit volontairement un manifeste # d'index plat et non borné. Jamais activé par défaut ni par un build produit. unbounded-index-oracle = ["test-util"] +# Témoin négatif GOV-RSS02 (ADR-071) : réintroduit volontairement, dans le +# harnais de test `gov_rss_oracle` lui-même (jamais dans le governor ou un +# cache réel), une fuite proportionnelle au volume que la comptabilité du +# governor ne peut pas voir (une copie orpheline hors réservation, retenue à +# côté de l'éviction correctement comptée). Jamais activé par défaut ni par +# un build produit. +governor-rss-negative-control-leak = ["test-util"] [[bin]] name = "crash_writer" @@ -267,5 +274,17 @@ path = "tests/compaction/generation_gc_retry.rs" name = "generation_pointer_loss_is_rejected_and_gen1_survives" path = "tests/compaction/generation_pointer_loss_is_rejected_and_gen1_survives.rs" +[[test]] +name = "cb_t5_bounded_release_window" +path = "tests/governor/cb_t5_bounded_release_window.rs" + +[[test]] +name = "gov_rss_oracle" +path = "tests/governor/gov_rss_oracle.rs" + +[[test]] +name = "gov_rotation_rss_oracle" +path = "tests/governor/gov_rotation_rss_oracle.rs" + [lints] workspace = true diff --git a/crates/basemyai-engine/src/memory_governor/admission.rs b/crates/basemyai-engine/src/memory_governor/admission.rs index c2f2f16..6666fb0 100644 --- a/crates/basemyai-engine/src/memory_governor/admission.rs +++ b/crates/basemyai-engine/src/memory_governor/admission.rs @@ -506,12 +506,29 @@ impl MemoryGovernor { pub fn admission_waiters(&self) -> usize { self.lock().waiters.len() } + + /// `bytes_by_class[c]` (ADR-071 §"Protections borrowables") — `test-util` + /// only, same rationale as [`Self::committed_bytes`]: lets GOV-U21/GOV-U23 + /// compute the real `H_b(c)`/`D_b(c)` shortfall formula directly against + /// the governor's own bookkeeping, instead of inferring it indirectly. + #[cfg(any(test, feature = "test-util"))] + pub fn bytes_by_class(&self, class: AdmissionClass) -> usize { + self.lock().bytes_by_class[class.index()] + } + + /// `slots_by_class[c]` — `test-util` only, the slot-side twin of + /// [`Self::bytes_by_class`] used by GOV-U22's `H_s(c)`/`D_s(c)` checks. + #[cfg(any(test, feature = "test-util"))] + pub fn slots_by_class(&self, class: AdmissionClass) -> usize { + self.lock().slots_by_class[class.index()] + } } #[cfg(test)] mod tests { - use std::sync::{Arc, Barrier}; - use std::time::Duration; + use std::sync::atomic::{AtomicBool, Ordering}; + use std::sync::{Arc, Barrier, mpsc}; + use std::time::{Duration, Instant}; use super::*; use crate::memory_governor::{GovernorConfig, MemoryDomain}; @@ -1222,4 +1239,978 @@ mod tests { ); assert!(matches!(err, EngineError::AdmissionClosed)); } + + // ---- ADR-071 §"Protections borrowables": GOV-U21/GOV-U22/GOV-U23 ---- + // + // `D_b(c) = max(0, H_b(c) - free_bytes)` and `D_s(c) = max(0, H_s(c) - + // free_slots)`, `H_b(c) = max(0, reserved_bytes[c] - bytes_by_class[c])`, + // `H_s(c) = max(0, reserved_slots[c] - slots_by_class[c])`, verbatim from + // the ADR. The three tests below compute these directly off the real + // `MemoryGovernor`'s `test-util` accessors, not off an inferred proxy. + + /// `D_b(class)` computed the same way `scheduler::capacity_eligible` + /// derives it, off the governor's live bookkeeping. + fn shortfall_bytes( + governor: &MemoryGovernor, + reserved_bytes: usize, + class: AdmissionClass, + budget_bytes: usize, + ) -> usize { + let free_bytes = budget_bytes.saturating_sub(governor.committed_bytes()); + let headroom = reserved_bytes.saturating_sub(governor.bytes_by_class(class)); + headroom.saturating_sub(free_bytes) + } + + /// `D_s(class)`, the slot-side twin of [`shortfall_bytes`]. + fn shortfall_slots( + governor: &MemoryGovernor, + reserved_slots: usize, + class: AdmissionClass, + max_queue_depth: usize, + ) -> usize { + let free_slots = max_queue_depth.saturating_sub(governor.outstanding_slots()); + let headroom = reserved_slots.saturating_sub(governor.slots_by_class(class)); + headroom.saturating_sub(free_slots) + } + + fn plan(peak_bytes: usize) -> MemoryPlan { + MemoryPlan { + peak_bytes, + intent_owned_bytes: 0, + write_buffer_bytes: 0, + transient_bytes: 0, + } + } + + // GOV-U21 (ADR-071 test spec table, "Protection bytes"): Maintenance + // first borrows deep into Foreground's still-idle reserve — legitimate, + // since ADR-071 explicitly allows borrowing an unwaited class's unused + // reserve ("capacité empruntable"). Foreground then genuinely blocks on + // `acquire_admission`, becoming a real FIFO waiter. From that instant, + // the oracle applies: "aucune nouvelle opération Maintenance/cache ne + // peut augmenter le shortfall Foreground". This test proves it three + // ways — a further Maintenance admission, a class-less cache + // `try_reserve`, and a Maintenance-tagged cache `try_reserve` — are all + // refused, and `D_b(Foreground)` is bit-for-bit unchanged by each + // refusal. It also proves the other half of the oracle: releasing + // (never preempting) Maintenance's existing borrow is what actually + // drains the shortfall, and Foreground is admitted once it reaches 0. + #[test] + fn gov_u21_maintenance_borrow_then_foreground_waiter_shortfall_never_grows() { + const BUDGET: usize = 1_000_000; + const RESERVED_FG: usize = 300_000; + const RESERVED_MN: usize = 100_000; + + let config = GovernorConfig { + budget_bytes: BUDGET, + max_queue_depth: 20, + max_admission_waiters: 10, + reserved_slots: [2, 2], + reserved_bytes: [RESERVED_FG, RESERVED_MN], + weight: [1, 1], + max_intent_peak_bytes: [RESERVED_FG, RESERVED_MN], + governor_overhead_bytes: 0, + emergency_flush_headroom_bytes: 0, + }; + let d = MemoryDomain::new(config).expect("valid config"); + let governor = d.governor(); + + // Maintenance borrows 8 * 100_000 = 800_000 while Foreground has no + // waiter yet. + let mut mn_permits = Vec::new(); + for _ in 0..8 { + mn_permits.push( + governor + .try_acquire_admission(AdmissionClass::Maintenance, plan(RESERVED_MN)) + .expect("borrowing while Foreground has no waiter must succeed"), + ); + } + assert_eq!(governor.committed_bytes(), 800_000); + assert_eq!(governor.bytes_by_class(AdmissionClass::Maintenance), 800_000); + + // Foreground now genuinely blocks: only 200_000 bytes are free and it + // asked for 300_000 — proven non-blocking-would-fail below the same + // way, then actually parked via the blocking call on a thread. + let err = expect_err( + governor.try_acquire_admission(AdmissionClass::Foreground, plan(RESERVED_FG)), + "the domain must genuinely lack capacity for Foreground right now", + ); + assert!(matches!(err, EngineError::AdmissionWouldBlock)); + + let governor_for_thread = Arc::clone(governor); + let fg_thread = std::thread::spawn(move || { + governor_for_thread.acquire_admission(AdmissionClass::Foreground, plan(RESERVED_FG)) + }); + + let deadline = std::time::Instant::now() + Duration::from_secs(10); + while governor.admission_waiters() < 1 { + assert!(std::time::Instant::now() < deadline, "Foreground must park as a waiter"); + std::thread::yield_now(); + } + assert_eq!( + governor.bytes_by_class(AdmissionClass::Foreground), + 0, + "still waiting, not yet granted" + ); + + let shortfall_before = shortfall_bytes(governor, RESERVED_FG, AdmissionClass::Foreground, BUDGET); + assert_eq!( + shortfall_before, 100_000, + "D_b(Foreground) = max(0, 300_000-0) - 200_000" + ); + + // (1) A further Maintenance admission — even a single byte — must be + // refused: any growth shrinks free_bytes and would increase + // D_b(Foreground). + let refused = expect_err( + governor.try_acquire_admission(AdmissionClass::Maintenance, plan(1)), + "a further Maintenance admission must be refused once Foreground is a protected waiter", + ); + assert!(matches!(refused, EngineError::AdmissionWouldBlock)); + assert_eq!( + shortfall_bytes(governor, RESERVED_FG, AdmissionClass::Foreground, BUDGET), + shortfall_before, + "a refused Maintenance admission must not move the shortfall at all" + ); + + // (2) A class-less cache-style `try_reserve` must be refused too — + // exactly the "cache" half of the oracle. + let refused_cache = expect_err( + governor.try_reserve(GovernedMemoryKind::SstDataBlock, None, 1), + "a cache try_reserve must be refused once it would increase Foreground's shortfall", + ); + assert!(matches!(refused_cache, EngineError::AdmissionWouldBlock)); + assert_eq!( + shortfall_bytes(governor, RESERVED_FG, AdmissionClass::Foreground, BUDGET), + shortfall_before + ); + + // (3) A Maintenance-tagged cache reservation must be refused for the + // same reason — the protection is about global free bytes, not about + // who the bytes are attributed to. + let refused_cache_mn = expect_err( + governor.try_reserve(GovernedMemoryKind::SstDataBlock, Some(AdmissionClass::Maintenance), 1), + "a Maintenance-tagged cache reservation must also be refused", + ); + assert!(matches!(refused_cache_mn, EngineError::AdmissionWouldBlock)); + assert_eq!( + shortfall_bytes(governor, RESERVED_FG, AdmissionClass::Foreground, BUDGET), + shortfall_before, + "shortfall must be bit-for-bit unchanged after every refused attempt" + ); + assert_eq!( + governor.committed_bytes(), + 800_000, + "no refused attempt may have mutated committed_bytes" + ); + + // Draining (not preemption) is what actually shrinks the shortfall: + // releasing just 1 of Maintenance's 8 borrowed permits frees 100_000 + // bytes, taking free_bytes from 200_000 to 300_000 — exactly + // Foreground's own reserve, so `D_b(Foreground)` reaches 0. The + // scheduler re-runs synchronously inside this same release (under + // the same lock, before `drop` returns), so Foreground's grant is + // already visible in `bytes_by_class`/`committed_bytes` right here, + // even though the parked `fg_thread` still needs its own wakeup to + // observe it and return. + drop(mn_permits.pop().expect("a permit to drop")); + assert_eq!( + governor.bytes_by_class(AdmissionClass::Foreground), + RESERVED_FG, + "Foreground must have been granted synchronously the instant its shortfall drained to 0" + ); + assert_eq!( + shortfall_bytes(governor, RESERVED_FG, AdmissionClass::Foreground, BUDGET), + 0, + "once granted its full reserve, Foreground's own H_b term is itself 0" + ); + + let permit = fg_thread + .join() + .expect("thread") + .expect("Foreground must eventually be granted once its protection is satisfied"); + assert_eq!(permit.class(), AdmissionClass::Foreground); + drop(permit); + drop(mn_permits); + } + + // GOV-U22 (ADR-071 test spec table, "Protection slots"): the same + // scenario as GOV-U21, but on `outstanding_slots`/`slots_by_class` + // instead of bytes — "dette de slots cesse de croître puis draine". + // Reserved bytes here are deliberately generous so only slots ever + // bind, isolating the slot half of the protection formula. + // + // Numbers are chosen so the "further Maintenance admission refused" + // assertion is actually decided by the per-class protection loop in + // `capacity_eligible`, not by the earlier, unconditional + // `outstanding_slots >= max_queue_depth` guard: at least 1 raw free slot + // must remain globally at that point, so a version of the code that + // dropped the protection loop entirely would grant it (this is exactly + // what the red/green proof below exercises — see this test's `GOV-U22` + // report entry). + #[test] + fn gov_u22_maintenance_borrow_then_foreground_waiter_slot_debt_drains_not_grows() { + const MAX_QUEUE_DEPTH: usize = 10; + const RESERVED_SLOTS_FG: usize = 4; + const RESERVED_SLOTS_MN: usize = 2; + const BUDGET: usize = 10_000_000; + const RESERVED_BYTES: usize = 1_000_000; + const PEAK: usize = 1_000; + + let config = GovernorConfig { + budget_bytes: BUDGET, + max_queue_depth: MAX_QUEUE_DEPTH, + max_admission_waiters: 10, + reserved_slots: [RESERVED_SLOTS_FG, RESERVED_SLOTS_MN], + reserved_bytes: [RESERVED_BYTES, RESERVED_BYTES], + weight: [1, 1], + max_intent_peak_bytes: [PEAK, PEAK], + governor_overhead_bytes: 0, + emergency_flush_headroom_bytes: 0, + }; + let d = MemoryDomain::new(config).expect("valid config"); + let governor = d.governor(); + + // Maintenance borrows 8 of the 10 slots while Foreground has no + // waiter yet — legitimate borrowing of Foreground's unused reserve, + // leaving 2 slots genuinely free. + let mut mn_permits = Vec::new(); + for _ in 0..8 { + mn_permits.push( + governor + .try_acquire_admission(AdmissionClass::Maintenance, plan(PEAK)) + .expect("borrowing slots while Foreground has no waiter must succeed"), + ); + } + assert_eq!(governor.outstanding_slots(), 8); + assert_eq!(governor.slots_by_class(AdmissionClass::Maintenance), 8); + + // Foreground now genuinely blocks. Note this cannot be proven via a + // non-blocking `try_acquire_admission` pre-check the way GOV-U21 + // does for bytes: with 2 slots still raw-free and Foreground's own + // waiter queue still empty at the instant of that direct call, the + // per-class protection loop would skip Foreground too (queue-empty + // classes are exactly the "capacité empruntable" case) and the call + // would actually succeed — self-protection against one's own + // request only ever engages once a request is already the head of + // its own non-empty FIFO, i.e. inside `visit()` after + // `acquire_admission` has enqueued it. So this test parks Foreground + // directly and proves the block by polling the real waiter + // bookkeeping instead. + let governor_for_thread = Arc::clone(governor); + let fg_thread = + std::thread::spawn(move || governor_for_thread.acquire_admission(AdmissionClass::Foreground, plan(PEAK))); + + let deadline = std::time::Instant::now() + Duration::from_secs(10); + while governor.admission_waiters() < 1 { + assert!(std::time::Instant::now() < deadline, "Foreground must park as a waiter"); + std::thread::yield_now(); + } + assert_eq!( + governor.slots_by_class(AdmissionClass::Foreground), + 0, + "still waiting, not yet granted — confirms it genuinely parked rather than being granted instantly" + ); + + let debt_before = shortfall_slots(governor, RESERVED_SLOTS_FG, AdmissionClass::Foreground, MAX_QUEUE_DEPTH); + assert_eq!(debt_before, 2, "D_s(Foreground) = max(0, 4-0) - 2"); + + // A further Maintenance admission must be refused. Crucially, 1 slot + // is still raw-free at this point (`outstanding_slots` = 8 < 10), so + // this refusal can only come from the per-class protection loop + // protecting Foreground's shortfall — not from blanket exhaustion. + assert!( + governor.outstanding_slots() < MAX_QUEUE_DEPTH, + "a raw-free slot must still exist, so the refusal below is a genuine protection decision" + ); + let refused = expect_err( + governor.try_acquire_admission(AdmissionClass::Maintenance, plan(PEAK)), + "a further Maintenance admission must be refused once Foreground is a protected slot waiter", + ); + assert!(matches!(refused, EngineError::AdmissionWouldBlock)); + assert_eq!( + shortfall_slots(governor, RESERVED_SLOTS_FG, AdmissionClass::Foreground, MAX_QUEUE_DEPTH), + debt_before, + "a refused Maintenance admission must not move the slot debt at all" + ); + assert_eq!( + governor.outstanding_slots(), + 8, + "no refused attempt may have mutated outstanding_slots" + ); + + // Cache reservations never take a slot at all (ADR-071 GOV-U28: + // "Cache ne peut réserver" a slot) — `outstanding_slots` staying + // exactly put through a successful cache `try_reserve` is itself the + // proof it cannot possibly increase D_s. + let cache = governor + .try_reserve(GovernedMemoryKind::SstDataBlock, None, 1) + .expect("a slot-free cache reservation must not be blocked by slot protection"); + assert_eq!( + governor.outstanding_slots(), + 8, + "cache reservations never touch outstanding_slots" + ); + drop(cache); + + // Draining: a single released slot only shrinks the debt (2 -> 1), + // it does not yet satisfy Foreground's own post-grant floor + // (`reserved_slots[Foreground] - 1 == 3` still needs 3 free slots + // after granting, only 2 would be available) — releasing a second + // slot is what actually unblocks it, exactly the "cesse de croître + // puis draine" (stops growing, then drains) oracle GOV-U22 names. + drop(mn_permits.pop().expect("first permit to drop")); + assert_eq!(governor.outstanding_slots(), 7); + assert_eq!( + shortfall_slots(governor, RESERVED_SLOTS_FG, AdmissionClass::Foreground, MAX_QUEUE_DEPTH), + 1, + "one released slot must shrink, not eliminate, the debt" + ); + assert_eq!( + governor.slots_by_class(AdmissionClass::Foreground), + 0, + "still not enough to grant Foreground yet" + ); + + drop(mn_permits.pop().expect("second permit to drop")); + assert_eq!( + governor.slots_by_class(AdmissionClass::Foreground), + 1, + "Foreground must have been granted synchronously the instant its slot debt drained to 0" + ); + assert_eq!( + shortfall_slots(governor, RESERVED_SLOTS_FG, AdmissionClass::Foreground, MAX_QUEUE_DEPTH), + 0, + "once granted its own slot, Foreground's own H_s term drops to 3, matched by 3 free slots" + ); + + let permit = fg_thread + .join() + .expect("thread") + .expect("Foreground must eventually be granted once its slot protection is satisfied"); + assert_eq!(permit.class(), AdmissionClass::Foreground); + drop(permit); + drop(mn_permits); + } + + // GOV-U23 (ADR-071 test spec table, "Symétrie"): the exact mirror of + // GOV-U21 with the classes swapped — Foreground borrows into + // Maintenance's idle reserve, Maintenance becomes the waiter. Nothing in + // `scheduler::capacity_eligible` special-cases which class is which (it + // loops over `AdmissionClass::ALL` uniformly), so this must reproduce + // GOV-U21's exact shortfall numbers with the labels swapped — proving + // the protection logic is not accidentally hardcoded to protect + // Foreground only. + #[test] + fn gov_u23_foreground_borrow_then_maintenance_waiter_shortfall_never_grows() { + const BUDGET: usize = 1_000_000; + const RESERVED_FG: usize = 100_000; + const RESERVED_MN: usize = 300_000; + + let config = GovernorConfig { + budget_bytes: BUDGET, + max_queue_depth: 20, + max_admission_waiters: 10, + reserved_slots: [2, 2], + reserved_bytes: [RESERVED_FG, RESERVED_MN], + weight: [1, 1], + max_intent_peak_bytes: [RESERVED_FG, RESERVED_MN], + governor_overhead_bytes: 0, + emergency_flush_headroom_bytes: 0, + }; + let d = MemoryDomain::new(config).expect("valid config"); + let governor = d.governor(); + + // Foreground borrows 8 * 100_000 = 800_000 while Maintenance has no + // waiter yet. + let mut fg_permits = Vec::new(); + for _ in 0..8 { + fg_permits.push( + governor + .try_acquire_admission(AdmissionClass::Foreground, plan(RESERVED_FG)) + .expect("borrowing while Maintenance has no waiter must succeed"), + ); + } + assert_eq!(governor.committed_bytes(), 800_000); + assert_eq!(governor.bytes_by_class(AdmissionClass::Foreground), 800_000); + + let err = expect_err( + governor.try_acquire_admission(AdmissionClass::Maintenance, plan(RESERVED_MN)), + "the domain must genuinely lack capacity for Maintenance right now", + ); + assert!(matches!(err, EngineError::AdmissionWouldBlock)); + + let governor_for_thread = Arc::clone(governor); + let mn_thread = std::thread::spawn(move || { + governor_for_thread.acquire_admission(AdmissionClass::Maintenance, plan(RESERVED_MN)) + }); + + let deadline = std::time::Instant::now() + Duration::from_secs(10); + while governor.admission_waiters() < 1 { + assert!( + std::time::Instant::now() < deadline, + "Maintenance must park as a waiter" + ); + std::thread::yield_now(); + } + assert_eq!( + governor.bytes_by_class(AdmissionClass::Maintenance), + 0, + "still waiting, not yet granted" + ); + + let shortfall_before = shortfall_bytes(governor, RESERVED_MN, AdmissionClass::Maintenance, BUDGET); + assert_eq!( + shortfall_before, 100_000, + "D_b(Maintenance) = max(0, 300_000-0) - 200_000 — identical formula, opposite class" + ); + + let refused = expect_err( + governor.try_acquire_admission(AdmissionClass::Foreground, plan(1)), + "a further Foreground admission must be refused once Maintenance is a protected waiter", + ); + assert!(matches!(refused, EngineError::AdmissionWouldBlock)); + assert_eq!( + shortfall_bytes(governor, RESERVED_MN, AdmissionClass::Maintenance, BUDGET), + shortfall_before, + "a refused Foreground admission must not move Maintenance's shortfall at all" + ); + + let refused_cache = expect_err( + governor.try_reserve(GovernedMemoryKind::SstDataBlock, Some(AdmissionClass::Foreground), 1), + "a Foreground-tagged cache reservation must be refused once it would increase Maintenance's shortfall", + ); + assert!(matches!(refused_cache, EngineError::AdmissionWouldBlock)); + assert_eq!( + shortfall_bytes(governor, RESERVED_MN, AdmissionClass::Maintenance, BUDGET), + shortfall_before, + "shortfall must be bit-for-bit unchanged after every refused attempt" + ); + assert_eq!( + governor.committed_bytes(), + 800_000, + "no refused attempt may have mutated committed_bytes" + ); + + // Symmetric to GOV-U21: releasing just 1 of Foreground's 8 borrowed + // permits frees exactly the 100_000 bytes Maintenance's shortfall + // needed, granting it synchronously inside this same release. + drop(fg_permits.pop().expect("a permit to drop")); + assert_eq!( + governor.bytes_by_class(AdmissionClass::Maintenance), + RESERVED_MN, + "Maintenance must have been granted synchronously the instant its shortfall drained to 0" + ); + assert_eq!( + shortfall_bytes(governor, RESERVED_MN, AdmissionClass::Maintenance, BUDGET), + 0, + "once granted its full reserve, Maintenance's own H_b term is itself 0" + ); + + let permit = mn_thread + .join() + .expect("thread") + .expect("Maintenance must eventually be granted once its protection is satisfied"); + assert_eq!(permit.class(), AdmissionClass::Maintenance); + drop(permit); + drop(fg_permits); + } + + // ---- GOV-U25 / GOV-U26: spurious wake and lost wake ---- + + // GOV-U25: spurious wake — `notify_all()` fired with no capacity change + // at all — must never grant an ineligible waiter. ADR-071 + // §"Linéarisation de l'admission": "Le `Condvar` ne décide jamais quel + // waiter gagne. Le scheduler marque un waiter précis `Granted` sous le + // mutex ; un réveil OS ne fait que permettre aux threads de revérifier + // leur prédicat." Proven by reaching into the private `changed` field + // directly (this `tests` module is a descendant of `admission`'s own + // module, exactly like `gov_u38`'s direct access to `state` above) and + // firing repeated spurious wakeups with zero intervening state + // mutation — the closest in-process stand-in for an OS spurious futex + // wakeup, and a strictly harder scenario than a "real but insufficient" + // release would be, since here nothing at all changes. + #[test] + fn gov_u25_spurious_wake_without_new_capacity_grants_no_ineligible_waiter() { + let config = GovernorConfig { + budget_bytes: 1_000, + max_queue_depth: 3, + max_admission_waiters: 4, + reserved_slots: [1, 1], + reserved_bytes: [999, 1], + weight: [1, 1], + max_intent_peak_bytes: [999, 1], + governor_overhead_bytes: 0, + emergency_flush_headroom_bytes: 0, + }; + let d = MemoryDomain::new(config).expect("valid config"); + let governor = d.governor(); + + // Hog leaves only 1 free byte in Foreground: any request for more + // than that is provably, deterministically unsatisfiable until hog + // drops — not a timing window (same technique as GOV-U27 above). + let hog = governor + .try_acquire_admission( + AdmissionClass::Foreground, + MemoryPlan { + peak_bytes: 999, + intent_owned_bytes: 0, + write_buffer_bytes: 0, + transient_bytes: 0, + }, + ) + .expect("hog grant"); + + let waiter_governor = Arc::clone(governor); + let waiter = std::thread::spawn(move || { + waiter_governor.acquire_admission( + AdmissionClass::Foreground, + MemoryPlan { + peak_bytes: 500, + intent_owned_bytes: 0, + write_buffer_bytes: 0, + transient_bytes: 0, + }, + ) + }); + + let deadline = std::time::Instant::now() + Duration::from_secs(10); + while governor.admission_waiters() < 1 { + assert!(std::time::Instant::now() < deadline, "waiter must eventually enqueue"); + std::thread::yield_now(); + } + // Give the waiter thread a real chance to actually be parked inside + // `Condvar::wait`, not merely enqueued. + std::thread::sleep(Duration::from_millis(100)); + + let before_bytes = governor.committed_bytes(); + let before_slots = governor.outstanding_slots(); + + // The GOV-U25 scenario itself: fire repeated spurious wakeups with + // no capacity change whatsoever. + for _ in 0..50 { + governor.changed.notify_all(); + std::thread::sleep(Duration::from_millis(5)); + } + + assert_eq!( + governor.committed_bytes(), + before_bytes, + "a spurious wake must never grant bytes to an ineligible waiter" + ); + assert_eq!( + governor.outstanding_slots(), + before_slots, + "a spurious wake must never grant a slot to an ineligible waiter" + ); + assert_eq!( + governor.admission_waiters(), + 1, + "the waiter must still be parked (Pending), not granted-and-removed" + ); + + // Sanity: the waiter is not dead — real capacity release still + // grants it, proving the spurious wakes above did not corrupt its + // bookkeeping into some stuck state either. + drop(hog); + let permit = waiter + .join() + .expect("waiter thread") + .expect("must eventually be granted once real capacity frees up"); + assert_eq!(permit.class(), AdmissionClass::Foreground); + } + + // GOV-U26: lost wake — a capacity release timed to land exactly around + // a waiter's `Condvar::wait()` call must never leave that waiter parked + // forever. `acquire_admission`'s wake loop holds `GovernorState`'s + // mutex continuously between checking this waiter's own predicate + // (`state_of(seq)`) and calling `wait()` on it — no unlock/relock gap + // in between — which is the classic safe-monitor pattern + // `std::sync::Condvar` is built to make lost-wakeup-proof, *provided* + // the releasing side also mutates and notifies under the very same + // lock, which `apply_or_fail_stop` does. This test does not merely + // assert that reasoning; it races a releaser against a waiter released + // from the same `Barrier` hundreds of times, so the exact interleaving + // (release completing before the waiter even enqueues, racing its + // enqueue, racing its transition into `wait()`, or landing while it is + // already parked) varies from iteration to iteration — exactly the + // ADR-071 GOV-U26 scenario ("Release exactly autour de `wait`"). Each + // iteration is bounded by a hard per-iteration timeout, so a real lost + // wakeup shows up as a deterministic failure, never a silent hang — + // matching this project's GV-L7 proof style (`docs/status.md`: "le + // test échoue par timeout déterministe"). + #[test] + fn gov_u26_release_racing_wait_never_loses_the_wakeup() { + let iterations = 200; + for i in 0..iterations { + let config = GovernorConfig { + budget_bytes: 1_000, + max_queue_depth: 4, + max_admission_waiters: 4, + reserved_slots: [1, 1], + reserved_bytes: [999, 1], + weight: [1, 1], + max_intent_peak_bytes: [999, 1], + governor_overhead_bytes: 0, + emergency_flush_headroom_bytes: 0, + }; + let d = MemoryDomain::new(config).expect("valid config"); + let governor = d.governor(); + + let hog = governor + .try_acquire_admission( + AdmissionClass::Foreground, + MemoryPlan { + peak_bytes: 999, + intent_owned_bytes: 0, + write_buffer_bytes: 0, + transient_bytes: 0, + }, + ) + .expect("hog grant"); + + let (tx, rx) = std::sync::mpsc::channel(); + let start = Arc::new(Barrier::new(2)); + + let waiter_governor = Arc::clone(governor); + let waiter_start = Arc::clone(&start); + let waiter = std::thread::spawn(move || { + waiter_start.wait(); + let outcome = waiter_governor.acquire_admission( + AdmissionClass::Foreground, + MemoryPlan { + peak_bytes: 999, + intent_owned_bytes: 0, + write_buffer_bytes: 0, + transient_bytes: 0, + }, + ); + let _ = tx.send(outcome.is_ok()); + }); + + // No stagger on either side: both threads leave the barrier at + // essentially the same instant, so whether the release lands + // before the waiter enqueues, while it is enqueuing, while it + // is transitioning into `wait()`, or while it is already + // parked is genuinely up to the OS scheduler — and varies + // iteration to iteration. + start.wait(); + drop(hog); + + match rx.recv_timeout(Duration::from_secs(2)) { + Ok(true) => {} + Ok(false) => panic!("iteration {i}: waiter did not receive Ok(permit)"), + Err(_) => panic!( + "iteration {i}: waiter never observed the release within the hard per-iteration timeout — lost wakeup" + ), + } + waiter.join().expect("waiter thread must not panic"); + } + } + + // ---- GOV-U40/GOV-U41/GOV-S03 (ADR-071 test spec table) ---- + + /// Bounds a `JoinHandle::join()` by a deterministic timeout instead of + /// risking an actual process hang if the property under test is broken + /// (e.g. a governor `close()` that stops waking parked waiters). Mirrors + /// the coordinator-level precedent + /// (`closing_a_governed_coordinator_wakes_a_caller_parked_on_byte_capacity` + /// in `basemyai::storage::native_store::coordinator`), which bounds its + /// own wait the same way via `tokio::time::timeout`. A supervisor thread + /// does the actual (possibly unbounded) `join()`; the caller only ever + /// waits on a channel with a timeout, so a genuinely hung target thread + /// leaks one supervisor thread for the rest of the test process instead + /// of hanging the test runner itself. + fn join_within(name: &str, handle: std::thread::JoinHandle, timeout: Duration) -> T { + let (tx, rx) = mpsc::channel(); + std::thread::spawn(move || { + let _ = tx.send(handle.join()); + }); + match rx.recv_timeout(timeout) { + Ok(Ok(value)) => value, + Ok(Err(_)) => panic!("{name} thread panicked"), + Err(_) => panic!("{name} thread did not finish within {timeout:?} — suspected hang"), + } + } + + fn zero_plan(peak_bytes: usize) -> MemoryPlan { + MemoryPlan { + peak_bytes, + intent_owned_bytes: 0, + write_buffer_bytes: 0, + transient_bytes: 0, + } + } + + // GOV-U40: two engine-like consumers sharing one `Arc` — + // concretely, two independent `Arc` handles cloned from + // the very same domain (ADR-071 §"Portée du domaine": "un produit qui + // veut une limite commune partage le même `Arc`", + // `mod.rs:79-81`) — must never together commit more than the domain's + // single `budget_bytes`, and admission from *either* side must block + // once that shared budget is exhausted, regardless of which "engine" + // asked. + // + // Driven concurrently by two real OS threads racing for the same + // capacity via `try_reserve`, not sequentially — the governor's own + // mutex is what must serialize them correctly, not test ordering. + #[test] + fn gov_u40_two_engines_sharing_one_domain_respect_a_single_shared_budget() { + let budget = 2_000_000usize; + let d = domain(budget, budget / 2, budget / 2, 1, 1); + // Two "engines": independent `Arc` handles onto the one shared + // governor behind `d`'s single `MemoryDomain`. + let engine_a = Arc::clone(d.governor()); + let engine_b = Arc::clone(d.governor()); + + let per_chunk = 50_000usize; + assert_eq!( + budget % per_chunk, + 0, + "fixture must divide evenly for an exact post-hoc count" + ); + let barrier = Arc::new(Barrier::new(2)); + let mut handles = Vec::new(); + for engine in [Arc::clone(&engine_a), Arc::clone(&engine_b)] { + let barrier = Arc::clone(&barrier); + handles.push(std::thread::spawn(move || { + barrier.wait(); + let mut held = Vec::new(); + loop { + match engine.try_reserve(GovernedMemoryKind::SstDataBlock, None, per_chunk) { + Ok(reservation) => held.push(reservation), + Err(EngineError::AdmissionWouldBlock) => break, + Err(other) => panic!("unexpected error draining the shared budget: {other:?}"), + } + } + held + })); + } + + let mut per_engine_counts = Vec::new(); + let mut all_reservations = Vec::new(); + for h in handles { + let held = join_within("engine", h, Duration::from_secs(10)); + per_engine_counts.push(held.len()); + all_reservations.extend(held); + } + + // Oracle: the combined total from both engines exactly fills the + // *single* shared budget — not twice it, which is what two + // accidentally-independent per-engine budgets would allow. + assert_eq!( + per_engine_counts.iter().sum::(), + budget / per_chunk, + "combined chunks across both engines must exactly fill the one shared budget, got {per_engine_counts:?}" + ); + // Not asserted: that *both* engines got a non-zero share. + // `try_reserve`'s raw mutex has no fairness contract (unlike the DRR + // admission path), so an OS scheduler that always favors one thread + // could legitimately let it drain the whole budget alone — that is + // still "a single shared budget respected", just an unfair split of + // it. The sum-equals-budget assertion above is what actually proves + // sharing; see `per_engine_counts` for the observed split. + assert_eq!(engine_a.committed_bytes(), budget); + + // Further admission from *either* handle is now blocked — proving + // there is really one budget, not two that happen to sum correctly. + let err_a = expect_err( + engine_a.try_reserve(GovernedMemoryKind::SstDataBlock, None, 1), + "engine A must be blocked once the shared budget is exhausted", + ); + assert!(matches!(err_a, EngineError::AdmissionWouldBlock)); + let err_b = expect_err( + engine_b.try_reserve(GovernedMemoryKind::SstDataBlock, None, 1), + "engine B must be blocked too — the same shared budget, not an independent one", + ); + assert!(matches!(err_b, EngineError::AdmissionWouldBlock)); + + drop(all_reservations); + assert_eq!( + engine_a.committed_bytes(), + 0, + "every reservation from both engines must release exactly" + ); + } + + // GOV-U41: two *independent* `MemoryDomain`s (separate `MemoryGovernor`s, + // separate budgets) must never leak capacity between each other — + // saturating domain A must have zero effect on domain B's own admission, + // and neither governor's own accounting is a process-global metric. + #[test] + fn gov_u41_two_independent_domains_never_share_capacity() { + let budget_a = 500_000usize; + let budget_b = 700_000usize; + let domain_a = domain(budget_a, budget_a / 4, budget_a / 4, 1, 1); + let domain_b = domain(budget_b, budget_b / 4, budget_b / 4, 1, 1); + let engine_a = domain_a.governor(); + let engine_b = domain_b.governor(); + + // Saturate A completely. + let hog_a = engine_a + .try_reserve(GovernedMemoryKind::SstDataBlock, None, budget_a) + .expect("A's own full budget must be reservable on its own governor"); + assert_eq!(engine_a.committed_bytes(), budget_a); + let err = expect_err( + engine_a.try_reserve(GovernedMemoryKind::SstDataBlock, None, 1), + "A must now be fully saturated", + ); + assert!(matches!(err, EngineError::AdmissionWouldBlock)); + + // B is untouched: no shared counter means its own committed_bytes is + // still exactly zero, and it can still admit its own full budget, + // unaffected by A's saturation. + assert_eq!( + engine_b.committed_bytes(), + 0, + "domain B's committed_bytes must be entirely independent of domain A's saturation" + ); + let full_b = engine_b + .try_reserve(GovernedMemoryKind::SstDataBlock, None, budget_b) + .expect("domain B must still admit its own full budget while A is saturated"); + assert_eq!(engine_b.committed_bytes(), budget_b); + + // And the reverse holds: draining B to its own limit doesn't touch + // A's own view of itself either. + assert_eq!( + engine_a.committed_bytes(), + budget_a, + "A's committed_bytes must be unaffected by B's own saturation" + ); + let err = expect_err( + engine_b.try_reserve(GovernedMemoryKind::SstDataBlock, None, 1), + "B must now be saturated on its own budget, independent of A", + ); + assert!(matches!(err, EngineError::AdmissionWouldBlock)); + + drop(hog_a); + drop(full_b); + assert_eq!(engine_a.committed_bytes(), 0); + assert_eq!(engine_b.committed_bytes(), 0); + } + + // GOV-S03 (stress): shutdown with several live `MemoryReservation`s and + // multiple parked waiters across *both* admission classes in flight. + // `MemoryGovernor::close()` must (1) wake every parked waiter within a + // bounded, deterministic time — no hang; (2) admit nothing new from that + // point on, even a caller racing exactly at the close boundary; and (3) + // leave bookkeeping bounded — no leaked waiter entries, every + // pre-existing reservation still releases exactly. + #[test] + fn gov_s03_shutdown_wakes_every_waiter_blocks_new_admission_and_bounds_cleanup() { + let d = domain(1_000_000, 400_000, 400_000, 1, 1); + let governor = Arc::clone(d.governor()); + + // Several live reservations in flight, unrelated to admission — + // simulating in-progress cache/flush work that must survive + // shutdown without leaking (oracle 3). + let live_reservations: Vec<_> = (0..3) + .map(|_| { + governor + .try_reserve(GovernedMemoryKind::SstDataBlock, None, 10_000) + .expect("seed a live reservation") + }) + .collect(); + + // Hog nearly all remaining capacity so every acquire_admission below + // is genuinely forced to park, not racing a timing window. + let free_before_hog = 1_000_000 - governor.committed_bytes(); + let hog = governor + .try_reserve(GovernedMemoryKind::FlushWorkingSet, None, free_before_hog - 1) + .expect("hog nearly all remaining capacity"); + assert_eq!(governor.committed_bytes(), 1_000_000 - 1); + + // Several waiters across both admission classes. + const WAITERS: usize = 6; + let mut waiter_handles = Vec::new(); + for i in 0..WAITERS { + let g = Arc::clone(&governor); + let class = if i % 2 == 0 { + AdmissionClass::Foreground + } else { + AdmissionClass::Maintenance + }; + waiter_handles.push(std::thread::spawn(move || g.acquire_admission(class, zero_plan(100)))); + } + + // Deterministic, not timing-based: wait until every waiter has + // genuinely enqueued before triggering shutdown. + let deadline = Instant::now() + Duration::from_secs(10); + while governor.admission_waiters() < WAITERS { + assert!(Instant::now() < deadline, "all waiters must eventually park"); + std::thread::yield_now(); + } + + // A racer hammering non-blocking admission attempts concurrently + // with `close()`, to catch a caller racing exactly at the close + // boundary (oracle 2's strongest form): once it has observed one + // `AdmissionClosed`, no *later* attempt may ever succeed. + let racer_governor = Arc::clone(&governor); + let racer_stop = Arc::new(AtomicBool::new(false)); + let racer_stop_flag = Arc::clone(&racer_stop); + let racer_saw_admission_after_closed = Arc::new(AtomicBool::new(false)); + let racer_flag = Arc::clone(&racer_saw_admission_after_closed); + let racer = std::thread::spawn(move || { + let mut closed_seen = false; + while !racer_stop_flag.load(Ordering::SeqCst) { + match racer_governor.try_acquire_admission(AdmissionClass::Foreground, zero_plan(1)) { + Ok(_permit) => { + if closed_seen { + racer_flag.store(true, Ordering::SeqCst); + } + } + Err(EngineError::AdmissionClosed) => closed_seen = true, + Err(_) => {} + } + } + }); + + governor.close(); + + // Oracle 1: every parked waiter wakes within a bounded timeout — + // no hang — and with the correct typed error, never a silent + // success or an indefinite park. + for (i, h) in waiter_handles.into_iter().enumerate() { + let outcome = join_within("waiter", h, Duration::from_secs(10)); + assert!( + matches!(outcome, Err(EngineError::AdmissionClosed)), + "waiter {i} must wake with AdmissionClosed, not hang or succeed" + ); + } + + // Let the racer keep hammering briefly past the wake barrier above, + // then stop it and join with the same bounded discipline. + std::thread::sleep(Duration::from_millis(50)); + racer_stop.store(true, Ordering::SeqCst); + join_within("racer", racer, Duration::from_secs(10)); + assert!( + !racer_saw_admission_after_closed.load(Ordering::SeqCst), + "oracle 2: no admission may ever succeed once AdmissionClosed has been observed, \ + not even one racing the close boundary" + ); + + // Oracle 2, direct form: a fresh call against the now-closed + // governor is refused deterministically, not by luck. + let err = expect_err( + governor.try_acquire_admission(AdmissionClass::Maintenance, zero_plan(1)), + "a fresh call after close() must be refused, never granted", + ); + assert!(matches!(err, EngineError::AdmissionClosed)); + + // Oracle 3: bookkeeping is bounded — no leaked waiter entries, and + // every pre-existing reservation still releases exactly, even past + // shutdown. + assert_eq!( + governor.admission_waiters(), + 0, + "closed waiters' bookkeeping must be fully cleared, not leaked" + ); + drop(hog); + drop(live_reservations); + assert_eq!( + governor.committed_bytes(), + 0, + "every pre-existing reservation must still release exactly after shutdown" + ); + } } diff --git a/crates/basemyai-engine/src/memory_governor/mod.rs b/crates/basemyai-engine/src/memory_governor/mod.rs index 8b80697..a5e9c9e 100644 --- a/crates/basemyai-engine/src/memory_governor/mod.rs +++ b/crates/basemyai-engine/src/memory_governor/mod.rs @@ -71,7 +71,20 @@ pub use admission::{AdmissionPermit, MemoryGovernor}; pub use config::GovernorConfig; pub use governed::{Governed, SharedGoverned}; pub use reservation::MemoryReservation; -pub use types::{AdmissionClass, GovernedMemoryKind, MemoryPlan, estimate_memtable_reservation_bytes}; +pub use types::{ + AdmissionClass, GovernedMemoryKind, MemoryPlan, estimate_emergency_flush_headroom_bytes, + estimate_memtable_reservation_bytes, +}; +// Crate-internal, test-only: lets `store::sst_block::write`'s `#[cfg(test)]` +// tests ground the SstWriter-only half of the EmergencyHeadroom derivation +// against a real writer run (`store::sst_block` is a private module, a +// sibling of this one under the crate root, so it cannot reach `types` +// directly). Gated by `cfg(test)` only, matching that consumer exactly — the +// function itself is used unconditionally by +// `estimate_emergency_flush_headroom_bytes` within `types.rs`, so only this +// cross-module re-export needs gating at all. +#[cfg(test)] +pub(crate) use types::sst_writer_working_set_bytes; use scheduler::SchedulerQuanta; use state::GovernorState; diff --git a/crates/basemyai-engine/src/memory_governor/types.rs b/crates/basemyai-engine/src/memory_governor/types.rs index 1416f8e..e281441 100644 --- a/crates/basemyai-engine/src/memory_governor/types.rs +++ b/crates/basemyai-engine/src/memory_governor/types.rs @@ -3,7 +3,7 @@ //! (ADR-071 §"Catégories runtime" / §"Classes d'ownership" / API table). use crate::error::{EngineError, Result}; -use crate::format::{crypto, wal}; +use crate::format::{crypto, sst_block, wal}; use crate::store::INTERNAL_KEY_SUFFIX_BYTES; /// Observability/policy axis a [`crate::memory_governor::MemoryReservation`] @@ -368,6 +368,173 @@ pub fn estimate_memtable_reservation_bytes( mul(memtable_flush_threshold, per_entry_bytes) } +/// Worst-case working set one flush needs *beyond* the memtable generation +/// it is retiring (ADR-071 §"Headroom de progrès background"; ADR-072 +/// `CB-08`, which elevates this from a flush-progress margin to the +/// non-permanent-blocking condition of the §B rotation mechanism). ADR-071 +/// leaves the number `UNSPECIFIED`; this is its derivation. +/// +/// # Method: read `store::engine::flush::run_job` and the `SstWriter` it +/// drives end to end, not approximate +/// +/// Two terms, and — per the analysis below — only two, because everything +/// else in the flush path is either already-charged (the memtable itself) +/// or bounded independent of both by construction. +/// +/// **1. The dominant term: one more full copy of the memtable's content.** +/// `Memtable::iter_versions` (`store::memtable`) deep-clones every key and +/// value into a fresh `Vec` before `run_job` ever calls the writer; that +/// `Vec` is then moved into `SstWriter::write_new_versioned` and consumed by +/// `for (key, value) in entries` — but `Vec::IntoIter` keeps its *whole* +/// backing allocation live until the loop finishes, not just the unconsumed +/// tail. For the entire flush, the memtable being retired (already charged +/// under `WriteBufferMutable`) and this clone (today entirely uncounted by +/// the governor) coexist. Same per-entry shape, same worst-case entry count +/// as [`estimate_memtable_reservation_bytes`] — reused verbatim rather than +/// re-derived, so the two can never drift apart. +/// +/// **2. `SstWriter`'s own working set: `O(block_size)`, not `O(memtable +/// size)`.** Not an approximation — the writer's own documented design goal +/// (`format::sst_block::write`'s module doc: "the largest live allocation is +/// one data block... the writer contributes `O(block_size)` regardless of +/// how much is merged through it", the R6.2/ADR-049 §3 rewrite) and ADR-053's +/// discipline ("one fixed Bloom partition, one leaf builder, at most one +/// internal-node builder per index level"). Every sub-term below calls the +/// *exact* capacity-limit functions `SstWriter::create` itself calls, so this +/// estimate cannot silently drift from the real encoder: +/// +/// - staged data block: the `current: Vec` accumulator (struct +/// slots up to `vec_capacity_bound(max_data_block_entries(block_size))`, +/// the same rounding `SstWriter` itself reserves) plus its wire content +/// (bounded by `block_size`, the flush trigger) plus the re-encoded +/// `plain` buffer `stage_current_block` builds from it while the original +/// staged entries are still alive (`write_staged_block` only drops +/// `stage.entries` after sealing) — another `block_size`; +/// - if encrypted, `seal_section`'s ciphertext and envelope copies (each +/// `plain.len() + ENCRYPTED_SST_BLOCK_OVERHEAD_BYTES`-sized), coexisting +/// with `plain` for the duration of one seal call; +/// - the leaf Bloom partition: one fixed bit array sized directly from +/// `block_size` (`bloom_partition_bits_len`); +/// - the leaf index builder (`leaf_entries`): its slot count +/// (`vec_capacity_bound(max_leaf_index_entries(index_chunk_plaintext_limit))`) +/// is sized by `SstWriter::create` for the worst case of *many small* +/// entries, so multiplying that count by a max-key-sized entry would +/// double-count — `preflight_leaf` caps the real accumulated key-byte +/// content at `index_chunk_plaintext_limit` directly and flushes before +/// exceeding it, so the term is `capacity × struct_size + +/// index_chunk_plaintext_limit`, two independently-bounded quantities +/// added, not one multiplied by the other; +/// - the internal index levels: at most one bounded builder *per level* +/// (same struct-slots-plus-content-cap shape as the leaf index), and +/// `MAX_INDEX_LEVELS` (32) is a fixed format constant — independent of +/// dataset volume, so this term is a fixed multiple of one level's bound, +/// never something that grows with the memtable. +/// +/// # What this does not close +/// +/// This is `basemyai-engine`'s own flush working set — it says nothing about +/// whether anything in this crate actually consumes a governed reservation +/// for it yet (nothing does: `store::engine::flush` never touches +/// `memory_governor`, same as `store::engine::write`'s MG-17 wiring — both +/// phase 2, ADR-071's own "Correction (audit code)" note). Deriving this +/// number is a prerequisite for closing `EmergencyHeadroom`'s "NON PROUVÉ" +/// status, not the whole of it: the *operational* proof (`GOV-U32` — a real +/// flush completing under a saturated domain) needs that phase-2 wiring to +/// exist first. +/// +/// # Errors +/// [`EngineError::MemoryAccountingOverflow`] if any step overflows `usize`, +/// or if `block_size`/`max_key_bytes` cannot support even one Bloom key or +/// one index entry (the same configurations `EngineOptions::validate` +/// already rejects) — `checked_add`/`checked_mul` throughout (MG-06). +pub fn estimate_emergency_flush_headroom_bytes( + memtable_flush_threshold: usize, + max_key_bytes: usize, + max_value_bytes: usize, + block_size: u32, + encrypted: bool, +) -> Result { + let memtable_clone_term = + estimate_memtable_reservation_bytes(memtable_flush_threshold, max_key_bytes, max_value_bytes)?; + let sst_writer_term = sst_writer_working_set_bytes(block_size, max_key_bytes, encrypted)?; + add(&[memtable_clone_term, sst_writer_term]) +} + +/// The `SstWriter`-only half of [`estimate_emergency_flush_headroom_bytes`]'s +/// derivation — `pub(crate)` (rather than private) specifically so +/// `store::sst_block::write`'s own tests can ground it directly against a +/// real `SstWriter` run (`memory_governor` and `store::sst_block` are +/// siblings under the crate root; `store::sst_block` is a private module, so +/// this function must cross that boundary, not the writer). That grounding +/// is the strongest check available: not just "this arithmetic is +/// internally consistent" but "this number was never exceeded by the actual +/// encoder it describes." +pub(crate) fn sst_writer_working_set_bytes(block_size: u32, max_key_bytes: usize, encrypted: bool) -> Result { + let block_size_usize = block_size as usize; + + let data_entries_capacity_limit = sst_block::vec_capacity_bound( + sst_block::max_data_block_entries(block_size).ok_or(EngineError::MemoryAccountingOverflow)?, + ) + .ok_or(EngineError::MemoryAccountingOverflow)?; + let staged_entries_struct_term = mul(data_entries_capacity_limit, size_of::())?; + // `current`'s wire content plus the re-encoded `plain` buffer built from + // it while it is still alive — both bounded by `block_size`. + let plain_copies_term = mul(block_size_usize, 2)?; + let crypto_term = if encrypted { + mul(add(&[block_size_usize, crypto::ENCRYPTED_SST_BLOCK_OVERHEAD_BYTES])?, 2)? + } else { + 0 + }; + let staged_block_term = add(&[staged_entries_struct_term, plain_copies_term, crypto_term])?; + + // One fixed Bloom partition, sized directly from `block_size` — not from + // the memtable's entry count. + let bloom_term = sst_block::bloom_partition_bits_len(block_size_usize) + .unwrap_or(0) + .checked_div(8) + .unwrap_or(0); + + let index_chunk_plaintext_limit = sst_block::index_chunk_plaintext_limit(block_size, max_key_bytes) + .ok_or(EngineError::MemoryAccountingOverflow)?; + + // `max_leaf_index_entries`/`max_index_node_children` size a Vec *slot + // count* assuming minimum-size entries (`INDEX_ENTRY_FIXED_BYTES`/ + // `INDEX_NODE_CHILD_MIN_BYTES`, ignoring key length) — the same + // worst-case-many-small-entries reasoning `SstWriter::create` itself + // uses to size `leaf_entries_capacity_limit`/`internal_children_ + // capacity_limit`. Multiplying that count by a *max-key-sized* entry + // would double-count: `preflight_leaf`/`push_internal_child` cap the + // real accumulated key-byte content at `index_chunk_plaintext_limit` + // directly (`leaf_encoded_len`/`internal_level_encoded_lens`, checked + // against that same limit before every push) and flush *before* + // exceeding it — so the struct-slot term (capacity × fixed struct size) + // and the key-content term (bounded by `index_chunk_plaintext_limit` + // itself, not by capacity × max_key_bytes) are two genuinely separate, + // independently-bounded quantities, not one multiplied by the other. + let leaf_entries_capacity_limit = sst_block::vec_capacity_bound( + sst_block::max_leaf_index_entries(index_chunk_plaintext_limit).ok_or(EngineError::MemoryAccountingOverflow)?, + ) + .ok_or(EngineError::MemoryAccountingOverflow)?; + let leaf_slots_term = mul(leaf_entries_capacity_limit, size_of::())?; + let leaf_index_term = add(&[leaf_slots_term, index_chunk_plaintext_limit])?; + + let internal_children_capacity_limit = sst_block::vec_capacity_bound( + sst_block::max_index_node_children(index_chunk_plaintext_limit).ok_or(EngineError::MemoryAccountingOverflow)?, + ) + .ok_or(EngineError::MemoryAccountingOverflow)?; + let node_slots_term = mul( + internal_children_capacity_limit, + size_of::(), + )?; + let per_level_term = add(&[node_slots_term, index_chunk_plaintext_limit])?; + // MAX_INDEX_LEVELS is a fixed format constant (32), not a function of + // this flush's data volume: at most one bounded builder is resident per + // level at a time, and the format caps the number of levels outright. + let internal_levels_term = mul(per_level_term, sst_block::MAX_INDEX_LEVELS as usize)?; + + add(&[staged_block_term, bloom_term, leaf_index_term, internal_levels_term]) +} + /// Transient bytes the encrypted WAL arm adds on top of the framed plain /// record — the AAD, the AEAD ciphertext and the on-disk envelope, all live /// at once with the record inside `store::wal::Wal::write_record`'s @@ -740,4 +907,107 @@ mod plan_tests { .expect_err("entry-count product must overflow, not wrap"); assert!(matches!(err, EngineError::MemoryAccountingOverflow)); } + + // ---- EmergencyHeadroom (ADR-071 §"Headroom de progrès background" / + // ADR-072 CB-08): derivation, not yet the operational GOV-U32 proof ---- + + #[test] + fn encrypted_sst_block_overhead_matches_the_real_envelope() { + // Pinned so a change to `EncryptedSstBlock`'s framing fails here, + // naming the cause, instead of silently shrinking the crypto term + // below what `seal_section` really allocates. + assert_eq!( + crypto::ENCRYPTED_SST_BLOCK_OVERHEAD_BYTES, + 50, + "magic+version+nonce+ct_len header (34) + Poly1305 tag (16)" + ); + assert_eq!( + sst_block::MAX_INDEX_LEVELS, + 32, + "SstFooter's index_levels field is a u8, not unbounded" + ); + } + + #[test] + fn flush_headroom_is_the_memtable_clone_plus_the_sst_writer_term() { + let memtable_term = estimate_memtable_reservation_bytes(100, 50, 200).expect("no overflow"); + let writer_term = sst_writer_working_set_bytes(4096, 50, false).expect("no overflow"); + let headroom = estimate_emergency_flush_headroom_bytes(100, 50, 200, 4096, false).expect("no overflow"); + assert_eq!( + headroom, + memtable_term + writer_term, + "the two terms this ADR identifies must be the whole of it, summed exactly" + ); + } + + #[test] + fn flush_headroom_encrypted_exceeds_plaintext_by_exactly_the_crypto_term() { + let plaintext = sst_writer_working_set_bytes(4096, 50, false).expect("no overflow"); + let encrypted = sst_writer_working_set_bytes(4096, 50, true).expect("no overflow"); + let expected_crypto_term = (4096 + crypto::ENCRYPTED_SST_BLOCK_OVERHEAD_BYTES) * 2; + assert_eq!(encrypted - plaintext, expected_crypto_term); + } + + #[test] + fn flush_headroom_overflow_is_typed_never_wrapped() { + let err = estimate_emergency_flush_headroom_bytes(usize::MAX, usize::MAX, 1, 4096, false) + .expect_err("the memtable clone term must overflow, not wrap"); + assert!(matches!(err, EngineError::MemoryAccountingOverflow)); + + let err = sst_writer_working_set_bytes(u32::MAX, usize::MAX, false) + .expect_err("an unreasonable max_key_bytes must overflow the writer term, not wrap"); + assert!(matches!(err, EngineError::MemoryAccountingOverflow)); + } + + /// Sanity check against a real engine configuration: the derived + /// headroom must stay dominated by the (already-pinned, ~16.6 GiB) + /// memtable clone term, not by the writer term — confirming the writer + /// term really is the small `O(block_size)` correction the doc comment + /// claims, not a second unbounded quantity hiding in the sum. + #[test] + fn default_engine_options_headroom_is_dominated_by_the_memtable_clone_term() { + let options = crate::store::EngineOptions::default(); + let memtable_term = estimate_memtable_reservation_bytes( + options.memtable_flush_threshold, + options.max_key_bytes, + options.max_value_bytes, + ) + .expect("no overflow"); + let writer_term = + sst_writer_working_set_bytes(options.block_size, options.max_key_bytes, false).expect("no overflow"); + let headroom = estimate_emergency_flush_headroom_bytes( + options.memtable_flush_threshold, + options.max_key_bytes, + options.max_value_bytes, + options.block_size, + false, + ) + .expect("no overflow"); + + assert_eq!(headroom, memtable_term + writer_term); + // Pinned exactly, not just bounded: dominated by + // `internal_levels_term`'s `index_chunk_plaintext_limit (~4 MiB) * + // MAX_INDEX_LEVELS (32)` ≈ 128 MiB — deliberately conservative for a + // 1 MiB `max_key_bytes` config (a real 1000-entry flush could never + // actually build 32 index levels; the format constant is the same + // regardless of how few entries a given flush holds). A future + // change to any of `sst_block`'s capacity-limit functions or to + // `EngineOptions::default()` should fail *here*, naming the cause, + // rather than silently moving the real headroom this crate would + // reserve. + assert_eq!(writer_term, 420_582_824); + assert!( + writer_term < memtable_term / 10, + "the SstWriter's own O(block_size) term ({writer_term}) must stay a modest correction on top of \ + the memtable clone term ({memtable_term}), not a comparably large quantity" + ); + } + + // The real-`SstWriter` grounding test for `sst_writer_working_set_bytes` + // lives in `store::sst_block::write`'s own test module + // (`sst_writer_term_bounds_a_real_writers_steady_state_across_many_ + // blocks_and_leaves`) — `store::sst_block` is a private module, a + // sibling of `memory_governor` under the crate root, so `SstWriter` + // cannot be reached from here; the function under test is `pub(crate)` + // specifically so that module can call back into this one instead. } diff --git a/crates/basemyai-engine/src/store/memtable.rs b/crates/basemyai-engine/src/store/memtable.rs index 77cb440..70626c4 100644 --- a/crates/basemyai-engine/src/store/memtable.rs +++ b/crates/basemyai-engine/src/store/memtable.rs @@ -133,6 +133,18 @@ impl MemtableChargeHandle { .as_ref() .map(MemoryReservation::bytes) } + + /// Test-only observability (ADR-072 CB-T5): the number of live strong + /// references to this generation's `Arc` right now — a witness + /// for proving the CB-07 release-window deviation stays bounded, never a + /// decision input (production code must never branch on + /// `Arc::strong_count`, which Rust documents as racy the moment more + /// than one thread can observe it — ADR-071 §"Pinning `Arc`", MG-14). + #[cfg(any(test, feature = "test-util"))] + #[must_use] + pub fn strong_count(&self) -> usize { + Arc::strong_count(&self.0) + } } impl Memtable { diff --git a/crates/basemyai-engine/src/store/sst_block/write.rs b/crates/basemyai-engine/src/store/sst_block/write.rs index b0210df..8038fa5 100644 --- a/crates/basemyai-engine/src/store/sst_block/write.rs +++ b/crates/basemyai-engine/src/store/sst_block/write.rs @@ -972,6 +972,67 @@ mod tests { assert_eq!(loaded.entries().expect("entries"), data); } + // ADR-071 §"Headroom de progrès background" / ADR-072 CB-08: grounds + // `memory_governor::sst_writer_working_set_bytes`'s steady-state terms + // (staged block, Bloom partition, leaf index, internal levels — every + // component `SstWriter::resident_bytes` can observe from outside) + // against a real writer run, not just arithmetic that agrees with + // itself. Same fixture shape as `partitioned_writer_builds_a_ + // hierarchical_index` above (already proven to force multiple leaves + // *and* an internal index root), so every term the estimate computes is + // actually exercised here, not vacuously zero. + // + // `resident_bytes()` cannot see the transient `plain`/sealed/ciphertext + // copies `write_staged_block`/`seal_section` allocate and free within a + // single call (by design — its own doc comment defers that closure + // oracle to `r6_allocator_peak`), so this test validates the + // *steady-state* portion of the estimate exhaustively and leaves the + // transient-buffer portion to the direct code-reading recorded in that + // function's own doc comment. + #[test] + fn sst_writer_term_bounds_a_real_writers_steady_state_across_many_blocks_and_leaves() { + let dir = tempfile::tempdir().expect("tempdir"); + let data = entries(200, 40); + let cache = Arc::new(MetadataCache::new(8 * 1024 * 1024)); + let block_size: u32 = 128; + let max_key_bytes: usize = 32; + let mut writer = SstWriter::create(dir.path(), 0, block_size, None, max_key_bytes, 0, Arc::clone(&cache)) + .expect("create writer"); + + let bound = crate::memory_governor::sst_writer_working_set_bytes(block_size, max_key_bytes, false) + .expect("no overflow"); + let mut peak_resident_bytes = 0usize; + for (index, (key, value)) in data.iter().enumerate() { + let kind = if value.is_some() { + ValueKind::Value + } else { + ValueKind::Tombstone + }; + writer + .push(InternalKey::new(key.clone(), 1, kind), value.clone()) + .expect("push"); + peak_resident_bytes = peak_resident_bytes.max(writer.resident_bytes()); + assert!( + writer.resident_bytes() <= bound, + "writer.resident_bytes()={} exceeded the derived steady-state bound={bound} after {index} pushes", + writer.resident_bytes() + ); + } + let written = writer.finish().expect("finish"); + assert!( + written.footer.bloom_partition_count > 1, + "fixture must force multiple leaves — otherwise the leaf-index/Bloom terms are untested" + ); + assert!( + written.footer.index_levels > 1, + "fixture must force an internal index root — otherwise the internal-levels term is untested" + ); + assert!( + peak_resident_bytes > 0, + "the writer must have actually held something resident at some point" + ); + } + #[test] fn versioned_roundtrip_preserves_versions_and_snapshot_visibility() { let dir = tempfile::tempdir().expect("tempdir"); diff --git a/crates/basemyai-engine/tests/governor/cb_t5_bounded_release_window.rs b/crates/basemyai-engine/tests/governor/cb_t5_bounded_release_window.rs new file mode 100644 index 0000000..c727b93 --- /dev/null +++ b/crates/basemyai-engine/tests/governor/cb_t5_bounded_release_window.rs @@ -0,0 +1,286 @@ +// SPDX-License-Identifier: BUSL-1.1 +//! ADR-072 `CB-T5` — proves the documented `MG-15`/`MG-16` deviation at +//! `store::engine::flush.rs:560` (the memtable generation's governed charge +//! is released at flush-completion, not the true last `Arc` drop) +//! stays bounded to the lifetime of one concurrent, in-flight `Engine::get` +//! reference — never latent — exactly the claim in ADR-072 +//! §"Dérogation documentée à MG-15/16": "fenêtre bornée à la durée d'un seul +//! appel `Engine::get` synchrone en mémoire, jamais non-bornée". +//! +//! ## Reproduction strategy +//! +//! `Engine::snapshot()` (`store/engine/mod.rs`) is *exactly* the mechanism +//! `Engine::get` uses internally (`store/engine/read.rs:23`: `let snapshot = +//! self.snapshot();`) — it clones `Arc`, which in turn holds +//! `Arc` clones for whatever is currently mutable/sealed, and +//! drops that clone when the local variable goes out of scope. `Engine::get` +//! does this and returns within one synchronous call; nothing about the +//! mechanism differs if a caller holds the same public `ReadSnapshot` type +//! deliberately for a controlled window instead. +//! +//! This test takes a snapshot with the public API *before* triggering a +//! flush and holds it deliberately across the *entire* +//! `Engine::flush_memtables_only()` call. That call blocks the test thread +//! while the engine's own `basemyai-flush` background worker thread (spawned +//! by `FlushWorker::spawn`) does the actual sealed-memtable write and the +//! charge release at `flush.rs:560` — so two real, independent OS threads +//! are genuinely involved, and because the snapshot's lifetime spans the +//! *whole* flush call, the overlap with the release site is guaranteed by +//! construction, not left to a timing race. This is deliberately not a +//! probabilistic reproduction: ADR-072's own point 4 ("Snapshots — cas +//! prouvé, pas théorique") already identifies this exact call shape +//! (`WriteIntent::MemoryUpdate` → `Engine::get` → a local `ReadSnapshot`) as +//! the one site the deviation is characterised against, so pinning the +//! window by construction proves the same property the ADR claims without +//! needing a failpoint to hit a lucky interleaving. +//! +//! A second, independent `MemtableChargeHandle` (`witness`, ADR-072 §B's +//! public rotation-tracking handle) is kept alive purely as an +//! `Arc::strong_count` witness (test-util only — see +//! `MemtableChargeHandle::strong_count`'s doc comment: production code must +//! never branch on this count, only observe it). It proves the window's +//! *bound*: strong count returns to exactly its pre-overlap baseline the +//! instant the concurrent snapshot is dropped, with nothing else left +//! holding the generation alive. + +use basemyai_engine::{AdmissionClass, Engine, EngineOptions, GovernedMemoryKind, GovernorConfig, MemoryDomain}; + +fn charge_domain(budget_bytes: usize) -> MemoryDomain { + MemoryDomain::new(GovernorConfig { + budget_bytes, + max_queue_depth: 8, + max_admission_waiters: 8, + reserved_slots: [2, 2], + reserved_bytes: [budget_bytes / 2, budget_bytes / 4], + weight: [1, 1], + max_intent_peak_bytes: [budget_bytes / 2, budget_bytes / 4], + governor_overhead_bytes: 0, + emergency_flush_headroom_bytes: 0, + }) + .expect("valid governor config") +} + +/// Seal is driven explicitly by `flush_memtables_only()`, never by the +/// ordinary threshold/target-bytes auto-seal path — this isolates the one +/// seal+flush this test cares about. +fn explicit_seal_options() -> EngineOptions { + EngineOptions { + memtable_flush_threshold: usize::MAX, + memtable_target_bytes: usize::MAX, + ..EngineOptions::default() + } +} + +/// `CB-T5`: reproduces the `CB-07` release window and proves it collapses +/// exactly when the concurrent reference (the `Engine::get`-equivalent +/// snapshot) is dropped — never later, never left dangling. +#[test] +fn cb_t5_release_window_is_bounded_to_the_concurrent_readers_lifetime() { + let dir = tempfile::tempdir().expect("tempdir"); + let mut engine = Engine::open_with_options(dir.path(), explicit_seal_options()).expect("open"); + + let domain = charge_domain(1_000_000); + let governor = domain.governor(); + const CHARGE_BYTES: usize = 200_000; + + // Attach the generation's charge exactly the way `basemyai`'s + // owner_loop does at rotation (ADR-072 §B / CB-05): reserve, attach via + // a handle, then drop that handle — production never retains it past + // the attach call itself. + let reservation = governor + .try_reserve( + GovernedMemoryKind::WriteBufferMutable, + Some(AdmissionClass::Foreground), + CHARGE_BYTES, + ) + .expect("reserve the generation's worst-case charge"); + { + let attach_handle = engine.active_memtable_charge_handle(); + attach_handle + .attach(reservation) + .map_err(|_| ()) + .expect("a fresh generation accepts its charge"); + } + + let key = b"cb-t5-key"; + let value = vec![0x42u8; 4096]; + engine.put(key, &value).expect("seed the generation with real data"); + + // A second, independent handle onto the *same* generation, kept alive + // only as an `Arc::strong_count` witness — never consulted by the engine + // or the governor to decide anything, exactly like a test's `Weak` + // upgrade check would be. Its *absolute* count is not meaningful (the + // engine itself holds more than one internal reference to its own + // current memtable — e.g. both the `Engine::memtable` field and the + // published `SuperVersion`'s `mutable()` field — even with no concurrent + // reader involved at all); only the *delta* the concurrent snapshot adds + // and removes is what CB-T5 needs to prove. + let witness = engine.active_memtable_charge_handle(); + assert!( + witness.same_memtable_as(&engine.active_memtable_charge_handle()), + "no rotation has happened yet" + ); + + let committed_before_flush = governor.committed_bytes(); + assert!( + committed_before_flush >= CHARGE_BYTES, + "the generation's charge must be committed before flush" + ); + + // The "concurrent Engine::get" stand-in: `Engine::snapshot()` is exactly + // what `Engine::get` builds and drops internally. Taking it here and + // holding it deliberately across the whole flush call reproduces + // ADR-072 point 4's scenario with a real extra `Arc` clone, + // deterministically overlapping the release at `flush.rs:560`. + let concurrent_snapshot = engine.snapshot(); + + // Runs the actual seal + SST write + catalogue publish + charge release + // on the engine's own background `basemyai-flush` worker thread — + // genuine cross-thread concurrency with `concurrent_snapshot`, which is + // held here on this thread's stack, completely untouched by the flush + // call itself. + engine + .flush_memtables_only() + .expect("flush must complete while a concurrent reader still holds the sealed generation"); + + // --- The deviation, reproduced --- + // The governor already released the charge at flush-completion + // (flush.rs:560), *unconditionally*, regardless of `concurrent_snapshot` + // still holding a live `Arc` to the very same generation. + let committed_after_flush = governor.committed_bytes(); + assert_eq!( + committed_after_flush, + committed_before_flush - CHARGE_BYTES, + "CB-07: the charge must already be released at flush-completion, independent of the \ + concurrent snapshot still pinning the same memtable generation" + ); + + // Real memory is still resident and reachable through the concurrent + // reference — this *is* the transient undercount CB-07 documents: the + // governor already reports fewer committed bytes than what is genuinely + // still allocated and reachable. + assert_eq!( + concurrent_snapshot + .get(key) + .expect("read through the concurrent snapshot"), + Some(value), + "the flushed generation's data must still be resident and reachable through the \ + concurrent snapshot even after the governor has already released its charge" + ); + + // The witness's strong count right now, post-flush, with + // `concurrent_snapshot` still alive — captured only to compute the delta + // below, never compared to an absolute number (see the field comment + // above for why an absolute baseline is not meaningful here). + let strong_count_during_overlap = witness.strong_count(); + + // --- The bound, proved --- + // Dropping the concurrent reference is exactly what a synchronous + // `Engine::get` call returning does internally. The window must + // collapse *here*, not at some later, unbounded point: strong count must + // drop by *exactly* one — the concurrent snapshot's own contribution, + // and nothing else lingering (which would mean some other + // engine-internal referent, e.g. a stale `published.sealed` entry or the + // flush job's own `Arc`, was also still alive, i.e. the deviation's real + // window is wider than the one documented site). + drop(concurrent_snapshot); + let strong_count_after_drop = witness.strong_count(); + assert_eq!( + strong_count_during_overlap, + strong_count_after_drop + 1, + "CB-T5: dropping the concurrent reader's reference must reduce strong references by \ + exactly one — a smaller or larger drop would mean the CB-07 deviation is NOT bounded to \ + one synchronous Engine::get call, contrary to what ADR-072 documents \ + (during={strong_count_during_overlap}, after={strong_count_after_drop})" + ); + // Absolute check, not just a delta: once the concurrent reader is gone, + // `witness` must be the *only* live reference left — exactly 1. This is + // what actually catches a permanent leak inside the flush path itself + // (e.g. a bug that forgets to drop `FlushJob::memtable` and instead + // keeps it alive forever): such a bug would still pass the delta check + // above (both `during` and `after` would be off by the same constant), + // but not this one. + assert_eq!( + strong_count_after_drop, 1, + "CB-T5: after the concurrent reader drops and no external caller is involved at all, the \ + flushed generation's only remaining strong reference must be this test's own witness \ + handle — anything else means the flush path itself is leaking a reference beyond the \ + documented, bounded CB-07 deviation" + ); + + // Governor accounting was never touched again by any of this — it + // dropped exactly once, at flush-completion, and stays there regardless + // of what happens to the concurrent reader afterward. + assert_eq!( + governor.committed_bytes(), + committed_after_flush, + "governor accounting must not move again just because the concurrent reader dropped" + ); +} + +/// `CB-T5` companion: repeats the same overlap `N` times in a tight loop +/// (fresh generation, fresh concurrent snapshot each iteration) to rule out +/// the window only *happening* to collapse once by luck of allocator/thread +/// scheduling. Every iteration must show the same exactly-one-extra-then- +/// zero-extra shape. +#[test] +fn cb_t5_release_window_is_bounded_across_many_repeated_generations() { + let dir = tempfile::tempdir().expect("tempdir"); + let mut engine = Engine::open_with_options(dir.path(), explicit_seal_options()).expect("open"); + let domain = charge_domain(10_000_000); + let governor = domain.governor(); + const CHARGE_BYTES: usize = 8_192; + const ITERATIONS: usize = 25; + + for iteration in 0..ITERATIONS { + let reservation = governor + .try_reserve( + GovernedMemoryKind::WriteBufferMutable, + Some(AdmissionClass::Foreground), + CHARGE_BYTES, + ) + .unwrap_or_else(|error| panic!("iteration {iteration}: reserve charge: {error:?}")); + { + let attach_handle = engine.active_memtable_charge_handle(); + attach_handle + .attach(reservation) + .map_err(|_| ()) + .unwrap_or_else(|()| panic!("iteration {iteration}: attach charge")); + } + let key = format!("cb-t5-loop-key-{iteration}"); + engine + .put(key.as_bytes(), b"v") + .unwrap_or_else(|error| panic!("iteration {iteration}: put: {error:?}")); + + let witness = engine.active_memtable_charge_handle(); + let before = governor.committed_bytes(); + + let snapshot = engine.snapshot(); + engine + .flush_memtables_only() + .unwrap_or_else(|error| panic!("iteration {iteration}: flush: {error:?}")); + + assert_eq!( + governor.committed_bytes(), + before - CHARGE_BYTES, + "iteration {iteration}: charge must release exactly at flush-completion" + ); + let during_overlap = witness.strong_count(); + + drop(snapshot); + let after_drop = witness.strong_count(); + assert_eq!( + during_overlap, + after_drop + 1, + "iteration {iteration}: dropping the concurrent reader must reduce strong references \ + by exactly one, every iteration, not just the first \ + (during={during_overlap}, after={after_drop})" + ); + assert_eq!( + after_drop, 1, + "iteration {iteration}: after the concurrent reader drops, only this iteration's \ + witness handle must remain live — a leaked reference here would still pass the delta \ + check above, which is exactly why this absolute check exists too" + ); + } +} diff --git a/crates/basemyai-engine/tests/governor/gov_rotation_rss_oracle.rs b/crates/basemyai-engine/tests/governor/gov_rotation_rss_oracle.rs new file mode 100644 index 0000000..e438cdd --- /dev/null +++ b/crates/basemyai-engine/tests/governor/gov_rotation_rss_oracle.rs @@ -0,0 +1,230 @@ +// SPDX-License-Identifier: BUSL-1.1 +//! ADR-072 §"Points ouverts" — the memtable-rotation-specific RSS/stress +//! oracle, still open after `GOV-RSS01..03`: "Oracle RSS/stress dédié à +//! l'écart `governed_committed_bytes` vs mémoire réelle pour le mécanisme de +//! rotation §B". `gov_rss_oracle.rs`'s own module doc says explicitly that its +//! generic `GovernedRing` cache harness "does not attempt" this one — "it +//! requires a real `Engine` driven through many rotations". This file is +//! that oracle. +//! +//! ## What "`charged = resident + pinned_only`" means for §B specifically +//! +//! `gov_rss_oracle.rs` proves this equation for a governed *cache*, where an +//! evicted-but-still-referenced entry stays charged until its last reader +//! drops. The memtable rotation mechanism (`CB-05`/`CB-06`/`CB-07`) is +//! deliberately asymmetric to that: `CB-T5` +//! (`cb_t5_bounded_release_window.rs`) already proved the charge for a +//! generation releases *unconditionally at flush-completion*, independent of +//! whether a concurrent reader still pins that generation's real, resident +//! `Arc`. So for this mechanism, `pinned_only`'s contribution to +//! `charged` is always exactly zero, by construction — the real deviation is +//! that `resident` can transiently *exceed* `charged` (a pinned, flushed +//! generation is real resident memory the governor has already stopped +//! counting), never the other way around. What this file proves is the +//! sharper, meaningful invariant that claim reduces to for a mechanism with +//! this shape, repeated across many real rotation cycles rather than CB-T5's +//! single/25-iteration same-shape loop: +//! +//! - **No under-accounting**: a generation's charge is committed before any +//! write can land in it — attach happens strictly before the first `put` +//! every single cycle, never after. +//! - **No double-accounting**: `committed_bytes()` never exceeds exactly one +//! `CHARGE_BYTES` at a time, even when a caller wrongly attempts a second +//! attach onto an already-charged generation (rejected, verified not to +//! move accounting at all). +//! - **No growing leak**: `committed_bytes()` returns to *exactly* zero after +//! every one of many (`GENERATIONS`) rotate→write→(pin?)→flush cycles — +//! flat, not drifting upward with iteration count — regardless of whether +//! that generation was pinned by a concurrent reader across its flush. +//! - **Pinning never leaves a dangling reference**: every pinned generation's +//! witness strong count returns to exactly 1 (only this test's own witness +//! handle left) once its concurrent reader drops — cycle after cycle, not +//! just the first time. +//! - **Teardown**: the domain accepts a fresh reservation again after the +//! whole run and the engine drops cleanly — nothing left the domain, or the +//! engine, in a state that would refuse further legitimate use. + +use basemyai_engine::{AdmissionClass, Engine, EngineOptions, GovernedMemoryKind, GovernorConfig, MemoryDomain}; + +fn charge_domain(budget_bytes: usize) -> MemoryDomain { + MemoryDomain::new(GovernorConfig { + budget_bytes, + max_queue_depth: 8, + max_admission_waiters: 8, + reserved_slots: [2, 2], + reserved_bytes: [budget_bytes / 2, budget_bytes / 4], + weight: [1, 1], + max_intent_peak_bytes: [budget_bytes / 2, budget_bytes / 4], + governor_overhead_bytes: 0, + emergency_flush_headroom_bytes: 0, + }) + .expect("valid governor config") +} + +/// Seal driven explicitly by `flush_memtables_only()` only — same rationale +/// as `cb_t5_bounded_release_window.rs`: deterministic, one seal per call, +/// never a threshold race. +fn explicit_seal_options() -> EngineOptions { + EngineOptions { + memtable_flush_threshold: usize::MAX, + memtable_target_bytes: usize::MAX, + ..EngineOptions::default() + } +} + +const CHARGE_BYTES: usize = 8_192; +const GENERATIONS: usize = 40; +/// Every third generation is pinned by a concurrent snapshot held across its +/// flush (the `CB-07` window) — the rest flush with no concurrent reader at +/// all, so the oracle covers both shapes every run, not just one. +const PIN_EVERY: usize = 3; + +#[test] +fn memtable_rotation_charge_is_exact_and_flat_across_many_pinned_and_unpinned_generations() { + let dir = tempfile::tempdir().expect("tempdir"); + let mut engine = Engine::open_with_options(dir.path(), explicit_seal_options()).expect("open"); + let domain = charge_domain(CHARGE_BYTES * 4); + let governor = domain.governor(); + + for generation in 0..GENERATIONS { + // --- rotation + attach, strictly before any write: no under-accounting --- + assert_eq!( + governor.committed_bytes(), + 0, + "generation {generation}: the domain must be fully drained before the next \ + generation's charge — any leftover here would be a leak from a previous cycle" + ); + let reservation = governor + .try_reserve( + GovernedMemoryKind::WriteBufferMutable, + Some(AdmissionClass::Foreground), + CHARGE_BYTES, + ) + .unwrap_or_else(|error| panic!("generation {generation}: reserve the generation's charge: {error:?}")); + // Scoped exactly like `basemyai`'s own `attach_memtable_charge` + // (`coordinator.rs`): the handle used to attach is never retained + // past the attach call itself — holding it longer would itself be an + // extra strong reference the leak checks below would (rightly) catch + // as if it were a real engine-side leak. + { + let handle = engine.active_memtable_charge_handle(); + handle + .attach(reservation) + .map_err(|_| ()) + .unwrap_or_else(|()| panic!("generation {generation}: a fresh generation must accept its charge")); + assert_eq!( + handle.charged_bytes(), + Some(CHARGE_BYTES), + "generation {generation}: the handle must report exactly the reservation just attached" + ); + assert_eq!( + governor.committed_bytes(), + CHARGE_BYTES, + "generation {generation}: exactly one charge is committed, attached before any write" + ); + + // --- no double-accounting: a second attach on the same + // generation is rejected outright and never perturbs the + // governor's accounting, even attempted. Only exercised once — + // the property does not depend on the generation index, and + // repeating it every cycle would just add noise. + if generation == 0 { + let duplicate = governor + .try_reserve( + GovernedMemoryKind::WriteBufferMutable, + Some(AdmissionClass::Foreground), + 1, + ) + .expect("a 1-byte probe reservation must fit the domain's remaining free bytes"); + let rejected = handle + .attach(duplicate) + .expect_err("a generation that already carries a charge must reject a second attach"); + assert_eq!( + rejected.bytes(), + 1, + "the rejected reservation must be handed back untouched, not consumed" + ); + drop(rejected); + assert_eq!( + governor.committed_bytes(), + CHARGE_BYTES, + "generation 0: a rejected double-attach must leave accounting exactly where it was, \ + never CHARGE_BYTES + 1" + ); + } + } + + // --- real write into the now-charged generation --- + let key = format!("gov-rotation-rss-key-{generation}"); + engine + .put(key.as_bytes(), b"v") + .unwrap_or_else(|error| panic!("generation {generation}: put: {error:?}")); + + // --- flush, optionally pinned across it (the CB-07 window) --- + let witness = engine.active_memtable_charge_handle(); + let pinned = generation % PIN_EVERY == 0; + let concurrent_snapshot = pinned.then(|| engine.snapshot()); + + engine + .flush_memtables_only() + .unwrap_or_else(|error| panic!("generation {generation}: flush: {error:?}")); + + // --- release: unconditional at flush-completion, pinned or not --- + assert_eq!( + governor.committed_bytes(), + 0, + "generation {generation}: the charge must release exactly at flush-completion \ + regardless of whether a concurrent reader (pinned={pinned}) still holds this generation" + ); + + if let Some(snapshot) = concurrent_snapshot { + assert_eq!( + snapshot.get(key.as_bytes()).expect("read through the pinned snapshot"), + Some(b"v".to_vec()), + "generation {generation}: the flushed generation's data must still be resident and \ + reachable through the pinned snapshot even though the governor already released it" + ); + let during_overlap = witness.strong_count(); + drop(snapshot); + let after_drop = witness.strong_count(); + assert_eq!( + during_overlap, + after_drop + 1, + "generation {generation}: dropping the pinned reader must reduce strong references \ + by exactly one (during={during_overlap}, after={after_drop})" + ); + } + // No growing leak, pinned or not: after this generation's own reader + // (if any) has dropped, only this loop's own witness handle remains. + assert_eq!( + witness.strong_count(), + 1, + "generation {generation}: no reference from this generation must survive past this point \ + — a leaked reference here would accumulate across {GENERATIONS} generations and this \ + check would eventually catch it even if generation {generation} alone looked clean" + ); + } + + // --- teardown: the domain is left exactly as usable as it started --- + assert_eq!( + governor.committed_bytes(), + 0, + "after {GENERATIONS} full rotation cycles, the domain must be back at zero, not drifted" + ); + let post_run_probe = governor + .try_reserve( + GovernedMemoryKind::WriteBufferMutable, + Some(AdmissionClass::Foreground), + CHARGE_BYTES, + ) + .expect("the domain must still accept a fresh reservation after the whole run — no residual leak blocking it"); + drop(post_run_probe); + assert_eq!(governor.committed_bytes(), 0); + + drop(engine); + assert_eq!( + governor.committed_bytes(), + 0, + "dropping the engine itself must not move governor accounting — teardown is clean" + ); +} diff --git a/crates/basemyai-engine/tests/governor/gov_rss_oracle.rs b/crates/basemyai-engine/tests/governor/gov_rss_oracle.rs new file mode 100644 index 0000000..bb8f661 --- /dev/null +++ b/crates/basemyai-engine/tests/governor/gov_rss_oracle.rs @@ -0,0 +1,514 @@ +// SPDX-License-Identifier: BUSL-1.1 +//! `GOV-RSS01..03` (ADR-071 §"Tests normatifs, oracle RSS et critères de +//! ratification") — the *generic* governor RSS oracle, extending the +//! ADR-053/D-6 flatness methodology (`tests/engine/r6_allocator_peak.rs`, +//! `memory_bounds::r6_memory_bounds`) to `memory_governor`'s own accounting +//! rather than the compaction-merge allocator it was originally built for. +//! +//! **Scope note (explicit, per the task that added this file):** this closes +//! `GOV-RSS01..03` as ADR-071 states them — generic properties of +//! `MemoryGovernor`/`Governed` under growing volume, a negative control, +//! and pinned-reader residency. It does **not** close the separate, still- +//! open oracle ADR-072 §"Points ouverts" calls out: "Oracle RSS/stress dédié +//! à l'écart `governed_committed_bytes` vs mémoire réelle du mécanisme §B" — +//! that one is specific to repeated *memtable-generation* rotation inside +//! `basemyai-engine`'s real write path (`CB-05`/`CB-06`/`CB-07`) and requires +//! a real `Engine` driven through many rotations, which this file does not +//! attempt. +//! +//! ## Harness shape +//! +//! A small governed LRU-style cache (`GovernedRing`) is built directly on +//! `memory_governor`'s public API — `MemoryDomain`, `MemoryGovernor::try_reserve`, +//! `Governed`/`SharedGoverned` — exactly the shape ADR-071 documents a +//! real cache using it would take (§"Reclaim": "Une insertion cache n'attend +//! jamais indéfiniment... bypass"). Fixed config (cache capacity, reader +//! concurrency) and growing *volume* (total insert/evict operations) is the +//! same axis ADR-053's flatness oracle already established +//! (`docs/adr/ADR-053-...md`, `memory_bounds::r6_memory_bounds`): a bounded +//! working set must not grow with dataset/operation volume, only with +//! configuration and concurrency. +//! +//! RSS is sampled with the same native-API shape as +//! `tests/engine/r6_allocator_peak.rs`'s `process_memory` (Windows +//! `K32GetProcessMemoryInfo`, Linux `/proc/self/status`, macOS +//! `proc_pid_rusage`), simplified to self-process sampling since this +//! oracle does not need r6's cross-process allocator-category isolation — +//! `memory_governor` has no global-allocator category instrumentation to +//! isolate, so an in-process, real-heap-backed cache plus periodic RSS +//! samples of this same process is a faithful, much cheaper measurement. +//! +//! ## Slack thresholds +//! +//! `RSS_FLATNESS_SLACK_BYTES` below is a first, deliberately generous +//! provisional threshold for this new oracle — not yet the output of a +//! dedicated per-platform calibration campaign the way +//! `rss_flatness_min_bytes` in `r6_allocator_peak.rs` was (ADR-053 §"Les +//! seuils numériques de slack ne sont pas inventés ici"). ADR-071 explicitly +//! defers that campaign ("Les seuils numériques de slack ne sont pas +//! inventés ici. Ils sont mesurés par plateforme avant activation par +//! défaut") — this file's thresholds should be revisited by that same future +//! campaign, on this platform (Windows, per `docs/status.md`) and others. + +use std::collections::VecDeque; +use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::{Arc, Barrier, Mutex}; +use std::thread; + +use basemyai_engine::{Governed, GovernedMemoryKind, GovernorConfig, MemoryDomain, MemoryGovernor, SharedGoverned}; + +/// One governed cache entry's payload size. Deliberately real heap bytes +/// (not a zero-sized marker) so a genuine leak would actually show up in +/// process RSS, not just in a counter. +const ENTRY_BYTES: usize = 8 * 1024; +/// Fixed cache capacity — a *configuration* constant, never derived from +/// volume. This, plus `READER_CONCURRENCY`, is the entire analytical bound +/// on resident + pinned governed bytes. +const CACHE_CAPACITY_ENTRIES: usize = 128; +/// Fixed concurrent-pinned-reader count (GOV-RSS03) — also configuration, +/// not volume. +const READER_CONCURRENCY: usize = 6; + +#[derive(Debug, Clone, Copy)] +struct ProcessMemory { + rss_bytes: u64, +} + +#[cfg(target_os = "windows")] +fn process_memory() -> Option { + use windows_sys::Win32::System::ProcessStatus::{K32GetProcessMemoryInfo, PROCESS_MEMORY_COUNTERS}; + use windows_sys::Win32::System::Threading::GetCurrentProcess; + + // SAFETY: `GetCurrentProcess` returns a valid pseudo-handle that never + // needs closing; `counters` is exactly the struct size the API expects. + unsafe { + let handle = GetCurrentProcess(); + let mut counters = PROCESS_MEMORY_COUNTERS { + cb: std::mem::size_of::() as u32, + ..std::mem::zeroed() + }; + let ok = K32GetProcessMemoryInfo(handle, &raw mut counters, counters.cb) != 0; + ok.then_some(ProcessMemory { + rss_bytes: counters.WorkingSetSize as u64, + }) + } +} + +#[cfg(target_os = "linux")] +fn process_memory() -> Option { + let text = std::fs::read_to_string("/proc/self/smaps_rollup") + .ok() + .or_else(|| std::fs::read_to_string("/proc/self/status").ok())?; + let kib = |name: &str| -> Option { + let line = text.lines().find(|line| line.starts_with(name))?; + line.split_whitespace().nth(1)?.parse::().ok() + }; + let rss_kib = kib("Rss:").or_else(|| kib("VmRSS:"))?; + Some(ProcessMemory { + rss_bytes: rss_kib * 1024, + }) +} + +#[cfg(target_os = "macos")] +fn process_memory() -> Option { + #[repr(C)] + struct RusageInfoV4 { + uuid: [u8; 16], + fields: [u64; 35], + } + + impl Default for RusageInfoV4 { + fn default() -> Self { + Self { + uuid: [0; 16], + fields: [0; 35], + } + } + } + + #[link(name = "proc")] + unsafe extern "C" { + fn proc_pid_rusage(pid: i32, flavor: i32, buffer: *mut std::ffi::c_void) -> i32; + } + const RUSAGE_INFO_V4: i32 = 4; + + // SAFETY: `usage` has the exact public rusage_info_v4 C layout and + // remains live for the duration of the call. + unsafe { + let mut usage = RusageInfoV4::default(); + let pid = std::process::id() as i32; + (proc_pid_rusage(pid, RUSAGE_INFO_V4, (&raw mut usage).cast()) == 0).then_some(ProcessMemory { + rss_bytes: usage.fields[6], + }) + } +} + +/// A minimal governed LRU-style cache, built directly on `memory_governor`'s +/// public RAII API — the shape ADR-071 documents a real cache using it would +/// take. Insertion never blocks (§"Reclaim": a cache bypasses rather than +/// waits); eviction drops the oldest entry's `SharedGoverned`, releasing its +/// `MemoryReservation` exactly when the last `Arc` referencing it (this +/// ring's own slot, plus any pinning reader) actually drops (MG-14/MG-15). +/// GOV-RSS02 negative-control-only leak sink. Deliberately a **process- +/// lifetime `static`**, not a `GovernedRing` field: the whole point of the +/// negative control is a leak that outlives any one scenario run (a real bug +/// would not politely clean itself up when the cache it leaked out of goes +/// out of scope). An earlier version of this harness scoped the sink to the +/// ring instance and `drop(ring)` at the end of each `run_scenario` call +/// silently freed it again — the RSS extent measured only ~800 KiB instead +/// of the ~150 MiB the leak should have produced across the volume sweep, +/// which caught this harness bug rather than proving anything about a real +/// one. Fixed by moving the sink here. +#[cfg(feature = "governor-rss-negative-control-leak")] +static NEGATIVE_CONTROL_LEAK_SINK: Mutex>> = Mutex::new(Vec::new()); + +struct GovernedRing { + governor: Arc, + capacity: usize, + entries: Mutex>>>, +} + +impl GovernedRing { + fn new(governor: &Arc, capacity: usize) -> Self { + Self { + governor: Arc::clone(governor), + capacity, + entries: Mutex::new(VecDeque::with_capacity(capacity)), + } + } + + /// Inserts one fresh `ENTRY_BYTES`-sized entry, evicting the oldest one + /// if the ring is already at `capacity`. Returns the fresh entry's + /// handle so a caller can optionally pin it (GOV-RSS03). + /// + /// Bypasses (returns `None`, inserting nothing) if the governor has no + /// room right now — the correct cache contract (ADR-071 §"Reclaim"), + /// never a panic or a block. + fn insert(&self, seed: u8) -> Option>> { + let reservation = self + .governor + .try_reserve(GovernedMemoryKind::SstDataBlock, None, ENTRY_BYTES) + .ok()?; + let payload = vec![seed; ENTRY_BYTES]; + let shared: SharedGoverned> = Arc::new(Governed::new(payload, reservation)); + let mut entries = self.entries.lock().unwrap_or_else(std::sync::PoisonError::into_inner); + if entries.len() >= self.capacity + && let Some(evicted) = entries.pop_front() + { + self.retire(evicted); + } + entries.push_back(Arc::clone(&shared)); + Some(shared) + } + + /// The correctly-governed release path: dropping the evicted `Arc` + /// releases its `MemoryReservation` the instant no pinning reader holds + /// another clone (MG-15) — real memory genuinely returns to the + /// allocator once the last reference goes. + #[cfg(not(feature = "governor-rss-negative-control-leak"))] + fn retire(&self, evicted: SharedGoverned>) { + drop(evicted); + } + + /// GOV-RSS02 negative control: still drops the governed `Arc` — so + /// `MemoryGovernor::committed_bytes()` stays perfectly bounded and would + /// tell an operator everything is fine — but *also* retains an + /// unrelated, ungoverned raw copy of the same bytes forever. This is + /// exactly the failure mode an RSS oracle exists to catch and a + /// governor-internal-only oracle structurally cannot: a real allocation + /// the reservation accounting never even knew about. + #[cfg(feature = "governor-rss-negative-control-leak")] + fn retire(&self, evicted: SharedGoverned>) { + let copy: Vec = (**evicted).clone(); + NEGATIVE_CONTROL_LEAK_SINK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .push(copy); + drop(evicted); + } + + fn committed_bytes(&self) -> usize { + self.governor.committed_bytes() + } +} + +fn tiny_domain(budget_bytes: usize, max_queue_depth: usize) -> MemoryDomain { + MemoryDomain::new(GovernorConfig { + budget_bytes, + max_queue_depth, + max_admission_waiters: max_queue_depth, + reserved_slots: [1, 1], + reserved_bytes: [budget_bytes / 2, budget_bytes / 2], + weight: [1, 1], + max_intent_peak_bytes: [budget_bytes / 2, budget_bytes / 2], + governor_overhead_bytes: 0, + emergency_flush_headroom_bytes: 0, + }) + .expect("valid governor config") +} + +/// Runs `volume` sequential insert/evict operations through a +/// fixed-capacity `GovernedRing`, with `READER_CONCURRENCY` background +/// threads each continuously pinning (`Arc::clone`-ing) the most recent +/// entry they can see for a short window before dropping it again — +/// simulating concurrent readers racing eviction (GOV-RSS03), fixed in +/// count regardless of `volume`. +/// +/// Returns the process RSS sampled once quiescent (all reader threads +/// joined, all evictions settled) and the governor's own peak +/// `committed_bytes` observed during the run. +fn run_scenario(volume: usize) -> (u64, usize) { + // Budget generous enough that only the *cache's own* capacity/reader + // bound ever limits it — never the volume. + let budget_bytes = (CACHE_CAPACITY_ENTRIES + READER_CONCURRENCY + 8) * ENTRY_BYTES * 4; + let domain = tiny_domain(budget_bytes, CACHE_CAPACITY_ENTRIES + READER_CONCURRENCY + 8); + let governor = Arc::clone(domain.governor()); + let ring = Arc::new(GovernedRing::new(&governor, CACHE_CAPACITY_ENTRIES)); + + let stop = Arc::new(AtomicBool::new(false)); + let latest: Arc>>>> = Arc::new(Mutex::new(None)); + let start_barrier = Arc::new(Barrier::new(READER_CONCURRENCY + 1)); + + let readers: Vec<_> = (0..READER_CONCURRENCY) + .map(|_| { + let stop = Arc::clone(&stop); + let latest = Arc::clone(&latest); + let start_barrier = Arc::clone(&start_barrier); + thread::spawn(move || { + start_barrier.wait(); + while !stop.load(Ordering::Acquire) { + let pinned = latest.lock().unwrap_or_else(std::sync::PoisonError::into_inner).clone(); + if let Some(pinned) = pinned { + // Hold the pin briefly — long enough to genuinely + // overlap eviction races, short enough that the + // sweep below still finishes promptly. + std::hint::black_box(&*pinned); + thread::yield_now(); + drop(pinned); + } else { + thread::yield_now(); + } + } + }) + }) + .collect(); + + start_barrier.wait(); + let mut peak_committed = ring.committed_bytes(); + for i in 0..volume { + let seed = (i % 251) as u8; + if let Some(fresh) = ring.insert(seed) { + *latest.lock().unwrap_or_else(std::sync::PoisonError::into_inner) = Some(fresh); + } + peak_committed = peak_committed.max(ring.committed_bytes()); + } + + stop.store(true, Ordering::Release); + for reader in readers { + reader.join().expect("reader thread"); + } + // Drop the last pinned handle and let every remaining ring entry go out + // of scope with `ring` itself, so the quiescent sample below reflects a + // genuinely settled state, not readers still mid-pin. + *latest.lock().unwrap_or_else(std::sync::PoisonError::into_inner) = None; + drop(ring); + drop(domain); + + // A few yields to let the allocator actually return freed pages before + // sampling — RSS (unlike a heap counter) can lag a `drop()` briefly on + // some platforms. + for _ in 0..8 { + thread::yield_now(); + } + let sample = process_memory().expect("native RSS sample must be available on a supported platform"); + (sample.rss_bytes, peak_committed) +} + +/// `GOV-RSS01`: extends ADR-053's flatness methodology to the governor. +/// Same fixed cache capacity/reader concurrency, growing operation volume — +/// RSS extent across volumes must stay within a calibrated slack, not grow +/// with volume. +#[test] +#[cfg_attr( + feature = "governor-rss-negative-control-leak", + ignore = "GOV-RSS01 is the positive case; the negative control below is what runs under this feature" +)] +fn gov_rss01_committed_and_rss_stay_flat_across_growing_volumes() { + const VOLUMES: [usize; 3] = [500, 4_000, 20_000]; + // Provisional slack (see module doc): generous relative to + // `ENTRY_BYTES * (CACHE_CAPACITY_ENTRIES + READER_CONCURRENCY)` — about + // 1.05 MiB analytically for this config — pending a dedicated per- + // platform calibration campaign, matching ADR-071's own deferral of + // numeric slack values. + const RSS_FLATNESS_SLACK_BYTES: u64 = 24 * 1024 * 1024; + + let analytical_bound_bytes = ((CACHE_CAPACITY_ENTRIES + READER_CONCURRENCY) * ENTRY_BYTES) as u64; + + let mut rss_points = Vec::new(); + let mut committed_points = Vec::new(); + for &volume in &VOLUMES { + let (rss_bytes, peak_committed) = run_scenario(volume); + eprintln!( + "GOV-RSS01 volume={volume}: rss_bytes={rss_bytes} peak_committed={peak_committed} \ + analytical_bound={analytical_bound_bytes}" + ); + rss_points.push(rss_bytes); + committed_points.push(peak_committed as u64); + } + + let committed_extent = committed_points.iter().max().unwrap() - committed_points.iter().min().unwrap(); + assert!( + committed_extent <= analytical_bound_bytes, + "GOV-RSS01: governor committed-bytes extent {committed_extent} across growing volumes exceeds \ + the configuration-derived analytical bound {analytical_bound_bytes} — accounting is tracking \ + volume, not configuration+concurrency" + ); + for &committed in &committed_points { + assert!( + committed <= analytical_bound_bytes + (ENTRY_BYTES as u64), + "GOV-RSS01: peak committed bytes {committed} exceeds the analytical bound {analytical_bound_bytes} \ + (+ one entry's slack for an in-flight insert racing eviction)" + ); + } + + let rss_extent = rss_points.iter().max().unwrap() - rss_points.iter().min().unwrap(); + eprintln!("GOV-RSS01: rss points={rss_points:?} extent={rss_extent} slack={RSS_FLATNESS_SLACK_BYTES}"); + assert!( + rss_extent <= RSS_FLATNESS_SLACK_BYTES, + "GOV-RSS01: RSS extent {rss_extent} across growing volumes {VOLUMES:?} exceeds the flatness \ + slack {RSS_FLATNESS_SLACK_BYTES} — real process memory is tracking operation volume instead \ + of plateauing at the fixed cache-capacity + reader-concurrency bound" + ); +} + +/// `GOV-RSS02`: negative control. Built and run only under +/// `--features governor-rss-negative-control-leak`, which makes +/// `GovernedRing::retire` leak an ungoverned copy of every evicted entry — +/// invisible to `MemoryGovernor::committed_bytes()`, which stays perfectly +/// flat throughout — while real RSS grows linearly with volume. This proves +/// GOV-RSS01's *RSS* assertion (not just its governor-accounting assertion) +/// is actually load-bearing: without it, this exact bug would pass silently +/// forever. +#[test] +#[cfg(feature = "governor-rss-negative-control-leak")] +fn gov_rss02_negative_control_the_oracle_detects_a_volume_proportional_leak() { + const VOLUMES: [usize; 3] = [500, 4_000, 20_000]; + const RSS_FLATNESS_SLACK_BYTES: u64 = 24 * 1024 * 1024; + + let mut rss_points = Vec::new(); + let mut committed_points = Vec::new(); + for &volume in &VOLUMES { + let (rss_bytes, peak_committed) = run_scenario(volume); + eprintln!("GOV-RSS02 (leaking) volume={volume}: rss_bytes={rss_bytes} peak_committed={peak_committed}"); + rss_points.push(rss_bytes); + committed_points.push(peak_committed as u64); + } + + // The governor's OWN accounting must stay exactly as flat as GOV-RSS01 + // — this is the crux of the negative control: the leak is real but + // structurally invisible to `committed_bytes()`, because the leaked + // copy was never reserved through the governor at all. + let committed_extent = committed_points.iter().max().unwrap() - committed_points.iter().min().unwrap(); + let analytical_bound_bytes = ((CACHE_CAPACITY_ENTRIES + READER_CONCURRENCY) * ENTRY_BYTES) as u64; + assert!( + committed_extent <= analytical_bound_bytes, + "sanity: the governor's own bookkeeping must still look perfectly bounded during the leak — \ + if this fails, the negative control is leaking through the reservation accounting too, which \ + would not prove what GOV-RSS02 needs to prove" + ); + + // But real RSS must NOT look bounded: it must exceed the same flatness + // slack GOV-RSS01 uses, growing with volume. This is the detection. + let rss_extent = rss_points.iter().max().unwrap() - rss_points.iter().min().unwrap(); + eprintln!("GOV-RSS02: rss points={rss_points:?} extent={rss_extent} slack={RSS_FLATNESS_SLACK_BYTES}"); + assert!( + rss_extent > RSS_FLATNESS_SLACK_BYTES, + "GOV-RSS02: the deliberately introduced volume-proportional leak (extent {rss_extent}) failed to \ + exceed the flatness slack {RSS_FLATNESS_SLACK_BYTES} — the RSS oracle is NOT sensitive enough to \ + detect a real leak the governor's own accounting cannot see; GOV-RSS01 would be a rubber stamp" + ); + assert!( + rss_points[2] > rss_points[0], + "GOV-RSS02: RSS at the largest volume ({}) must exceed RSS at the smallest ({}) — the leak must \ + actually correlate with volume, not just be noisy", + rss_points[2], + rss_points[0] + ); +} + +/// `GOV-RSS03`: pinned real reads. With eviction actively cycling entries +/// out of the ring while `READER_CONCURRENCY` threads continuously pin +/// whatever the latest entry is, the peak governed bytes observed must +/// still respect `charged = resident + pinned_only` — bounded by +/// `(capacity + concurrency) * ENTRY_BYTES`, never growing with how many +/// total pin/evict races happened (volume), and in particular strictly +/// above the `resident`-only bound whenever a pin genuinely outlives its +/// entry's eviction, proving pinned bytes are tracked at all. +#[test] +#[cfg_attr( + feature = "governor-rss-negative-control-leak", + ignore = "GOV-RSS03 is the positive-path pin/evict scenario, not the negative control" +)] +fn gov_rss03_pinned_reader_residency_tracks_charged_not_only_resident() { + let budget_bytes = (CACHE_CAPACITY_ENTRIES + READER_CONCURRENCY + 8) * ENTRY_BYTES * 4; + let domain = tiny_domain(budget_bytes, CACHE_CAPACITY_ENTRIES + READER_CONCURRENCY + 8); + let governor = Arc::clone(domain.governor()); + let ring = Arc::new(GovernedRing::new(&governor, CACHE_CAPACITY_ENTRIES)); + + // Fill to capacity first. + let mut handles = Vec::with_capacity(CACHE_CAPACITY_ENTRIES); + for i in 0..CACHE_CAPACITY_ENTRIES { + let fresh = ring.insert(i as u8).expect("capacity available while filling"); + handles.push(fresh); + } + let resident_only_bytes = ring.committed_bytes(); + assert_eq!( + resident_only_bytes, + CACHE_CAPACITY_ENTRIES * ENTRY_BYTES, + "GOV-RSS03 setup: a freshly filled ring must be exactly capacity * entry size" + ); + + // Pin every currently-resident entry (simulating READER_CONCURRENCY + // readers each holding one, capped — never all of them, matching the + // fixed concurrency bound) by cloning the `Arc`, then evict all of them + // from the ring itself. + let pinned: Vec<_> = handles.into_iter().take(READER_CONCURRENCY).collect(); + for _ in 0..READER_CONCURRENCY { + // Evict the (now-pinned) front entries by pushing fresh ones past + // capacity — this is a real eviction, not a manual removal. + ring.insert(0xAA) + .expect("capacity available while evicting the pinned entries out"); + } + + // The pinned entries are no longer in the ring (evicted), but each is + // still held here — `charged` must still count them, distinct from + // `resident` (only the ring's own capacity worth). + let charged_with_pins = ring.committed_bytes(); + assert_eq!( + charged_with_pins, + resident_only_bytes + READER_CONCURRENCY * ENTRY_BYTES, + "GOV-RSS03: charged bytes must equal resident (still-in-ring capacity) plus pinned_only \ + (evicted but still Arc-held) — not just the ring's own resident count" + ); + + // Bound check: charged must never exceed the fixed analytical bound + // regardless of how many total inserts happened to get here — this is + // the volume-independence half of GOV-RSS03. + let analytical_bound_bytes = (CACHE_CAPACITY_ENTRIES + READER_CONCURRENCY) * ENTRY_BYTES; + assert!( + charged_with_pins <= analytical_bound_bytes, + "GOV-RSS03: charged bytes {charged_with_pins} exceeds the fixed analytical bound \ + {analytical_bound_bytes} (capacity + concurrency, independent of volume)" + ); + + // Now drop every pin — charged must collapse back to resident-only, + // proving pinned bytes are real, released capacity, not a permanent + // double-count. + drop(pinned); + let after_unpin = ring.committed_bytes(); + assert_eq!( + after_unpin, resident_only_bytes, + "GOV-RSS03: once every pin drops, charged bytes must return to exactly resident-only — a \ + pinned reservation must not outlive its last Arc" + ); +} diff --git a/crates/basemyai/Cargo.toml b/crates/basemyai/Cargo.toml index e77766e..78eb3ee 100644 --- a/crates/basemyai/Cargo.toml +++ b/crates/basemyai/Cargo.toml @@ -173,6 +173,10 @@ path = "tests/storage/format.rs" name = "plaintext_open_forbidden" path = "tests/storage/plaintext_open_forbidden.rs" +[[test]] +name = "governed_production_activation" +path = "tests/storage/governed_production_activation.rs" + [[test]] name = "llm_provision" path = "tests/provision/llm_provision.rs" diff --git a/crates/basemyai/src/storage/mod.rs b/crates/basemyai/src/storage/mod.rs index c75e971..902c80c 100644 --- a/crates/basemyai/src/storage/mod.rs +++ b/crates/basemyai/src/storage/mod.rs @@ -15,7 +15,10 @@ pub mod integrity; mod native_store; pub use basemyai_engine::Argon2idProfile; -pub use native_store::{BMAI_FORMAT_VERSION, NativeExportRows, NativeMemoryStore}; +pub use native_store::{ + BMAI_FORMAT_VERSION, MemoryGovernorConfig, MemoryGovernorDomain, NativeExportRows, NativeMemoryStore, + open_with_key_governed, +}; pub(crate) use native_store::{NativeImportEdge, NativeImportEntity, NativeImportMemory}; use basemyai_core::Metric; diff --git a/crates/basemyai/src/storage/native_store/coordinator.rs b/crates/basemyai/src/storage/native_store/coordinator.rs index da8c028..d6c574a 100644 --- a/crates/basemyai/src/storage/native_store/coordinator.rs +++ b/crates/basemyai/src/storage/native_store/coordinator.rs @@ -872,6 +872,24 @@ struct CoordinatorCore { close_result: Mutex>>, lifecycle_changed: Notify, owner: Mutex>>, + /// Test-only synchronisation seam for the ADR-072 §B shutdown-race + /// regression coverage (`owner_commands`, right before + /// `charge_rotated_memtable`): lets a test pause `owner_loop` at exactly + /// the point between `execute()` returning (rotation already detected) + /// and the new generation's charge being attached, so a concurrent drop + /// of the last external `WriteCoordinator` handle can be placed + /// deterministically inside that window instead of relying on timing + /// luck. `FnOnce`, taken (not just called) so it fires at most once per + /// coordinator and is a no-op zero-cost `None` outside that one test. + #[cfg(any(test, feature = "test-util"))] + rotation_charge_test_hook: Mutex>>, + /// Same seam as `rotation_charge_test_hook`, one call site later — right + /// after `charge_rotated_memtable` has run but before `core_upgraded` is + /// dropped, so a test can inspect state through `core` (still guaranteed + /// alive at that instant) before this intent's processing finishes and + /// any shutdown cascade it may have triggered starts running. + #[cfg(any(test, feature = "test-util"))] + post_charge_test_hook: Mutex>>, } impl Drop for CoordinatorCore { @@ -914,28 +932,32 @@ impl WriteCoordinator { finalization_capacity: usize, finalization_sink: FinalizationSink, ) -> (ReadGateway, Self) { + // `memory_governor: None` means `build()`'s only fallible step (the + // bootstrap memtable reservation) never runs — see `build`'s doc + // comment. An ungoverned coordinator's construction is infallible by + // construction, so unwrapping here is not a shortcut, it is the + // caller-side proof of that fact. Self::build(state, capacity, finalization_capacity, finalization_sink, None) + .expect("build() cannot fail with memory_governor: None") } /// Same topology as [`Self::dormant`], but every write is admitted by /// `memory_governor` first (ADR-072 §A / CB-02) instead of going straight /// to the tokio concurrency semaphore. /// - /// Deliberately unrouted for now: ADR-072's ratification gates forbid - /// activating a real hard cap before §B (the long-lived memtable - /// residency charge) also lands, because a cap without §B would under- - /// count memtable RAM from the very first write. Until then this is the - /// mechanism plus its tests, never the default construction path — - /// `NativeMemoryStore::from_engine` keeps calling [`Self::dormant`]. - #[allow( - dead_code, - reason = "ADR-072 §A mechanism; its production call site lands with §B, per ADR-072's ratification gates" - )] + /// # Errors + /// [`CoordinatorError::AdmissionRefused`] if `memory_governor`'s domain + /// cannot immediately host one full memtable generation of this engine + /// (ADR-072 §B / CB-06 calibration) — a real, expected runtime outcome + /// when several engines share one [`basemyai_engine::memory_governor::MemoryDomain`] + /// and an earlier one already claimed the room this one needs, never a + /// panic (see [`Self::build`]'s doc comment for why this cannot block + /// instead). pub(super) fn dormant_governed( state: NativeInner, capacity: usize, memory_governor: Arc, - ) -> (ReadGateway, Self) { + ) -> Result<(ReadGateway, Self), CoordinatorError> { Self::build( state, capacity, @@ -945,13 +967,23 @@ impl WriteCoordinator { ) } + /// # Errors + /// [`CoordinatorError::AdmissionRefused`] — only reachable when + /// `memory_governor` is `Some` — if the domain cannot immediately host + /// one full memtable generation of this engine (ADR-072 §B / CB-06). This + /// is a real, valid runtime condition, not a configuration bug: several + /// engines are allowed to share one `MemoryDomain` (ADR-071 + /// §"Portée du domaine"), and an earlier engine's own bootstrap charge + /// can legitimately leave too little room for a later one. Never a panic + /// — see the non-blocking rationale below, which still applies verbatim + /// to a shared domain. fn build( mut state: NativeInner, capacity: usize, finalization_capacity: usize, finalization_sink: FinalizationSink, memory_governor: Option>, - ) -> (ReadGateway, Self) { + ) -> Result<(ReadGateway, Self), CoordinatorError> { assert!(capacity > 0, "writer admission capacity must be non-zero"); assert!(finalization_capacity > 0, "finalization capacity must be non-zero"); static NEXT_RUNTIME: AtomicU64 = AtomicU64::new(1); @@ -993,10 +1025,16 @@ impl WriteCoordinator { // encore, aucune écriture n'est en vol, aucun flush ne peut aboutir : // il n'existe aucun agent capable de rendre un octet. Un // `reserve_blocking` ne serait donc pas de la contre-pression mais un - // interblocage inconditionnel. Un refus ici signifie exactement une - // chose — le domaine est trop petit pour héberger une génération de - // memtable de ce moteur — et c'est une erreur de configuration, qu'on - // fait remonter tout de suite plutôt que de la laisser pendre. + // interblocage inconditionnel. + // + // Un refus ici n'est **pas nécessairement** une erreur de + // configuration : un domaine partagé par plusieurs engines (ADR-071 + // §"Portée du domaine") peut voir sa capacité déjà consommée par le + // bootstrap d'un engine ouvert plus tôt sur le même `MemoryDomain` — + // une condition runtime valide, jamais un bug. Faire remonter une + // erreur typée (plutôt que paniquer) laisse l'appelant décider — + // réessayer avec un domaine plus grand, fermer cet engine, etc. — + // au lieu d'avorter tout le process pour un cas de partage légitime. if let Some(governor) = memory_governor.as_ref() { let reservation = governor .try_reserve( @@ -1004,12 +1042,12 @@ impl WriteCoordinator { Some(basemyai_engine::AdmissionClass::Foreground), memtable_reservation_bytes, ) - .unwrap_or_else(|error| { - panic!( - "the memory domain cannot host one memtable generation of this engine \ + .map_err(|error| { + CoordinatorError::AdmissionRefused(format!( + "the memory domain cannot host one more memtable generation \ ({memtable_reservation_bytes} bytes, ADR-072 CB-06 calibration): {error}" - ) - }); + )) + })?; state .engine .active_memtable_charge_handle() @@ -1037,6 +1075,10 @@ impl WriteCoordinator { close_result: Mutex::new(None), lifecycle_changed: Notify::new(), owner: Mutex::new(None), + #[cfg(any(test, feature = "test-util"))] + rotation_charge_test_hook: Mutex::new(None), + #[cfg(any(test, feature = "test-util"))] + post_charge_test_hook: Mutex::new(None), }); let owner_core = Arc::downgrade(&core); let gateway = ReadGateway { @@ -1052,7 +1094,7 @@ impl WriteCoordinator { .lock() .expect("owner handle lock is initialized") .replace(owner); - (gateway, Self { core }) + Ok((gateway, Self { core })) } fn authorize(&self, agent: &str, class: OperationClass) -> Result { @@ -1535,20 +1577,27 @@ fn release_governor_permit( | Err(CoordinatorError::Closed | CoordinatorError::WrongRuntime | CoordinatorError::Unauthorized) => {} // Every durable — or durability-ambiguous — verdict. // - // TODO(ADR-072 §B): once the long-lived memtable-rotation - // reservation exists, transfer/reclassify into it here instead of - // releasing — today there is nowhere to hand these bytes off to, so - // releasing here is the only correct option even though it - // under-counts real memtable residency until §B lands. Tracked by - // ADR-072. + // Release here is correct by design, not a §B-pending compromise: + // the per-write `MemoryPlan` (`acquire_governed_admission`) sets + // `write_buffer_bytes = 0` unconditionally (ADR-072 §A step 4), so + // this permit never carries memtable-residency weight in the first + // place — it only ever covers transient WAL/intent framing bytes, + // which are genuinely done being needed once `execute()` has + // returned regardless of outcome. Memtable residency itself is + // covered by a wholly separate reservation, sized once per + // generation at `seal_memtable` and attached via + // `attach_memtable_charge`/`charge_rotated_memtable` (ADR-072 §B / + // CB-05, `charge_rotated_memtable` below) — that mechanism, not this + // one, is what would need extending if a future outcome ever needed + // to retain bytes past this point. // // `OutcomeUnknown` is deliberately grouped here rather than with the // aborted branch: architecturally one never assumes safe-to-release - // under ambiguity. Release is the pragmatic interim choice only - // because §B has not yet created anything to retain the bytes into — - // not a claim that this branch is compliant. Same for the raw - // `Err(Terminal)` an `execute()` panic produces, whose durability is - // unknown by construction. + // under ambiguity. It is release-correct for the same reason as the + // other durable/ambiguous outcomes above — nothing in this permit's + // scope needs retaining — not because retention was skipped as an + // expedient. Same for the raw `Err(Terminal)` an `execute()` panic + // produces, whose durability is unknown by construction. Ok( CoordinatorOutcome::Committed { .. } | CoordinatorOutcome::DurableReopenRequired { .. } @@ -1557,8 +1606,10 @@ fn release_governor_permit( ) | Err(_) => {} } - // For this increment both branches converge on ordinary RAII release; - // the classification above is what §B will branch on for real. + // Both branches converge on ordinary RAII release: the classification + // above exists to document *why* each outcome releases rather than to + // gate a future retain-vs-release split, since this permit never scopes + // memtable residency (see above). drop(permit); } @@ -1600,16 +1651,25 @@ fn attach_memtable_charge( /// ADR-072 §B : charge la génération de memtable née pendant l'`execute()` qui /// vient de retourner, s'il y en a eu une. /// +/// Prend un `Arc` déjà mis à niveau, pas un `Weak` à mettre à niveau +/// soi-même — voir le commentaire de son site d'appel dans `owner_commands` +/// pour la fenêtre de shutdown que ce choix ferme (ADR-072 +/// §"Post-ratification hardening", point 3 : un `Weak::upgrade` fait ici même +/// pouvait échouer si le dernier handle `WriteCoordinator` droppait *pendant* +/// `execute()`, sautant silencieusement la charge d'une génération pourtant +/// rotée). +/// /// `None` — donc « rien à signaler » — couvre quatre cas volontairement -/// confondus : aucune rotation, coordinateur déjà en cours de destruction -/// (`Weak` mort : plus aucune écriture ne suivra), aucun gouverneur câblé, et -/// le succès. `Some(cause)` est réservé aux vraies pannes de comptabilité. +/// confondus : aucune rotation, coordinateur déjà mort avant même le début du +/// traitement de cette intention (plus aucune écriture ne suivra), aucun +/// gouverneur câblé, et le succès. `Some(cause)` est réservé aux vraies +/// pannes de comptabilité. fn charge_rotated_memtable( - core: &std::sync::Weak, + core: Option<&Arc>, rotated: Option, ) -> Option { let handle = rotated?; - let core = core.upgrade()?; + let core = core?; let governor = core.memory_governor.as_ref()?; attach_memtable_charge(governor, &handle, core.memtable_reservation_bytes).err() } @@ -1686,6 +1746,20 @@ fn owner_commands( // closure, so `execute()` cannot touch it while it holds // `ProductStateCell.state` (ADR-072 CB-03). // + // ADR-072 §B, fenêtre de shutdown fermée (§"Post-ratification + // hardening", point 3) : mis à niveau une seule fois, ici, + // avant tout appel à `execute()` — pas par `charge_rotated_memtable` + // elle-même. Sans ce clone, le dernier handle `WriteCoordinator` + // pouvait être droppé *pendant* `execute()` (qui ne détient + // lui-même qu'un `Weak`), faisant échouer le `Weak::upgrade` + // de `charge_rotated_memtable` après coup et sautant + // silencieusement la charge d'une génération pourtant rotée. + // En le figeant avant `execute()`, si le coordinateur était + // vivant au moment où cette intention a été dépilée, il le + // reste — via ce clone local — jusqu'à ce que la charge ait + // été traitée, quoi qu'il arrive aux handles externes entre + // temps. + let core_upgraded = core.upgrade(); // ADR-072 §B : `execute()` y dépose le handle de la nouvelle // génération de memtable si une rotation a eu lieu pendant son // exécution. Reste `None` si `execute()` panique — auquel cas @@ -1706,13 +1780,44 @@ fn owner_commands( // First thing after `execute()` returns: `NativeInner` is // released, so touching `GovernorState.mutex` is legal again. release_governor_permit(governor_permit, &outcome); + // Test-only synchronisation seam: fires at most once, and + // only when this intent actually rotated — lets a test pause + // `owner_loop` in exactly the window the fix above closes, + // deterministically, instead of racing real thread timing. + // Checked against `rotated_memtable` (a borrow), strictly + // before it is moved into `charge_rotated_memtable` below. + #[cfg(any(test, feature = "test-util"))] + if rotated_memtable.is_some() + && let Some(core) = core_upgraded.as_ref() + && let Ok(mut hook) = core.rotation_charge_test_hook.lock() + && let Some(hook) = hook.take() + { + hook(); + } // ADR-072 §B / CB-05, strictement après la libération du permis // par-write — dont les octets rendus sont précisément ceux dont // la réservation amont peut avoir besoin. La fenêtre est fermée // par la sérialisation d'`owner_loop` : ce thread est le seul à // pouvoir amener une écriture jusqu'à la nouvelle génération, et // il n'a pas encore rebouclé sur `blocking_recv()`. - let charge_failure = charge_rotated_memtable(core, rotated_memtable); + let charge_failure = charge_rotated_memtable(core_upgraded.as_ref(), rotated_memtable); + // Second test-only checkpoint, same rationale as the one + // above: `core_upgraded` is still guaranteed alive here, so a + // test can safely inspect state through it before this + // intent finishes and any shutdown it may trigger starts. + #[cfg(any(test, feature = "test-util"))] + if let Some(core) = core_upgraded.as_ref() + && let Ok(mut hook) = core.post_charge_test_hook.lock() + && let Some(hook) = hook.take() + { + hook(); + } + // The extra strong reference has done its job — release it + // before looping back to `blocking_recv()` so a last external + // handle that dropped during this intent's processing is + // free to finish tearing the coordinator down immediately + // after, exactly as if this clone had never existed. + drop(core_upgraded); let terminal = matches!(&outcome, Err(CoordinatorError::Terminal)) || matches!(&outcome, Ok(value) if value.is_terminal()); let cause = terminal.then(|| match &outcome { @@ -2656,6 +2761,32 @@ mod tests { ) } + /// Like [`encrypted_inner`], but with [`rotating_options`] — the ADR-072 + /// governed structural-rotation test needs a memtable small enough to + /// size a governor budget around, exactly like [`inner_with_options`] + /// does for the unencrypted case. + fn encrypted_inner_with_options(options: basemyai_engine::EngineOptions) -> (tempfile::TempDir, NativeInner) { + let directory = tempfile::tempdir().expect("temporary encrypted engine directory"); + let mut engine = Engine::open_encrypted_with_options(directory.path(), INITIAL_ROTATION_KEY, options) + .expect("encrypted engine opens"); + let vectors = PersistentVectorIndex::open( + &mut engine, + basemyai_engine::VectorIndexParams::with_dim(crate::EMBEDDING_DIM), + ) + .expect("vector index opens"); + let memory = PersistentMemoryIndex::open(&engine).expect("memory index opens"); + ( + directory, + NativeInner { + engine, + vectors, + memory, + graph: PersistentGraph::new(), + fts: PersistentFts::new(), + }, + ) + } + fn structural_intent(writer: &WriteCoordinator, operation: LiveStructural) -> WriteIntent { WriteIntent::LiveStructural { authorization: writer @@ -2734,6 +2865,124 @@ mod tests { .expect("valid governor config") } + // ADR-072's own §"Points ouverts" flags this exact scenario as confirmed + // and deliberately left unfixed: `WriteCoordinator::build` used to + // `panic!` when a shared `MemoryDomain`'s bootstrap reservation for a + // *second* engine found the domain already too full — even though + // sharing one `MemoryDomain` across several engines is an explicitly + // supported, valid runtime configuration (ADR-071 §"Portée du domaine": + // "un produit qui veut une limite commune partage le même + // `Arc`"), not a configuration error. This proves the fix: + // a real second `Engine`, same governor, insufficient remaining budget — + // a typed `CoordinatorError::AdmissionRefused`, never a panic that would + // abort the whole process for what is a completely ordinary admission + // refusal. + #[test] + fn build_returns_a_typed_error_not_a_panic_when_a_shared_domain_is_too_small_for_a_second_engine() { + let (_first_directory, first_state) = inner_with_options(rotating_options()); + let (_second_directory, second_state) = inner_with_options(rotating_options()); + let residency = first_state + .engine + .memtable_reservation_bytes() + .expect("calibration must not overflow"); + assert_eq!( + residency, + second_state + .engine + .memtable_reservation_bytes() + .expect("calibration must not overflow"), + "both engines share the same rotating_options() calibration" + ); + + // Sized to host exactly one engine's bootstrap residency charge and + // nothing more: `budget - residency` leaves only the two 1-byte + // per-class floors, far short of a second `residency`-sized claim. + let domain = governed_domain(residency + 2, 1); + let governor = domain.governor(); + + let (_first_reads, _first_writer) = WriteCoordinator::dormant_governed(first_state, 4, Arc::clone(governor)) + .expect("the first engine on a fresh shared domain must succeed"); + assert_eq!( + governor.committed_bytes(), + residency, + "the first engine's bootstrap charge alone must already be committed" + ); + + let refusal = match WriteCoordinator::dormant_governed(second_state, 4, Arc::clone(governor)) { + Ok(_) => panic!("a second engine must not fit the remaining 2 bytes of this shared domain"), + Err(error) => error, + }; + assert!( + matches!(refusal, CoordinatorError::AdmissionRefused(_)), + "a shared domain refusing a second engine's bootstrap charge is a real, valid runtime \ + outcome (ADR-071 §\"Portée du domaine\") — a typed error, not the process-aborting \ + panic this used to be: got {refusal:?}" + ); + // The failed second attempt must not have left any partial charge + // behind — `build()` returns before ever calling `attach` when + // `try_reserve` itself fails. + assert_eq!( + governor.committed_bytes(), + residency, + "a refused bootstrap reservation must not move the governor's accounting at all" + ); + } + + /// The companion positive case: the same two real engines, the same + /// shared governor, but a domain sized to actually host both — proving + /// the fix is a correct typed error on refusal, not a regression that + /// now refuses a legitimately-sized shared domain too. + #[tokio::test] + async fn two_real_engines_can_share_one_memory_domain_when_it_is_sized_for_both() { + let (_first_directory, first_state) = inner_with_options(rotating_options()); + let (_second_directory, second_state) = inner_with_options(rotating_options()); + let residency = first_state + .engine + .memtable_reservation_bytes() + .expect("calibration must not overflow"); + + // `per_class_bytes` must clear a real write's `MemoryPlan::peak_bytes` + // (WAL framing overhead alone is already 35 bytes unencrypted) — + // unlike the refusal test above, this domain also has to admit real + // per-write traffic through `submit()`, not just the two bootstrap + // residency charges. + const PER_CLASS_BYTES: usize = 4096; + let domain = governed_domain(2 * residency + 2 * PER_CLASS_BYTES, PER_CLASS_BYTES); + let governor = domain.governor(); + + let (_first_reads, first_writer) = WriteCoordinator::dormant_governed(first_state, 4, Arc::clone(governor)) + .expect("first engine fits the shared domain"); + let (_second_reads, second_writer) = WriteCoordinator::dormant_governed(second_state, 4, Arc::clone(governor)) + .expect("second engine also fits the shared domain once it is sized for both"); + assert_eq!( + governor.committed_bytes(), + 2 * residency, + "both engines' bootstrap residency charges must be committed simultaneously" + ); + + // Both coordinators must be genuinely independently functional on + // the shared domain, not just constructible. + let log = Arc::new(Mutex::new(Vec::new())); + first_writer + .submit(record(1, &log)) + .await + .expect("first coordinator admits a write") + .outcome() + .await + .expect("first coordinator's write completes"); + second_writer + .submit(record(2, &log)) + .await + .expect("second coordinator admits a write") + .outcome() + .await + .expect("second coordinator's write completes"); + assert_eq!(*log.lock().expect("test log"), vec![1, 2]); + + first_writer.close().await.expect("first coordinator closes cleanly"); + second_writer.close().await.expect("second coordinator closes cleanly"); + } + fn parked_record( marker: u64, log: &Arc>>, @@ -2981,7 +3230,8 @@ mod tests { .expect("calibration must not overflow"); let domain = governed_domain(64 * 1024 * 1024, 16 * 1024 * 1024); let governor = Arc::clone(domain.governor()); - let (_reads, writer) = WriteCoordinator::dormant_governed(state, 4, Arc::clone(&governor)); + let (_reads, writer) = WriteCoordinator::dormant_governed(state, 4, Arc::clone(&governor)) + .expect("test governor domain sized to host the bootstrap memtable reservation"); // ADR-072 §B moved this floor off zero: a governed coordinator always // carries its live memtable generation's residency charge, from // construction onward. Every per-write assertion below is stated @@ -3035,7 +3285,8 @@ mod tests { // budget, so the two can be set apart like this. let domain = governed_domain(64 * 1024 * 1024, 1); let governor = Arc::clone(domain.governor()); - let (_reads, writer) = WriteCoordinator::dormant_governed(state, 4, Arc::clone(&governor)); + let (_reads, writer) = WriteCoordinator::dormant_governed(state, 4, Arc::clone(&governor)) + .expect("test governor domain sized to host the bootstrap memtable reservation"); let log = Arc::new(Mutex::new(Vec::new())); assert!(matches!( @@ -3086,7 +3337,8 @@ mod tests { // already eats into `budget` too. let domain = governed_domain(budget, 900_000); let governor = Arc::clone(domain.governor()); - let (_reads, writer) = WriteCoordinator::dormant_governed(state, 4, Arc::clone(&governor)); + let (_reads, writer) = WriteCoordinator::dormant_governed(state, 4, Arc::clone(&governor)) + .expect("test governor domain sized to host the bootstrap memtable reservation"); // Hog every free byte beyond the bootstrap memtable charge but one — // `try_reserve` has no per-request ceiling (unlike admission), and @@ -3160,7 +3412,8 @@ mod tests { .expect("calibration must not overflow"); let domain = governed_domain(64 * 1024 * 1024, 16 * 1024 * 1024); let governor = Arc::clone(domain.governor()); - let (_reads, writer) = WriteCoordinator::dormant_governed(state, 2, Arc::clone(&governor)); + let (_reads, writer) = WriteCoordinator::dormant_governed(state, 2, Arc::clone(&governor)) + .expect("test governor domain sized to host the bootstrap memtable reservation"); let log = Arc::new(Mutex::new(Vec::new())); let (intent, entered_rx, release_tx) = parked_record(1, &log, true); @@ -3193,6 +3446,351 @@ mod tests { ); } + // ADR-072 §A/CB-T1d: the outcome table (§A) prescribes RELEASE for + // `Aborted` (WAL never durably written), and this is the one branch a + // stale purge epoch can trigger with no failpoint at all — the check + // (`current_epoch != expected_epoch`) runs in plain Rust before any WAL + // work starts. `agent_epochs` has no entry for "agent-a" on a fresh + // coordinator, so the current epoch defaults to 0 and any other + // `Some(epoch)` is rejected immediately (`coordinator.rs`'s + // `WriteIntent::MemoryPurgeChunk` arm). + #[tokio::test] + async fn governed_aborted_write_releases_its_admission_immediately() { + let (_directory, state) = inner_with_options(rotating_options()); + let residency = state + .engine + .memtable_reservation_bytes() + .expect("calibration must not overflow"); + let domain = governed_domain(64 * 1024 * 1024, 16 * 1024 * 1024); + let governor = Arc::clone(domain.governor()); + let (_reads, writer) = WriteCoordinator::dormant_governed(state, 4, Arc::clone(&governor)) + .expect("test governor domain sized to host the bootstrap memtable reservation"); + assert_eq!( + governor.committed_bytes(), + residency, + "only the live generation's residency is committed before any write" + ); + + let outcome = writer + .submit_outcome(WriteIntent::MemoryPurgeChunk { + authorization: writer + .authorize("agent-a", OperationClass::RepairOrRebuildOrGc) + .expect("purge auth"), + agent: "agent-a".into(), + cursor: None, + expected_epoch: Some(1), + }) + .await + .expect("stale-epoch outcome delivered"); + assert!( + matches!(outcome, CoordinatorOutcome::Aborted { .. }), + "a stale purge epoch must be rejected before WAL, not committed: {outcome:?}" + ); + assert_eq!( + governor.committed_bytes(), + residency, + "an Aborted write must release its per-write reservation back to the residency floor" + ); + assert_eq!(governor.outstanding_slots(), 0); + writer.close().await.expect("an aborted write is not terminal"); + } + + // ADR-072 §A/CB-T1d: `OutcomeUnknown` (the WAL outcome itself is unknown + // — crash at the append/sync boundary) is durability-ambiguous, and the + // outcome table (§A) prescribes RELEASE for it too — the only reservation + // that could receive these bytes (the §B memtable-rotation reservation) + // does not exist yet (see `release_governor_permit`'s own doc comment). + // Reuses `import_unknown_preserves_phase_and_terminalizes`'s failpoint + // technique, under a governed coordinator this time. + #[tokio::test] + async fn governed_outcome_unknown_write_releases_its_admission() { + let _serial = failpoint_lock().await; + failpoint::clear_all(); + let (_directory, state) = inner_with_options(rotating_options()); + let residency = state + .engine + .memtable_reservation_bytes() + .expect("calibration must not overflow"); + let domain = governed_domain(64 * 1024 * 1024, 16 * 1024 * 1024); + let governor = Arc::clone(domain.governor()); + let (_reads, writer) = WriteCoordinator::dormant_governed(state, 2, Arc::clone(&governor)) + .expect("test governor domain sized to host the bootstrap memtable reservation"); + + failpoint::set("after_wal_append", Action::Error); + let outcome = writer + .import_graph_entity_batch( + "agent-a".into(), + vec![OwnedGraphEntity { + id: "alice".into(), + entity: GraphEntity { + kind: "person".into(), + label: "Alice".into(), + valid_from: 0, + valid_until: None, + source: GraphSource::Import, + }, + }], + ) + .await + .expect("phase-aware import outcome"); + failpoint::clear_all(); + + assert!(matches!( + outcome, + CoordinatorOutcome::OutcomeUnknown { + phase: WalCommitPhase::Append, + .. + } + )); + // `<=`, not `==`, for the same reason as + // `a_governed_handler_panic_still_returns_the_admission`: release is + // synchronous before the caller is answered (deterministic absence), + // but the terminal shutdown this outcome triggers may already be + // racing this line to drop the engine and its residency charge with + // it. + assert!( + governor.committed_bytes() <= residency, + "an OutcomeUnknown write must never leave a phantom per-write reservation" + ); + assert_eq!(writer.close().await, Err(CoordinatorError::Terminal)); + assert_eq!( + governor.committed_bytes(), + 0, + "the residency charge must die with the engine after terminal shutdown" + ); + } + + // ADR-072 §A/CB-T1d: `StructuralReopenRequired` (durable structural + // publish confirmed, RAM mismatch) is the last of the four + // durable-or-ambiguous branches the outcome table (§A) prescribes RELEASE + // for. Reuses `structural_unknown_maps_phase_retains_candidate_and_shuts_ + // reads`'s failpoint technique, under a governed coordinator. + #[tokio::test] + async fn governed_structural_reopen_required_releases_its_admission() { + let _serial = failpoint_lock().await; + failpoint::clear_all(); + let (_directory, state) = encrypted_inner_with_options(rotating_options()); + let residency = state + .engine + .memtable_reservation_bytes() + .expect("calibration must not overflow"); + let domain = governed_domain(64 * 1024 * 1024, 16 * 1024 * 1024); + let governor = Arc::clone(domain.governor()); + let (reads, writer) = WriteCoordinator::dormant_governed(state, 2, Arc::clone(&governor)) + .expect("test governor domain sized to host the bootstrap memtable reservation"); + + failpoint::set("during_generation_directory_sync", Action::Error); + let outcome = writer + .submit(structural_intent(&writer, LiveStructural::KeyFull { secret: secret() })) + .await + .expect("full rotation admitted") + .outcome() + .await + .expect("structural terminal outcome delivered"); + failpoint::clear_all(); + + assert!(matches!( + outcome, + CoordinatorOutcome::StructuralReopenRequired { + phase: StructuralCommitPhase::DirectorySync, + candidate_ownership: StructuralCandidateOwnership::RetainForRecovery, + .. + } + )); + assert!( + governor.committed_bytes() <= residency, + "a StructuralReopenRequired write must never leave a phantom per-write reservation" + ); + assert_eq!(reads.read_sync(|_| ()), Err(CoordinatorError::Terminal)); + assert_eq!(writer.close().await, Err(CoordinatorError::Terminal)); + assert_eq!( + governor.committed_bytes(), + 0, + "the residency charge must die with the engine after terminal shutdown" + ); + } + + // ADR-072 §A/CB-T1d: `DurableReopenRequired` (fsync confirmed, install + // failed) is the one `CoordinatorOutcome` branch this file cannot reach + // end-to-end. Its only two real producers both require engine state to + // change *between* staging a commit and installing it: + // - `install_committed_batch_with`'s own identity/pending-install + // mismatch (`basemyai-engine/src/store/engine/write.rs:200-230`); + // - `StagedMemoryPut`/`StagedForgetChunk`'s `expected_next_vec_id` + // mismatch (`basemyai-engine/src/idx/memory/persistent.rs:271-289`). + // `execute()` holds `NativeInner` synchronously for its whole duration + // with no `.await`/yield point, and `owner_loop` serializes every + // `execute()` call strictly one at a time — no other write can ever + // interleave between staging and install of the same commit. Confirmed + // by grep: neither producer has a test anywhere in this workspace today. + // Per the task's own review guidance this uses direct `CoordinatorOutcome` + // construction against a *real* governed `WriteCoordinator`/ + // `AdmissionPermit` (not a mock) — it exercises `release_governor_permit`'s + // actual classification (CB-04), just without a naturally-reachable + // end-to-end trigger. The `SequenceRange` is minted by a real preceding + // commit rather than invented, since the type has no public constructor + // outside `basemyai-engine` — its value is opaque to + // `release_governor_permit` anyway, which classifies on the outer + // `CoordinatorOutcome` variant alone. + #[tokio::test] + async fn governed_durable_reopen_required_releases_its_admission() { + let (_directory, state) = inner_with_options(rotating_options()); + let residency = state + .engine + .memtable_reservation_bytes() + .expect("calibration must not overflow"); + let domain = governed_domain(64 * 1024 * 1024, 16 * 1024 * 1024); + let governor = Arc::clone(domain.governor()); + let (_reads, writer) = WriteCoordinator::dormant_governed(state, 4, Arc::clone(&governor)) + .expect("test governor domain sized to host the bootstrap memtable reservation"); + + let seeded = writer + .graph_entity_batch("agent-a".into(), vec![purge_entity(1)]) + .await + .expect("seed write commits"); + let CoordinatorOutcome::Committed { + sequence_range: Some(sequence_range), + .. + } = seeded + else { + panic!("seed write must be a durable commit with a real sequence range: {seeded:?}"); + }; + assert_eq!( + governor.committed_bytes(), + residency, + "the seed write's own admission must already be back at the residency floor" + ); + + let permit = writer + .acquire_governed_admission(1, 4_096) + .await + .expect("a small plan must be granted") + .expect("a governed coordinator must hand back a real permit"); + assert!( + governor.committed_bytes() > residency, + "the freshly acquired permit must commit bytes on top of the residency floor" + ); + + let outcome: Result = Ok(CoordinatorOutcome::DurableReopenRequired { + request: WriteRequestId { + runtime: writer.core.runtime, + ordinal: 999, + }, + sequence_range, + cause: "synthetic DurableReopenRequired for CB-T1d coverage".to_owned(), + }); + release_governor_permit(Some(permit), &outcome); + + assert_eq!( + governor.committed_bytes(), + residency, + "CB-04: DurableReopenRequired must release the per-write reservation back to the \ + residency floor, matching every other durable-or-ambiguous branch's current release \ + behavior (see `release_governor_permit`'s doc comment on the pending §B hand-off)" + ); + writer.close().await.expect("clean close"); + } + + // ADR-072 §A/CB-T2: step 5 (governor admission) must precede step 6 + // (`core.permits` semaphore) not merely for an *immediate* refusal + // (`a_write_over_the_governor_ceiling_is_refused_before_the_concurrency_ + // semaphore` already proves that) but under real, sustained contention — + // a caller genuinely blocked on the governor's `Condvar` must not be + // holding a semaphore slot while it waits, or it would starve every + // other writer of a resource that has nothing to do with byte capacity. + // + // The governor enforces *strict* intra-class FIFO (`admission.rs`: "never + // jumps ahead of an already-parked blocking waiter"), and every product + // write uses the same `AdmissionClass::Foreground` — so once one caller + // is parked on a saturated class, no other same-class write can be + // admitted past it either, by design. That rules out proving this by + // running other coordinator writes to completion alongside the parked + // one: they would queue behind it and never observe anything. What *can* + // be proven directly is the real invariant CB-T2 is actually about: does + // the parked call hold a `core.permits` slot? `core.permits` is a real + // `Arc`, the exact same object `submit()` acquires from — so + // acquiring every one of its slots directly, from this test, while the + // call is genuinely parked on the real governor, is a direct proof, not + // a mock: if the parked call held even one slot, one of these + // acquisitions would fail. + #[tokio::test] + async fn a_write_parked_on_governor_byte_capacity_never_holds_a_concurrency_permit() { + let (_directory, state) = inner_with_options(rotating_options()); + let budget = state + .engine + .memtable_reservation_bytes() + .expect("calibration must not overflow") + + 2_000_000; + let domain = governed_domain(budget, 900_000); + let governor = Arc::clone(domain.governor()); + let concurrency = 4; + let (_reads, writer) = WriteCoordinator::dormant_governed(state, concurrency, Arc::clone(&governor)) + .expect("test governor domain sized to host the bootstrap memtable reservation"); + + // Hog every free byte beyond the bootstrap memtable charge but one — + // same technique as + // `closing_a_governed_coordinator_wakes_a_caller_parked_on_byte_ + // capacity`: no real write's plan can ever fit one byte, so the next + // submitted write is guaranteed to park on the governor's `Condvar`. + let free_before = budget - governor.committed_bytes(); + let hog = governor + .try_reserve( + basemyai_engine::GovernedMemoryKind::FlushWorkingSet, + None, + free_before - 1, + ) + .expect("hogging free budget with no queued waiters to protect must succeed"); + + let log = Arc::new(Mutex::new(Vec::new())); + let intent = record(1, &log); + let parked_writer = writer.clone(); + let mut parked = tokio::spawn(async move { parked_writer.submit(intent).await }); + + let deadline = Instant::now() + Duration::from_secs(10); + while governor.admission_waiters() < 1 { + assert!(Instant::now() < deadline, "the write must park on byte capacity"); + tokio::task::yield_now().await; + } + + // The counter-level check first: + assert_eq!( + writer.core.permits.available_permits(), + concurrency, + "a caller parked on governor byte capacity must not hold a core.permits slot" + ); + // ... and the operational one: actually acquire every slot the + // semaphore has, concurrently with the still-parked governor call. + let mut held = Vec::with_capacity(concurrency); + for n in 0..concurrency { + held.push( + Arc::clone(&writer.core.permits) + .try_acquire_owned() + .unwrap_or_else(|error| { + panic!( + "slot {n} of {concurrency} must be free while the governor caller is \ + parked, not held by it: {error}" + ) + }), + ); + } + assert_eq!( + governor.admission_waiters(), + 1, + "the original write must still be parked while every semaphore slot was acquired" + ); + + drop(held); + drop(hog); + let ticket = tokio::time::timeout(Duration::from_secs(10), &mut parked) + .await + .expect("freeing the hogged budget must unpark the write, not hang") + .expect("the spawned task itself must not panic or be cancelled") + .expect("the parked write is admitted once budget frees"); + ticket.outcome().await.expect("parked write commits"); + assert_eq!(*log.lock().expect("test log"), vec![1]); + writer.close().await.expect("clean close"); + } + // ADR-072 §B / CB-05, bootstrap arm: `Engine::open` mints a first memtable // that *no* rotation ever produces, so `execute()`'s before/after // comparison can never see it. It must therefore be charged at @@ -3213,7 +3811,8 @@ mod tests { let domain = governed_domain(reservation * 6, reservation * 2); let governor = Arc::clone(domain.governor()); - let (reads, writer) = WriteCoordinator::dormant_governed(state, 4, Arc::clone(&governor)); + let (reads, writer) = WriteCoordinator::dormant_governed(state, 4, Arc::clone(&governor)) + .expect("test governor domain sized to host the bootstrap memtable reservation"); assert_eq!( governor.committed_bytes(), @@ -3258,7 +3857,8 @@ mod tests { .expect("calibration must not overflow"); let domain = governed_domain(reservation * 6, reservation * 2); let governor = Arc::clone(domain.governor()); - let (reads, writer) = WriteCoordinator::dormant_governed(state, 4, Arc::clone(&governor)); + let (reads, writer) = WriteCoordinator::dormant_governed(state, 4, Arc::clone(&governor)) + .expect("test governor domain sized to host the bootstrap memtable reservation"); // Held across the rotation on purpose: it keeps naming the *retired* // generation, so CB-07's release can be observed on that generation @@ -3344,6 +3944,131 @@ mod tests { writer.close().await.expect("clean close"); } + // ADR-072 §"Post-ratification hardening", point 3 — the shutdown race + // that survived the previous review, confirmed real but left + // uncharacterized: "Si toutes les poignées WriteCoordinator sont + // droppées pendant qu'owner_loop est en plein execute() d'une écriture + // qui déclenche une rotation, core.upgrade() dans charge_rotated_memtable + // peut retourner None et la nouvelle génération n'est alors jamais + // chargée." Before the `core_upgraded` fix in `owner_commands` + // (captured once, before `execute()` runs, instead of re-upgraded + // inside `charge_rotated_memtable` after the fact), this window could + // silently skip a rotated generation's governed charge — no error, no + // panic, just a governor that quietly stopped matching reality. + // + // Reproduced by construction, not timing luck: `rotation_charge_test_hook` + // pauses `owner_loop` deterministically in exactly this window (right + // after `execute()` returns with a detected rotation, right before + // `charge_rotated_memtable` runs), so the last `WriteCoordinator` handle + // can be dropped precisely inside it every single run. + #[tokio::test] + async fn last_write_coordinator_handle_dropped_right_after_a_rotating_write_still_charges_the_new_generation() { + let (_directory, state) = inner_with_options(basemyai_engine::EngineOptions { + memtable_flush_threshold: 1, + ..rotating_options() + }); + let reservation = state + .engine + .memtable_reservation_bytes() + .expect("calibration must not overflow"); + let domain = governed_domain(reservation * 6, reservation * 2); + let governor = Arc::clone(domain.governor()); + let (reads, writer) = WriteCoordinator::dormant_governed(state, 4, Arc::clone(&governor)) + .expect("test governor domain sized to host the bootstrap memtable reservation"); + + let first = reads + .read_sync(|state| state.engine.active_memtable_charge_handle()) + .expect("read the bootstrap generation"); + + // Checkpoint 1, before the charge: the test drops the last external + // handle here. + let (reached1_tx, reached1_rx) = std::sync::mpsc::channel::<()>(); + let (proceed1_tx, proceed1_rx) = std::sync::mpsc::channel::<()>(); + writer + .core + .rotation_charge_test_hook + .lock() + .expect("hook lock") + .replace(Box::new(move || { + reached1_tx + .send(()) + .expect("signal the test thread that execute() already returned"); + proceed1_rx + .recv() + .expect("wait for the test thread to drop the last handle and release this pause"); + })); + // Checkpoint 2, right after the charge: `core` (and therefore the + // engine `reads` reaches through it) is still guaranteed alive here + // — the test inspects state before this intent's own completion can + // trigger a shutdown cascade that would otherwise race the read. + let (reached2_tx, reached2_rx) = std::sync::mpsc::channel::<()>(); + let (proceed2_tx, proceed2_rx) = std::sync::mpsc::channel::<()>(); + writer + .core + .post_charge_test_hook + .lock() + .expect("hook lock") + .replace(Box::new(move || { + reached2_tx + .send(()) + .expect("signal the test thread that the charge has been attached"); + proceed2_rx + .recv() + .expect("wait for the test thread's inspection to finish"); + })); + + // `memtable_flush_threshold: 1` guarantees this single write rotates + // the bootstrap generation within its own `execute()`. `submit()` + // alone never blocks on either hook — only the eventual `outcome()` + // does — so this returns with the write already safely enqueued, + // before anything pauses. + let ticket = writer + .submit(graph_intent(&writer, "shutdown-race")) + .await + .expect("the rotating write is admitted"); + + // Blocks on a std channel recv, so this must run off the tokio + // worker thread. + tokio::task::spawn_blocking(move || reached1_rx.recv()) + .await + .expect("join the blocking wait") + .expect("the owner thread must have reached the first hook for this rotating write"); + + // The critical moment: the *last* external `WriteCoordinator` handle + // drops right here — strictly after `execute()` returned (the hook + // only fires post-`execute()`, with a rotation already detected) and + // strictly before `charge_rotated_memtable` runs (the owner thread is + // parked mid-hook, unable to reach it until `proceed1_tx` fires). + drop(writer); + proceed1_tx + .send(()) + .expect("let the owner thread continue into charge_rotated_memtable"); + + tokio::task::spawn_blocking(move || reached2_rx.recv()) + .await + .expect("join the blocking wait") + .expect("the owner thread must have reached the second hook"); + + // The charge has now been attached (or not, if the fix regressed) — + // and `core` is still guaranteed alive, since the owner thread has + // not yet dropped `core_upgraded`. Safe, race-free inspection. + let live = reads + .read_sync(|state| state.engine.active_memtable_charge_handle()) + .expect("the coordinator must still be fully open at this checkpoint"); + assert!( + !live.same_memtable_as(&first), + "the memtable must have rotated for this assertion to mean anything" + ); + assert_eq!( + live.charged_bytes(), + Some(reservation), + "the last WriteCoordinator handle dropping strictly between execute() returning and \ + charge_rotated_memtable running must not skip the rotated generation's charge" + ); + proceed2_tx.send(()).expect("let the owner thread finish this intent"); + ticket.outcome().await.expect("the rotating write still commits"); + } + #[tokio::test] async fn legacy_capability_rejects_a_different_agent_before_store_access() { let (_directory, state) = inner(); diff --git a/crates/basemyai/src/storage/native_store/mod.rs b/crates/basemyai/src/storage/native_store/mod.rs index c002fe9..d51cd96 100644 --- a/crates/basemyai/src/storage/native_store/mod.rs +++ b/crates/basemyai/src/storage/native_store/mod.rs @@ -140,6 +140,90 @@ fn ensure_container_meta(engine: &mut Engine) -> Result<()> { /// `k` — politique ADR-012 (filtre agent+validité toujours présent). const OVERSAMPLE: usize = 8; +/// Configuration for a [`MemoryGovernorDomain`] (ADR-071 §"Configuration" / +/// ADR-072) — the hard byte budget and two-class (foreground/maintenance) +/// fairness shape a governed [`NativeMemoryStore`] is admitted against. +/// Plain data, owned by `basemyai` itself: unlike +/// `basemyai_engine::memory_governor::GovernorConfig`, which it mirrors +/// field-for-field, this type never appears next to an engine type in a +/// `basemyai` public signature (ADR-068 §10 — no new public API may name a +/// `basemyai_engine::*` type without its own transition ADR; +/// [`MemoryGovernorDomain`] is that transition for this one mechanism). +#[derive(Debug, Clone)] +pub struct MemoryGovernorConfig { + /// Hard cap of the domain. + pub budget_bytes: usize, + /// Total admitted slots across both classes. + pub max_queue_depth: usize, + /// Bound on parked-waiter bookkeeping (distinct from `max_queue_depth`: + /// this bounds parked threads, not granted permits). + pub max_admission_waiters: usize, + pub reserved_foreground_slots: usize, + pub reserved_maintenance_slots: usize, + pub reserved_foreground_bytes: usize, + pub reserved_maintenance_bytes: usize, + /// Integer `> 0`. + pub foreground_weight: u32, + /// Integer `> 0`. + pub maintenance_weight: u32, + pub max_foreground_intent_peak_bytes: usize, + pub max_maintenance_intent_peak_bytes: usize, + /// Fixed governor bookkeeping, committed once at construction. + pub governor_overhead_bytes: usize, + /// Worst-case minimal-flush working set, committed once at construction + /// so a full domain can still make flush progress (ADR-071 + /// §"Headroom de progrès background") — + /// `basemyai_engine::memory_governor::estimate_emergency_flush_headroom_bytes` + /// derives this number from an engine's real calibration. + pub emergency_flush_headroom_bytes: usize, +} + +/// A shareable governed memory domain (ADR-071 §"Portée du domaine") a +/// [`NativeMemoryStore`] can be opened against via +/// [`open_with_key_governed`] — the production activation path for the +/// memory governor. Several stores may share one domain: an operator +/// wanting one common limit across several engines shares the same +/// `MemoryGovernorDomain`. +/// +/// Owned by `basemyai` itself, wrapping the engine's real governor behind a +/// type that never names it in a public signature (ADR-068 §10 — see +/// [`MemoryGovernorConfig`]'s doc comment). +pub struct MemoryGovernorDomain { + governor: Arc, +} + +impl MemoryGovernorDomain { + /// # Errors + /// A storage error naming the first violated configuration relation + /// (ADR-071 §"Configuration") if `config` is invalid — e.g. a zero + /// budget, reserved bytes exceeding the budget, or a `u128` overflow + /// while deriving the DRR scheduler's quanta. + pub fn new(config: MemoryGovernorConfig) -> Result { + let domain = basemyai_engine::MemoryDomain::new(basemyai_engine::GovernorConfig { + budget_bytes: config.budget_bytes, + max_queue_depth: config.max_queue_depth, + max_admission_waiters: config.max_admission_waiters, + reserved_slots: [config.reserved_foreground_slots, config.reserved_maintenance_slots], + reserved_bytes: [config.reserved_foreground_bytes, config.reserved_maintenance_bytes], + weight: [config.foreground_weight, config.maintenance_weight], + max_intent_peak_bytes: [ + config.max_foreground_intent_peak_bytes, + config.max_maintenance_intent_peak_bytes, + ], + governor_overhead_bytes: config.governor_overhead_bytes, + emergency_flush_headroom_bytes: config.emergency_flush_headroom_bytes, + }) + .map_err(map_engine_error)?; + Ok(Self { + governor: Arc::clone(domain.governor()), + }) + } + + pub(crate) fn governor(&self) -> &Arc { + &self.governor + } +} + /// Moteur de stockage natif — ADR-024/ADR-027/ADR-033 (unique implémentation `MemoryStore`). /// /// Concurrence (N5.5, barre hardening M6) : `inner` est un `RwLock`, pas un @@ -225,6 +309,57 @@ fn record_valid_at(record: &basemyai_engine::MemoryRecord, now: i64) -> bool { record.valid_from <= now && record.valid_until.is_none_or(|until| until > now) } +/// Like [`NativeMemoryStore::open_with_key`], but every write is admitted +/// through `domain`'s governor first (ADR-071/ADR-072) instead of leaving +/// the coordinator dormant — the production activation entry point for the +/// memory governor. `domain` may be shared with other stores +/// ([`MemoryGovernorDomain`]'s own "Portée du domaine" contract); this +/// store's bootstrap memtable reservation may then be legitimately refused +/// if an earlier sibling already claimed the room it needs (see the +/// `Errors` section) — a typed error, never a panic +/// ([`coordinator::WriteCoordinator::build`]'s own fix, this same task). +/// +/// A **free function**, deliberately not `NativeMemoryStore::open_with_key_governed`: +/// `xtask`'s `PRODUCT_API_OWNERS` guard freezes `NativeMemoryStore`'s own +/// (and `MemoryStore`'s/`Memory`'s) inherent/trait method surface until the +/// V2-03 WriteCoordinator cutover lands (`xtask/v2-03-cutover-complete`, +/// absent today) — unrelated to ADR-068 §10's engine-leak concern, which +/// [`MemoryGovernorDomain`] already satisfies on its own. Adding a method to +/// a type whose construction protocol V2-03 is expected to reshape would +/// just be throwaway surface; a free function returning `NativeMemoryStore` +/// sidesteps that without touching the frozen set at all. +/// +/// `domain` is [`MemoryGovernorDomain`], a `basemyai`-owned type, not +/// `basemyai_engine::MemoryDomain` directly — ADR-068 §10 freezes +/// `basemyai`'s public API against naming any new `basemyai_engine::*` +/// type, so this entry point stays compliant by construction: the engine's +/// real governor is wrapped, never named here. +/// +/// This does not change what any *existing* caller of +/// [`NativeMemoryStore::open_with_key`]/[`NativeMemoryStore::open_encrypted`]/etc. +/// gets — those remain fully ungoverned, unchanged. Activation is opt-in, +/// per store. +/// +/// # Errors +/// Everything [`NativeMemoryStore::open_with_key`] can fail with, plus a +/// storage error naming the governor's refusal if `domain` cannot +/// immediately host this store's calibrated worst-case memtable-generation +/// charge (ADR-072 §B / CB-06) — never a panic, including when `domain` is +/// shared and a sibling already consumed the room this store needed. +pub fn open_with_key_governed( + path: impl AsRef, + key: &basemyai_core::EncryptionKey, + domain: &MemoryGovernorDomain, +) -> Result { + let engine = match key.mode() { + basemyai_core::EncryptionKeyMode::RawKey => Engine::open_encrypted(&path, key.expose().as_bytes()), + basemyai_core::EncryptionKeyMode::Passphrase => Engine::open_with_passphrase(&path, key.expose().as_bytes()), + _ => return Err(storage("unsupported encryption key mode")), + } + .map_err(map_engine_error)?; + NativeMemoryStore::from_engine_governed(engine, Arc::clone(domain.governor())) +} + impl NativeMemoryStore { /// Ouvre (en le créant au besoin) un store natif **en clair** dans le /// répertoire `path` — **réservé aux tests** (`test-util`). @@ -277,6 +412,39 @@ impl NativeMemoryStore { } } + /// Like [`open_with_key_governed`] (a free function — see its own + /// doc comment for why), with explicit `EngineOptions` — same rationale + /// as [`Self::open_with_engine_options`]: without this, exercising + /// rotation and flush under a governed domain would need the ~16.6 GiB + /// default-calibrated memtable reservation + /// (`docs/adr/ADR-072-...md` §B) before either could ever be observed. + /// `pub(crate)`, test-only: no production need for custom + /// `EngineOptions` has come up yet, and every consumer that does need + /// one is in-crate (`mod tests` below). + /// + /// # Errors + /// Same as [`open_with_key_governed`]. + #[cfg(any(test, feature = "test-util"))] + #[allow(dead_code, reason = "awaiting its own ADR-068 transition ADR before public exposure")] + pub(crate) fn open_with_engine_options_governed( + path: impl AsRef, + key: &basemyai_core::EncryptionKey, + options: basemyai_engine::EngineOptions, + domain: &basemyai_engine::MemoryDomain, + ) -> Result { + let engine = match key.mode() { + basemyai_core::EncryptionKeyMode::RawKey => { + Engine::open_encrypted_with_options(&path, key.expose().as_bytes(), options) + } + basemyai_core::EncryptionKeyMode::Passphrase => { + Engine::open_with_passphrase_and_options(&path, key.expose().as_bytes(), options) + } + _ => return Err(storage("unsupported encryption key mode")), + } + .map_err(map_engine_error)?; + Self::from_engine_governed(engine, Arc::clone(domain.governor())) + } + /// Ouvre (en le créant au besoin) un store natif chiffré avec une /// passphrase humaine, étirée avec Argon2id et persistée comme telle dans /// `CryptoMeta:2` (ADR-042). @@ -306,7 +474,45 @@ impl NativeMemoryStore { ) } - fn from_engine(mut engine: Engine) -> Result { + fn from_engine(engine: Engine) -> Result { + let state = Self::open_indexes(engine)?; + let (reads, writer) = coordinator::WriteCoordinator::dormant(state, 256); + Ok(Self { + reads, + writer, + #[cfg(any(test, feature = "test-util"))] + _tempdir: None, + }) + } + + /// Same index bootstrap as [`Self::from_engine`], but the coordinator is + /// built governed against `memory_governor` ([`open_with_key_governed`]). + fn from_engine_governed( + engine: Engine, + memory_governor: Arc, + ) -> Result { + let state = Self::open_indexes(engine)?; + let (reads, writer) = coordinator::WriteCoordinator::dormant_governed(state, 256, memory_governor).map_err( + |error| match error { + coordinator::CoordinatorError::AdmissionRefused(cause) => storage(format!( + "memory governor domain refused this store's bootstrap charge: {cause}" + )), + other => storage(format!("failed to construct a governed coordinator: {other:?}")), + }, + )?; + Ok(Self { + reads, + writer, + #[cfg(any(test, feature = "test-util"))] + _tempdir: None, + }) + } + + /// Opens the vector/memory indexes and container metadata a fresh + /// [`NativeInner`] needs, shared by [`Self::from_engine`] and + /// [`Self::from_engine_governed`] — everything up to, but not including, + /// the coordinator construction those two diverge on. + fn open_indexes(mut engine: Engine) -> Result { // R6.1 (ADR-049 §1) removed the opt-out that used to live here. // Until then this caller disabled the engine's automatic compaction // because it had "somewhere better" to run the merge: `with_inner`'s @@ -318,19 +524,12 @@ impl NativeMemoryStore { let vectors = PersistentVectorIndex::open(&mut engine, params).map_err(storage)?; let memory = PersistentMemoryIndex::open(&engine).map_err(storage)?; ensure_container_meta(&mut engine)?; - let state = NativeInner { + Ok(NativeInner { engine, vectors, memory, graph: PersistentGraph::new(), fts: PersistentFts::new(), - }; - let (reads, writer) = coordinator::WriteCoordinator::dormant(state, 256); - Ok(Self { - reads, - writer, - #[cfg(any(test, feature = "test-util"))] - _tempdir: None, }) } @@ -569,3 +768,177 @@ impl NativeMemoryStore { // lock" semantics, which changed from *recovered* to *terminal*): // `WriteCoordinator`'s own `#[cfg(test)] mod tests` in `coordinator.rs`, // `handler_panic_terminalizes_queued_and_late_intents`. + +/// `GOV-U32` — operational proof that a real flush progresses under a +/// saturated governed domain, driven through +/// [`NativeMemoryStore::open_with_engine_options_governed`] — a real +/// `NativeMemoryStore`, not a synthetic `try_reserve`-only harness. +/// ADR-071/ADR-072 both state this proof needs a real coordinator sitting at +/// its governed hard cap, not just the analytical `EmergencyHeadroom` +/// derivation (already closed, +/// `basemyai_engine::memory_governor::types::plan_tests`). +/// +/// `#[cfg(test)]`, in-crate: exercises `open_with_engine_options_governed` +/// specifically for its tunable `EngineOptions` (`pub(crate)`, test-only — +/// see its doc comment), needed to force rotation/flush on a bounded +/// dataset instead of the ~16.6 GiB default calibration. The genuinely +/// public entry point ([`open_with_key_governed`], a free function) has its +/// own out-of-crate proof: `crates/basemyai/tests/storage/ +/// governed_production_activation.rs`. +#[cfg(test)] +mod tests { + use std::sync::Arc; + use std::time::Duration; + + use basemyai_engine::{EngineOptions, GovernorConfig, MemoryDomain}; + + use super::NativeMemoryStore; + use crate::storage::MemoryStore; + use crate::{AgentId, Validity}; + + fn rotating_options() -> EngineOptions { + EngineOptions { + memtable_flush_threshold: 8, + max_key_bytes: 512, + max_value_bytes: 4096, + ..EngineOptions::default() + } + } + + /// The domain is deliberately undersized for two full memtable + /// generations at once (`budget_bytes = headroom + reservation + + /// 2 * per_class_bytes`, with `2 * per_class_bytes < reservation`): + /// every single rotation this test drives must genuinely wait for the + /// *previous* generation's background flush to complete and release its + /// charge before `owner_loop`'s `reserve_blocking` can admit the next + /// one — real backpressure at the hard cap, not spare capacity sailing + /// through. The whole write loop is wrapped in a bounded timeout + /// specifically so that a governor sized/wired wrong (blocking on + /// memory it should have reserved for its own progress) fails the test + /// deterministically instead of hanging CI forever. + #[tokio::test] + async fn a_real_flush_progresses_under_a_saturated_governed_domain() { + let dir = tempfile::tempdir().expect("tempdir"); + let key = basemyai_core::EncryptionKey::raw("gov-u32-contract-key"); + let options = rotating_options(); + + let reservation = basemyai_engine::estimate_memtable_reservation_bytes( + options.memtable_flush_threshold, + options.max_key_bytes, + options.max_value_bytes, + ) + .expect("no overflow"); + let headroom = basemyai_engine::memory_governor::estimate_emergency_flush_headroom_bytes( + options.memtable_flush_threshold, + options.max_key_bytes, + options.max_value_bytes, + options.block_size, + true, // this store is encrypted (EncryptionKey::raw below) + ) + .expect("no overflow"); + assert!(headroom > 0, "a real derived headroom, not a placeholder zero"); + + const PER_CLASS_BYTES: usize = 16 * 1024; + assert!( + 2 * PER_CLASS_BYTES < reservation, + "the domain must be undersized for two full generations at once, or this test would \ + never exercise real backpressure" + ); + let budget_bytes = headroom + reservation + 2 * PER_CLASS_BYTES; + let domain = MemoryDomain::new(GovernorConfig { + budget_bytes, + max_queue_depth: 16, + max_admission_waiters: 16, + reserved_slots: [2, 2], + reserved_bytes: [PER_CLASS_BYTES, PER_CLASS_BYTES], + weight: [1, 1], + max_intent_peak_bytes: [PER_CLASS_BYTES, PER_CLASS_BYTES], + governor_overhead_bytes: 0, + emergency_flush_headroom_bytes: headroom, + }) + .expect("valid governor config"); + let governor = Arc::clone(domain.governor()); + + let store = NativeMemoryStore::open_with_engine_options_governed(dir.path(), &key, options, &domain) + .expect("governed store opens against a domain sized for exactly one live generation"); + + assert_eq!( + governor.committed_bytes(), + headroom + reservation, + "the permanent headroom carve-out plus the bootstrap generation's charge, before any write" + ); + + let agent = AgentId::new("gov-u32-agent").expect("valid agent id"); + + // Enough entities/edges to force several real rotations at this + // tight sizing, each one genuinely blocked on the previous + // generation's flush. + const EDGES: i64 = 60; + let drive = async { + store + .graph_upsert_entity( + &agent, + "n0", + "node", + "n0", + Validity::since(0), + basemyai_engine::GraphSource::User, + ) + .await + .expect("seed entity n0"); + for i in 0..EDGES { + let dst = format!("n{}", i + 1); + store + .graph_upsert_entity( + &agent, + &dst, + "node", + &dst, + Validity::since(0), + basemyai_engine::GraphSource::User, + ) + .await + .unwrap_or_else(|error| panic!("entity {dst}: {error}")); + store + .graph_upsert_edge( + &agent, + &format!("n{i}"), + "rel", + &dst, + 1.0, + i, + basemyai_engine::GraphSource::User, + ) + .await + .unwrap_or_else(|error| panic!("edge {i}: {error}")); + } + }; + tokio::time::timeout(Duration::from_secs(30), drive).await.expect( + "60 writes at a governed hard cap must complete well within 30s -- a hang here is exactly \ + the self-deadlock GOV-U32 rules out (the governor blocking memory it needed for its own \ + progress)", + ); + + // No data lost across however many rotations actually happened. + let reached = store + .graph_traverse(&agent, "n0", EDGES as u32, EDGES + 1) + .await + .expect("graph_traverse must still work after repeated rotation under a tight domain"); + assert_eq!( + reached.len(), + EDGES as usize, + "every written edge must still be reachable after this whole run" + ); + + assert!( + governor.committed_bytes() >= headroom, + "the permanent EmergencyHeadroom carve-out must never have been released" + ); + assert!( + governor.lifecycle_is_running(), + "the governor must still be Running, not fail-stopped, after driving it repeatedly to its hard cap" + ); + + drop(store); + } +} diff --git a/crates/basemyai/tests/storage/governed_production_activation.rs b/crates/basemyai/tests/storage/governed_production_activation.rs new file mode 100644 index 0000000..9ced64b --- /dev/null +++ b/crates/basemyai/tests/storage/governed_production_activation.rs @@ -0,0 +1,105 @@ +//! Out-of-crate proof that `basemyai::storage::open_with_key_governed` — +//! the memory governor's genuinely public production activation entry +//! point — works end to end using only public API, no crate-internal +//! access. This is what a real external consumer of `basemyai` gets. +//! +//! The deep saturation/rotation/flush proof (`GOV-U32`: many real rotations +//! under a domain deliberately undersized for two generations at once, +//! forcing genuine backpressure at the hard cap) lives in-crate +//! (`basemyai::storage::native_store::tests`), because it needs +//! `open_with_engine_options_governed` (`pub(crate)`, test-only) for a +//! bounded `EngineOptions` calibration — the default engine calibration's +//! ~16.6 GiB memtable reservation (ADR-072 §B) would make that test +//! impractically slow to actually drive to saturation. This file complements +//! it at the actual product boundary. + +use basemyai::AgentId; +use basemyai::storage::{ + MemoryGovernorConfig, MemoryGovernorDomain, MemoryStore, NativeMemoryStore, open_with_key_governed, +}; + +fn config(budget_bytes: usize) -> MemoryGovernorConfig { + let per_class_bytes = (budget_bytes / 4).max(1); + MemoryGovernorConfig { + budget_bytes, + max_queue_depth: 16, + max_admission_waiters: 16, + reserved_foreground_slots: 2, + reserved_maintenance_slots: 2, + reserved_foreground_bytes: per_class_bytes, + reserved_maintenance_bytes: per_class_bytes, + foreground_weight: 1, + maintenance_weight: 1, + max_foreground_intent_peak_bytes: per_class_bytes, + max_maintenance_intent_peak_bytes: per_class_bytes, + governor_overhead_bytes: 0, + emergency_flush_headroom_bytes: 0, + } +} + +/// Just an accounting ceiling (no real allocation happens for it): large +/// enough to comfortably host the default engine calibration's ~16.6 GiB +/// bootstrap memtable reservation (ADR-072 §B) plus per-write admission +/// headroom. +const AMPLE_BUDGET_BYTES: usize = 32 * 1024 * 1024 * 1024; + +#[tokio::test] +async fn a_real_caller_can_open_and_write_through_the_public_governed_entry_point() { + let dir = tempfile::tempdir().expect("tempdir"); + let key = basemyai_core::EncryptionKey::raw("governed-production-activation-key"); + + let domain = MemoryGovernorDomain::new(config(AMPLE_BUDGET_BYTES)).expect("valid governor config"); + + let store: NativeMemoryStore = open_with_key_governed(dir.path(), &key, &domain) + .expect("a real caller, using only public API, opens a governed store"); + + let agent = AgentId::new("governed-activation-agent").expect("valid agent id"); + for id in ["n0", "n1"] { + store + .graph_upsert_entity( + &agent, + id, + "node", + id, + basemyai::Validity::since(0), + basemyai_engine::GraphSource::User, + ) + .await + .unwrap_or_else(|error| panic!("entity {id}: {error}")); + } + store + .graph_upsert_edge(&agent, "n0", "rel", "n1", 1.0, 0, basemyai_engine::GraphSource::User) + .await + .expect("edge write through the governed entry point succeeds"); + + let reached = store + .graph_traverse(&agent, "n0", 1, 1) + .await + .expect("the read path works through a governed store too"); + assert_eq!(reached.len(), 1, "the edge just written must be reachable"); + + drop(store); +} + +/// The bootstrap reservation is validated *before* the store is usable: +/// an under-sized domain is refused with a typed error, never a panic +/// (`WriteCoordinator::build`'s fix, same task) — a real caller can catch +/// and handle this, e.g. to retry with a larger domain. +#[test] +fn a_domain_too_small_for_the_bootstrap_reservation_is_refused_not_a_panic() { + let dir = tempfile::tempdir().expect("tempdir"); + let key = basemyai_core::EncryptionKey::raw("governed-production-activation-key"); + + // Valid but tiny (`budget_bytes: 1024`) — far too small to host the + // ~16.6 GiB default bootstrap memtable reservation. + let domain = MemoryGovernorDomain::new(config(1024)).expect("valid governor config"); + + let error = match open_with_key_governed(dir.path(), &key, &domain) { + Ok(_) => panic!("an undersized domain must be refused, not accepted"), + Err(error) => error, + }; + assert!( + matches!(error, basemyai::MemoryError::Core(basemyai_core::CoreError::Storage(_))), + "a typed storage error naming the refusal, not a panic: got {error:?}" + ); +} diff --git a/docs/adr/ADR-071-global-admission-accounting-and-governor.md b/docs/adr/ADR-071-global-admission-accounting-and-governor.md index c442736..a0b6390 100644 --- a/docs/adr/ADR-071-global-admission-accounting-and-governor.md +++ b/docs/adr/ADR-071-global-admission-accounting-and-governor.md @@ -2,7 +2,30 @@ **Identifiant : ADR-071** **Titre canonique :** _Governor mémoire global : réservations, admission bidimensionnelle, fairness et working-set gouverné_ -**Statut :** 🟢 **Accepted (rev 2)** — ratifié après revue croisée (concurrence, comptabilité mémoire, writer/recovery, red-team) confrontant le texte au code réel de `basemyai-engine` ; corrections de rev 2 listées en fin de document. **Amended by [ADR-072](ADR-072-cross-crate-admission-boundary-and-memtable-residency.md)** (2026-08-16) sur le placement de l'autorité d'admission — §"où vit le governor, non tranché" (ci-dessous) est tranché par ADR-072 en faveur de `basemyai::WriteCoordinator::submit`, ni `Engine` ni `WriterRuntime`, après audit du code réel de `basemyai` (absent du périmètre d'audit initial de ce document). Le reste de ce document reste en vigueur tel quel. +**Statut :** 🟢 **Accepted as architecture (rev 2)** — le texte normatif +lui-même (invariants `MG-*`, règles de verrouillage `GV-*`, algèbre DRR, +contrats RAII) est ratifié après revue croisée (concurrence, comptabilité +mémoire, writer/recovery, red-team) confrontant le texte au code réel de +`basemyai-engine` ; corrections de rev 2 listées en fin de document. +**Ce statut ne couvre pas l'implémentation.** §"Gates de ratification" +plus bas liste les conditions qui doivent être *simultanément* vraies avant +que le governor lui-même soit ratifié au sens opérationnel — activable, +testé, prêt à gouverner un budget réel. À la date de la dernière revue +(2026-08-17), elles ne le sont pas : `EmergencyHeadroom`'s byte formula is +now derived and tested against a real `SstWriter` run +(`memory_governor::estimate_emergency_flush_headroom_bytes`, §"Post- +ratification hardening" ci-dessous) but its *operational* proof +(`GOV-U32` — a real flush completing under a saturated domain) is not, +because nothing in this crate consumes a governed reservation for a flush +yet; le câblage direct de MG-17 dans `store/engine/write.rs` +(chantier phase 2 distinct du mécanisme §B déjà implémenté par ADR-072, +qui n'en dépend pas) n'est pas scopé, et une partie substantielle de la +suite `GOV-U*`/`GOV-S*`/`GOV-RSS*` ci-dessous n'est pas encore écrite (voir +`docs/status.md`, entrée du 2026-08-17, pour le détail actualisé). Lire « Accepted » comme +« l'architecture est la bonne, construisez dessus », pas comme « le +governor est prêt à gouverner » — les deux affirmations ont été confondues +dans une version antérieure de cette ligne de statut, ce que cette +reformulation corrige explicitement. **Amended by [ADR-072](ADR-072-cross-crate-admission-boundary-and-memtable-residency.md)** (2026-08-16) sur le placement de l'autorité d'admission — §"où vit le governor, non tranché" (ci-dessous) est tranché par ADR-072 en faveur de `basemyai::WriteCoordinator::submit`, ni `Engine` ni `WriterRuntime`, après audit du code réel de `basemyai` (absent du périmètre d'audit initial de ce document). Le reste de ce document reste en vigueur tel quel. **Jalon :** V2-04 **Date de recherche et de rédaction :** 2026-08-15 **Formats persistants :** aucun changement ; `format.lock` doit rester strictement inchangé. @@ -1287,3 +1310,9 @@ Une revue indépendante de `crates/basemyai-engine/src/memory_governor/` (le cod Un audit séparé du câblage ADR-072 (§A/§B) n’a trouvé **aucune violation confirmée** de CB-01…CB-07 — l’ingénierie d’ordre de verrouillage est correcte telle qu’écrite. Trois observations non bloquantes (portée `MemoryDomain` partagé pour l’argument panic du bootstrap, `MemoryGovernor::close()` non câblé dans les chemins d’arrêt du coordinateur, une fenêtre étroite de shutdown pouvant sauter la charge d’une génération rotée) sont documentées dans ADR-072 lui-même, avec la seconde déjà corrigée (voir ADR-072 §"Post-ratification hardening"). Une analyse de couverture de tests indépendante confirme que `GOV-U01, U04-U11, U19-U21(partiel), U24, U27, U31b, U38` sont couverts, mais que `GOV-U21-U23, U25-U26, U40-U41, S03` (des propriétés pures du governor, sans dépendance cache/writer) n’ont aujourd’hui **aucun** test, alors qu’ADR-071 les liste comme requis avant ratification et non comme hors-scope phase 1. Ce n’est pas corrigé par ce hardening — c’est une dette de test explicitement identifiée et non résolue, à traiter avant toute activation production. + +3. **`EmergencyHeadroom`, moitié analytique dérivée et testée — la moitié opérationnelle reste ouverte.** Lecture complète de `store::engine::flush::run_job` et de l’écrivain incrémental `SstWriter` qu’il pilote (`format::sst_block::write`, réécriture R6.2/ADR-049 §3) pour identifier chaque allocation qu’un flush fait au-delà de la memtable qu’il retire : + - **Terme dominant, exact** : `Memtable::iter_versions` clone en profondeur chaque clé et valeur dans un `Vec` frais avant que `run_job` n’appelle l’écrivain ; ce `Vec`, une fois déplacé dans `SstWriter::write_new_versioned`, garde toute son allocation vivante pendant la durée du `for (key, value) in entries` (`Vec::IntoIter` ne libère pas la queue déjà consommée). Pendant tout le flush, la memtable retirée (déjà chargée sous `WriteBufferMutable`) et ce clone (aujourd’hui totalement hors compte) coexistent. Même forme par-entrée que `estimate_memtable_reservation_bytes`, réutilisée telle quelle (`memory_governor::estimate_emergency_flush_headroom_bytes`). + - **Second terme, borné `O(block_size)` et non `O(taille de la memtable)`** : documenté comme l’objectif de conception explicite du réécrivain lui-même (« the largest live allocation is one data block ») et par la discipline ADR-053 (un seul Bloom fixe, un seul bâtisseur de feuille, au plus un bâtisseur de nœud interne par niveau). Chaque sous-terme appelle les *mêmes* fonctions de borne de capacité que `SstWriter::create` lui-même — `memory_governor::sst_writer_working_set_bytes`, ne peut donc pas dériver silencieusement de l’encodeur réel. + - **Preuve, pas seulement arithmétique cohérente avec elle-même** : `sst_writer_term_bounds_a_real_writers_steady_state_across_many_blocks_and_leaves` (`store/sst_block/write.rs`) fait tourner un vrai `SstWriter` sur un jeu de données forçant plusieurs feuilles et plusieurs niveaux d’index (même fixture que `partitioned_writer_builds_a_hierarchical_index`) et vérifie `writer.resident_bytes() <= sst_writer_working_set_bytes(...)` à chaque insertion — une première erreur de dérivation (multiplier une capacité de slots dimensionnée pour des entrées minimales par la taille d’une clé maximale, plutôt que d’utiliser séparément la borne réelle en octets `index_chunk_plaintext_limit`) a été trouvée et corrigée précisément grâce à ce test, qui produisait un terme aberrant de 4,67 billions d’octets avant correction. + - **Ce que cette dérivation ne ferme pas** : `resident_bytes()` ne voit pas les tampons transitoires `plain`/scellé/chiffré que `write_staged_block`/`seal_section` allouent et libèrent en une seule fois — cette portion reste dérivée par lecture directe du code, documentée mais non mesurée à l’exécution. Et surtout : rien dans `basemyai-engine` ne consomme encore de réservation gouvernée pour un flush (`store::engine::flush` ne touche pas `memory_governor`) — dériver le nombre est un prérequis pour fermer le statut « NON PROUVÉ », pas la totalité : la preuve *opérationnelle* (`GOV-U32`, un vrai flush qui progresse sous un domaine saturé) reste bloquée sur le câblage MG-17/`write.rs` de phase 2 (point 1 ci-dessus dans le chantier écarté de `CB-T4`, ADR-072). diff --git a/docs/adr/ADR-072-cross-crate-admission-boundary-and-memtable-residency.md b/docs/adr/ADR-072-cross-crate-admission-boundary-and-memtable-residency.md index 983f738..2e86f0a 100644 --- a/docs/adr/ADR-072-cross-crate-admission-boundary-and-memtable-residency.md +++ b/docs/adr/ADR-072-cross-crate-admission-boundary-and-memtable-residency.md @@ -127,10 +127,30 @@ relâché), `owner_commands` classe l'issue et décide release/retain : | `CoordinatorOutcome` | Statut de durabilité WAL | Action sur `governor_permit` | | --- | --- | --- | | `Aborted { .. }` | WAL jamais durablement écrit (échec pré-réservation, ou `WalWriteOutcome::Aborted` propre) | **RELEASE** | -| `Committed { .. }` | fsync confirmé + install produit réussi | **RETAIN** | -| `DurableReopenRequired { .. }` | fsync confirmé, install produit échoué | **RETAIN** | -| `OutcomeUnknown { .. }` | issue WAL elle-même inconnue (crash à la frontière) | **RETAIN** | -| `StructuralReopenRequired { .. }` | publication durable structurelle confirmée, mismatch RAM | **RETAIN** | +| `Committed { .. }` | fsync confirmé + install produit réussi | **RELEASE** | +| `DurableReopenRequired { .. }` | fsync confirmé, install produit échoué | **RELEASE** | +| `OutcomeUnknown { .. }` | issue WAL elle-même inconnue (crash à la frontière) | **RELEASE** | +| `StructuralReopenRequired { .. }` | publication durable structurelle confirmée, mismatch RAM | **RELEASE** | + +**Correction (2026-08-17, revue indépendante post-implémentation) — la +colonne RETAIN ci-dessus était un résidu de conception non mis à jour, pas +une spécification suivie par le code.** Elle datait du modèle "transfert +incrémental" (option 1 du tableau de comparaison §B, explicitement +**rejetée** au profit de la "réservation amont" — option 2) et n'a jamais +été corrigée après ce choix. Avec `write_buffer_bytes = 0` imposé par +l'étape 4 de §A ci-dessus, le `MemoryPlan` par-write ne porte **jamais** la +charge de résidence memtable — il ne couvre que les octets transitoires +WAL/intent, dont le besoin s'éteint dès le retour d'`execute()`, quelle que +soit l'issue. La résidence memtable est couverte par un mécanisme +entièrement distinct : la réservation amont par génération de §B/CB-05 +(`attach_memtable_charge`/`charge_rotated_memtable`, dimensionnée une seule +fois au `seal_memtable`). RELEASE uniforme est donc correct par +construction pour `release_governor_permit`, pas un compromis provisoire en +attente de §B — §B est déjà en place et couvre ce besoin par un chemin +séparé. Le code (`coordinator.rs`, `release_governor_permit`) a toujours +implémenté RELEASE uniforme ; c'était cette table et le commentaire +`TODO(ADR-072 §B)` du code (retiré) qui étaient en retard sur la +conception réelle, pas l'inverse. Cette table est exhaustive et ne laisse aucune variante ambiguë après traçage direct de `CoordinatorOutcome` (`coordinator.rs:151-178`) jusqu'à @@ -312,8 +332,17 @@ CB-08 EmergencyHeadroom (ADR-071) est une condition de non-blocage ## Points ouverts / différés (non bloquants pour la ratification, listés explicitement) - Forme exacte du signal de rotation sur `CoordinatorOutcome` (§B). -- Dérivation analytique d'`EmergencyHeadroom` (héritée d'ADR-071, - désormais explicitement liée à CB-05/CB-08). +- **Dérivation analytique d'`EmergencyHeadroom` — moitié faite (2026-08-17).** + `memory_governor::estimate_emergency_flush_headroom_bytes` dérive et teste + la formule (mémoire clonée par `Memtable::iter_versions` + working set + `O(block_size)` du `SstWriter`, ce dernier grounded contre un vrai writer + via `sst_writer_term_bounds_a_real_writers_steady_state_across_many_ + blocks_and_leaves`). Ce qui reste ouvert, toujours lié à CB-05/CB-08 : + personne ne consomme encore cette réservation (`store::engine::flush` ne + touche pas `memory_governor`), donc la preuve *opérationnelle* (`GOV-U32`, + un flush réel qui progresse sous domaine saturé) reste bloquée sur le + câblage MG-17/`write.rs` de phase 2 — la même dépendance déjà identifiée + pour `CB-T4` ci-dessus. - **Revue de la réservation memtable de 16,6 GiB (`memtable_flush_threshold × per-entry-max`, §B) et, probablement, de la condition de seal elle-même.** Cette valeur est le pire cas analytique sous les défauts @@ -420,20 +449,43 @@ ont cependant été trouvés et le premier corrigé le jour même : Une analyse de couverture séparée confirme que `CB-T1a` (traversée du canal), `CB-T1c` (panic handler) et `CB-T3` (ordre rotation-avant-écriture) -sont couverts, mais relève un écart plus grave que les autres pour -**`CB-T4` (« `maybe_flush` tolère `AdmissionWouldBlock` »)** : ce n'est pas -seulement un test manquant, c'est un mécanisme **non implémenté** — -`AdmissionWouldBlock` n'apparaît nulle part dans -`store/engine/write.rs`, confirmé par recherche exhaustive ; le -`TODO(ADR-072 §B)` déjà présent dans `coordinator.rs` marque précisément ce -manque. Tant que ce n'est pas câblé, l'histoire « le flush est la soupape -d'un domaine saturé » de §B n'a pas de chemin de code, indépendamment de -tout test. `CB-T1d` (4 des 5 branches de `CoordinatorOutcome` sans +sont couverts. `CB-T1d` (4 des 5 branches de `CoordinatorOutcome` sans couverture gouvernée), `CB-T2` (ordre gouverneur-avant-sémaphore sous contention réelle, pas seulement un refus immédiat) et `CB-T5` (borne de la -fenêtre CB-07 jamais reproduite) restent également ouverts et sont, comme -`CB-T4`, requis avant activation production — pas seulement avant -ratification de ce document. +fenêtre CB-07 jamais reproduite) restent ouverts et sont requis avant +activation production — pas seulement avant ratification de ce document. + +**Correction (2026-08-17) — `CB-T4` ne décrit pas le mécanisme réellement +construit et est requalifié ci-dessous.** Une première lecture de cette +revue avait classé `CB-T4` (« `maybe_flush` tolère `AdmissionWouldBlock` ») +comme un mécanisme manquant, en citant le `TODO(ADR-072 §B)` de +`coordinator.rs` comme preuve. Vérification directe : ce `TODO` concerne en +réalité la décision release-vs-retain de `release_governor_permit` (CB-04), +pas `maybe_flush` — mauvaise référence croisée, pas un constat correct. La +vraie situation est plus fondamentale : la réservation amont de rotation +implémentée par §B (`reserve_blocking`, `attach_memtable_charge`) **ne peut +structurellement jamais produire `AdmissionWouldBlock`** — le contrat de +`reserve_blocking` lui-même l'exclut explicitement (« `AdmissionWouldBlock` +n'est jamais retourné — attendre au lieu de le retourner est le point », +`memory_governor/admission.rs`). C'est un choix de conception assumé, pas +un oubli : si le domaine sature au moment de la réservation, le write +bloque jusqu'à libération de capacité (généralement par un flush), et +`EmergencyHeadroom` est précisément la garantie anti-interblocage de ce +choix (CB-08). Câbler une tolérance `AdmissionWouldBlock` dans `maybe_flush` +irait donc à l'encontre du mécanisme §B tel que ratifié. + +Ce que `CB-T4` décrit réellement appartient à un chantier distinct et non +scopé : le câblage direct de MG-17 dans `store/engine/write.rs` lui-même +(ADR-071 §"Correction (audit code, 2026-08-15) — le writer réel n'est pas +`writer_runtime.rs`" : réserver le pic *avant* le fsync WAL, « un +changement structurel du chemin d'écriture », explicitement listé comme +phase 2 dans `memory_governor/mod.rs`). Ce chantier n'a pas encore de +conception — décider s'il utilise une réservation bloquante ou non +bloquante, et donc si une tolérance `AdmissionWouldBlock` a un sens, en +fait partie. **`CB-T4` est donc requalifié comme dépendance phase 2 +(câblage `write.rs`/MG-17), retiré de la liste "requis avant activation +production" du mécanisme §B déjà implémenté**, qui n'en a structurellement +pas besoin. --- @@ -448,9 +500,11 @@ ratification de ce document. - Réservation amont de rotation : aucune écriture n'atteint la nouvelle génération avant que la réservation ait été acquise (exploite la sérialisation d'`owner_loop`, pas un mock de verrou). -- `maybe_flush` tolère `AdmissionWouldBlock` sans démouvoir un write - déjà durable (miroir de la tolérance existante à `BackgroundFlush`/ - `WriterReconcileRequired`, `write.rs:301-319`). +- ~~`maybe_flush` tolère `AdmissionWouldBlock`~~ — **requalifié + (2026-08-17), voir §"Post-ratification hardening" ci-dessus.** Le + mécanisme §B implémenté (`reserve_blocking`) ne produit structurellement + jamais cette erreur ; ce test appartient au chantier distinct, non + scopé, du câblage MG-17 dans `store/engine/write.rs` (phase 2). - Test de dérogation CB-07 : reproduire délibérément la fenêtre `Engine::get`-concurrent-au-flush et prouver qu'elle reste bornée à la durée d'un seul appel `get()`, jamais latente. @@ -478,7 +532,17 @@ réservation amont d'une rotation, **tous** les writes du coordinateur se bloquent jusqu'à libération de capacité, généralement par un flush — et le flush lui-même a besoin de capacité gouvernée pour progresser. Activer le hard cap avec un `EmergencyHeadroom` non prouvé revient à activer un -mécanisme qui peut s'auto-bloquer sans chemin de sortie garanti. Ce gate -reste ouvert tant que ADR-071 documente `EmergencyHeadroom` comme -« NON PROUVÉ » (voir ce document, §B et invariant CB-08) ; il n'est pas -levé par le seul fait que §A/§B compilent et passent leurs tests unitaires. +mécanisme qui peut s'auto-bloquer sans chemin de sortie garanti. + +**Statut (2026-08-17) : moitié faite, gate toujours fermé.** La borne +byte-level est désormais dérivée et testée +(`memory_governor::estimate_emergency_flush_headroom_bytes`, grounded +contre un vrai `SstWriter` — voir ADR-071 §"Post-ratification hardening"). +Ce que « dérivée analytiquement » exigeait est donc satisfait. Ce qui ne +l'est pas : rien dans `basemyai-engine` ne consomme encore cette +réservation pour un flush réel, donc l'affirmation « le flush peut +progresser sous un domaine saturé » n'a aucun chemin de code à ce jour — +seule une preuve *opérationnelle* (`GOV-U32`) fermerait ce point, et elle +dépend du câblage MG-17/`write.rs` de phase 2, non scopé. Ce gate reste +donc ouvert ; il n'est pas levé par le seul fait que §A/§B compilent et +passent leurs tests unitaires, ni par le seul fait que la formule existe. diff --git a/docs/status.md b/docs/status.md index 172c48a..06f5103 100644 --- a/docs/status.md +++ b/docs/status.md @@ -1,5 +1,163 @@ # BaseMyAI — Implementation Status Matrix +**Mise à jour 2026-08-18 — dette de test P1 ADR-071/ADR-072 fermée +(`CB-T1d/T2/T5`, `GOV-U21-23/25-26/40-41`, `GOV-S03`, `GOV-RSS01-03`) ; +gate d'activation production reste fermé (inchangé, cf. entrées +ci-dessous).** Cinq chantiers indépendants (isolation `git worktree`), +fusionnés sur une branche `integration/p1-governor-gate-closure` (non +mergée sur `dev` à ce stade — décision humaine), puis revus par un agent +indépendant qui n'a participé à aucune implémentation. + +- **Fermés, avec preuve red-before/green-after** : + - `CB-T1d` — 4 des 5 branches `CoordinatorOutcome` sans couverture + gouvernée (`Aborted`, `OutcomeUnknown`, `StructuralReopenRequired` + directement ; `DurableReopenRequired` par construction directe, cette + branche étant structurellement inatteignable en production aujourd'hui + — confirmé par grep et par la revue indépendante, documenté comme tel + dans le test, pas maquillé en bout-en-bout). + - `CB-T2` — ordre gouverneur-avant-sémaphore sous contention réelle : + prouvé qu'un appelant parqué sur la capacité en octets du gouverneur + ne détient jamais un slot `core.permits` pendant son attente (la + conception FIFO stricte intra-classe du gouverneur rendait le design + initialement suggéré — « N autres writers progressent en concurrence » + — structurellement impossible pour des writers de même classe ; + remplacé par une preuve directe au niveau sémaphore, documentée). + - `GOV-U21/U22/U23` — protections bytes/slots/symétrie entre + `Foreground`/`Maintenance` : aucun bug trouvé, propriétés déjà + correctes, couverture ajoutée avec preuve par injection de défaut + (protection cross-classe rendue self-only, rouge confirmé, reverté). + - `GOV-U25/U26` — spurious wake / lost wake : aucun bug trouvé (le + pattern Mesa-monitor de `acquire_admission` empêchait déjà les deux), + couverture ajoutée avec preuve par injection (bug classique + unlock-sleep-relock reproduit puis reverté). + - `GOV-U40/U41` — domaine partagé entre deux « engines » vs domaines + indépendants : propriétés correctes, couverture ajoutée. Le point déjà + documenté par ADR-072 (`WriteCoordinator::build` panique sur un + domaine partagé faute d'agent capable de libérer des octets) reste + confirmé mais **non corrigé, à dessein** — inatteignable en production + (`dormant_governed` sans site d'appel), déjà tracké. + - `GOV-S03` — arrêt sous stress (waiters + réservations en vol) : + `MemoryGovernor::close()` réveille tous les waiters dans un délai + borné, aucune admission nouvelle après fermeture, bookkeeping revient + à zéro. + - `CB-T5` — fenêtre de sous-comptabilité CB-07 (release à + `flush.rs:560` vs dernier-`Arc` réel via un `Engine::snapshot()` + concurrent) reproduite par construction (vraie memtable, vrai thread + de flush, pas un failpoint arbitraire) et prouvée bornée exactement + comme documenté par ADR-072 — aucun bug trouvé. + - `GOV-RSS01/RSS03` — oracle RSS générique (ADR-053 étendu) : plateau + RSS confirmé sous volumes croissants, `charged = resident + + pinned_only` confirmé sous éviction+pinning réel. `GOV-RSS02` + (contrôle négatif) a d'abord été mal câblé (sink de fuite scopé à une + instance droppée, donc invisible) — corrigé (`static` process-lifetime) + et revérifié : 226 Mio de croissance non détectée par + `committed_bytes()` correctement détectés par le harnais RSS. + Portée **explicitement limitée au générique ADR-071** : l'oracle + RSS/stress spécifique à la rotation memtable §B, listé séparément par + ADR-072 (« au-delà des `GOV-RSS01..03` génériques »), reste ouvert. +- **Corrigé, hors périmètre des 5 chantiers** — `docs(V2-04)`, commit + séparé : la colonne RETAIN de la table §A d'ADR-072 (4 des 5 branches + `CoordinatorOutcome`) était un résidu de conception jamais mis à jour + après que §B a retenu la « réservation amont » plutôt que le « transfert + incrémental » — avec `write_buffer_bytes = 0` imposé par §A, le permit + par-write ne porte jamais la charge memtable, donc RELEASE uniforme + (ce que fait déjà `release_governor_permit`) est correct par + construction, pas un compromis provisoire. Table ADR-072 et + `TODO(ADR-072 §B)` du code (celui-ci antérieur à cette session, issu du + commit `b7cc3c0` original) corrigés pour refléter le code réel. Aucun + changement de comportement. +- **Revue indépendante** : ré-exécution personnelle des preuves + red/green de `GOV-U21/22/23` et `GOV-U26` (défauts réinjectés, + confirmés rouges, revertés, confirmés verts) plutôt qu'acceptées sur + parole ; diff fonction-par-fonction des 3 branches ayant fusionné dans + le même fichier (`admission.rs`) contre l'état final, confirmé + identique octet-pour-octet, rien perdu ni altéré par la résolution + manuelle de conflit. Une régression potentielle investiguée + (`seal_recovery::flush_after_recovery_publishes_the_sst_and_keeps_the_empty_active_segment`, + flaky sous charge dans `cargo xtask test` complet) — confirmée + **préexistante et sans rapport** avec ce chantier (fichier non touché, + 8/8 vert en isolation et en run propre à trois reprises) ; c'est la même + flake déjà notée le 2026-08-10 (`docs/status.md`), toujours non + corrigée, hors périmètre. +- **Gates rejoués sur la branche fusionnée** : `cargo xtask check` + **vert** ; `cargo xtask test-wiring` **vert** (42 binaires) ; `cargo + xtask test` **vert** (suite complète + xtask lui-même 43/43, 1 + maintainer-only ignoré) ; `cargo xtask test-crash-consistency` **13/13, + 348,61 s** — aucune régression ; `cargo test -p basemyai-engine + --features test-util --lib` **618/618** ; `cargo test -p basemyai + --features test-util --lib` **154/154** — revérifié après la correction + documentaire ultérieure (commentaire seul, aucun changement de + comportement), toujours **154/154** ; `format.lock` strictement + inchangé. +- **N'est PAS débloqué par ce chantier** : le gate d'activation + production reste fermé, inchangé depuis les entrées précédentes — + `EmergencyHeadroom` opérationnel (`GOV-U32`) et le câblage direct + MG-17/`write.rs` restent phase 2, non scopés ici, comme documenté par + ADR-071/ADR-072 eux-mêmes. `dormant_governed` reste sans site d'appel + production. + +**Mise à jour 2026-08-17 (suite 2) — CB-T4 requalifié, EmergencyHeadroom +dérivé et testé (moitié analytique), worktrees/branches mortes nettoyées.** +Suite directe de l'entrée précédente, sur décision explicite de +priorisation (P0 avant activation) : + +- **CB-T4 corrigé** — l'attribution initiale (le `TODO(ADR-072 §B)` de + `coordinator.rs` comme preuve d'un mécanisme manquant) était une mauvaise + référence croisée : ce TODO concerne CB-04 (release-vs-retain), pas + `maybe_flush`. Le mécanisme §B implémenté (`reserve_blocking`) ne peut + structurellement jamais produire `AdmissionWouldBlock` — c'est le choix + de conception assumé (bloquer + `EmergencyHeadroom`), pas un oubli. + `CB-T4` décrit en réalité le câblage MG-17 dans `store/engine/write.rs`, + chantier phase 2 distinct et non scopé — retiré de la liste "requis + avant activation" du mécanisme §B. +- **`EmergencyHeadroom` : moitié analytique dérivée, testée, groundée + contre un vrai writer — moitié opérationnelle toujours bloquée.** + `memory_governor::estimate_emergency_flush_headroom_bytes` (nouveau) somme + deux termes trouvés en lisant `store::engine::flush::run_job` et + `format::sst_block::write::SstWriter` de bout en bout : (1) le clone + complet de la memtable que `Memtable::iter_versions` produit avant + l'écriture de l'SST, dominant, réutilise exactement la formule CB-06 ; (2) + le working set propre du `SstWriter`, prouvé `O(block_size)` et non + `O(taille de la memtable)` par la conception documentée du réécrivain + R6.2/ADR-049 §3 lui-même. Le test + `sst_writer_term_bounds_a_real_writers_steady_state_across_many_blocks_and_leaves` + (`store/sst_block/write.rs`) fait tourner un vrai `SstWriter` et vérifie + que `resident_bytes()` ne dépasse jamais la borne dérivée — une première + version de la formule (multiplier une capacité de slots par une taille de + clé maximale au lieu d'utiliser la borne réelle en octets séparément) + produisait un terme aberrant de 4,67 billions d'octets ; le test l'a + détecté immédiatement, avant tout commit. Ce qui reste ouvert : rien ne + consomme encore cette réservation pour un flush réel + (`store::engine::flush` ne touche pas `memory_governor`), donc la preuve + *opérationnelle* (`GOV-U32`) reste bloquée sur le câblage MG-17/`write.rs` + de phase 2 — même dépendance que `CB-T4`. Le gate d'activation production + reste donc fermé (ADR-072 §"Gates de ratification"). +- **Nettoyage worktrees/branches** : `.claude/worktrees/agent-*` (5 + répertoires, ~13 Go) pointaient vers `D:/dev/_ecosystems/forgemyai/basemyai` + — un chemin de dépôt qui n'existe plus sur disque — et étaient donc + totalement orphelins (confirmé : aucun ne figurait dans `git worktree + list` du dépôt courant). Supprimés. 15 branches locales + `worktree-agent-*` (plus `hotfix/candle-core-0.11`) sans worktree vivant + et entièrement fusionnées dans `dev` supprimées via `git branch -d` + (jamais `-D` : la vérification stricte de git a d'ailleurs refusé de + supprimer `codex/v2-03-write-coordinator`/`codex/v2-normative-integration` + malgré leur apparition dans `--merged dev`, laissées intactes pour revue + manuelle). Les worktrees enregistrés légitimes + (`basemyai-r5-bench-r3`/`r4`, le worktree codex bugscan) n'ont pas été + touchés. +- **Gates rejoués** : `cargo xtask check` vert (fmt + clippy `-D warnings` + toutes crates, y compris le cross-module cfg-gating de + `sst_writer_working_set_bytes`) ; `cargo test -p basemyai-engine + --features test-util --lib` **610/610** (+6 nouveaux tests) ; `cargo + xtask test-crash-consistency` relancé sur l'arbre corrigé **13/13, + 539,25 s** — aucune régression. +- **Correction hygiène commit non liée** : `crates/basemyai-core/src/storage/engine.rs` + portait un diff non expliqué (commentaires de doc raccourcis) découvert + pendant cette session, sans lien avec les autres changements uncommitted + de la session précédente ; confirmé anodin (aucune perte de contenu + factuel, juste moins de prose) et committé séparément après vérification + explicite avec l'utilisateur. + **Mise à jour 2026-08-17 (suite) — première revue adversariale indépendante de la phase 1 ADR-071/ADR-072 : deux bugs réels corrigés, dette de test substantielle identifiée et trackée, aucune activation production @@ -28,10 +186,17 @@ passé le code committé ci-dessous au crible pour la première fois. `flush.rs` — tel qu'écrit, ré-audité ligne à ligne contre le code réel. - **Dette identifiée, non corrigée (trackée dans les deux ADR, requise avant activation production)** : - - `CB-T4` — `store/engine/write.rs` ne gère nulle part - `AdmissionWouldBlock` : ce n'est pas un test manquant, c'est un - mécanisme absent. Le `TODO(ADR-072 §B)` déjà présent dans - `coordinator.rs` marque le manque. + - **`CB-T4` requalifié (2026-08-17)** — la citation initiale du + `TODO(ADR-072 §B)` de `coordinator.rs` comme preuve était une mauvaise + référence croisée (ce TODO concerne CB-04, pas `maybe_flush`). Le + mécanisme §B implémenté (`reserve_blocking`) ne peut structurellement + jamais produire `AdmissionWouldBlock` — c'est un choix de conception + assumé (bloquer + `EmergencyHeadroom` comme garde anti-interblocage), + pas un oubli. `CB-T4` décrit en réalité le câblage direct de MG-17 dans + `store/engine/write.rs`, un chantier phase 2 distinct et non scopé — + retiré de la liste "requis avant activation production" du mécanisme + §B, qui n'en dépend pas. Voir ADR-072 §"Post-ratification hardening" + pour le détail. - `GOV-U21-U23, U25-U26, U40-U41, GOV-S03` — propriétés pures du governor (fairness bytes/slots, spurious/lost wake, domaines partagés/indépendants, shutdown à l'échelle) sans aucun test, alors diff --git a/xtask/src/main.rs b/xtask/src/main.rs index d84a8b3..63e8087 100644 --- a/xtask/src/main.rs +++ b/xtask/src/main.rs @@ -336,6 +336,21 @@ const ENGINE_TESTS: &[(&str, EngineGate, &str)] = &[ EngineGate::Light, "J0 preflight ENG-DUR-002/004", ), + ( + "cb_t5_bounded_release_window", + EngineGate::Light, + "ADR-072 CB-T5 : fenêtre de sous-comptage CB-07 (release à flush.rs:560, pas au vrai \n dernier Arc) reproduite avec un lecteur concurrent réel — prouvée bornée à la durée \n de vie de ce lecteur, jamais latente", + ), + ( + "gov_rss_oracle", + EngineGate::Light, + "ADR-071 GOV-RSS01-03 : oracle RSS générique du governor (étend la méthodologie \n ADR-053/D-6) — flatness sous volumes croissants, contrôle négatif dédié \n (feature governor-rss-negative-control-leak, jamais activée par défaut), et \n charged = resident + pinned_only sous lecteurs concurrents + éviction", + ), + ( + "gov_rotation_rss_oracle", + EngineGate::Light, + "ADR-072 §B : oracle RSS/stress spécifique à la rotation memtable (toujours ouvert après \n GOV-RSS01-03, qui ne couvre que le governor générique) — 40 générations réelles, \n rotation + écriture + flush, un tiers pinnées par un lecteur concurrent (fenêtre \n CB-07), preuve qu'il n'existe ni sous-comptage, ni double comptage, ni fuite \n croissante sur committed_bytes(), et qu'aucune référence ne survit à son lecteur", + ), ]; /// Construit la ligne `cargo test -p basemyai-engine …` d'un gate donné à