Parcourir la source

Coordinate balance-projection appends through storage, not memory

Address a review of ADR-0019. The background cache-point append had no
dedup, so a hot account read at high QPS spawned one full fold per read.
An in-process guard cannot coordinate the multiple independent instances
the projection is built for, so move the guard into the shared
balance_projection rows: append_cache_point returns before the fold when
a cache point already sits within a debounce window below the target
watermark. Redundant spawns then cost one indexed read, and the dedup
holds across processes and restarts with no lease, lock, or CAS, matching
the coordination-free design.

Log a warning when a background append fails so a projection that
silently stops advancing is visible.

Add a deterministic test that folds a non-empty tail onto a real
snapshot, the watermark-boundary case the existing tests never reached
(they either created no snapshot or left the tail empty). Correct the
property-test comment that claimed cache points were appended during the
run, and the ADR header that still named the rejected lease and projector
service.
Cesar Rodas il y a 1 semaine
Parent
commit
631a6e9e42

+ 34 - 5
crates/kuatia/src/ledger/projection.rs

@@ -96,10 +96,19 @@ async fn fold_tail(
 /// near-duplicate row on every read (`min_new == 0` forces an append). Append-only
 /// and best effort; a stale or duplicate append is harmless because a read takes
 /// the closest-at-or-before cache point.
+///
+/// `debounce_ms` is the storage-based single-flight guard. When the newest cache
+/// point already sits within `debounce_ms` below the target watermark, this
+/// returns before the fold, so a hot account read at high QPS does not fan out a
+/// full fold per read. The guard lives in the shared `balance_projection` rows,
+/// not in process memory, so it dedups across every ledger instance and survives
+/// a restart, and it needs no lease, lock, or CAS (ADR-0019). `debounce_ms == 0`
+/// disables it (the reconcile path forces an append regardless).
 async fn append_cache_point(
     store: &dyn Store,
     grace_ms: i64,
     min_new: u64,
+    debounce_ms: i64,
     account: &AccountId,
     asset: &AssetId,
 ) -> Result<(), LedgerError> {
@@ -108,7 +117,12 @@ async fn append_cache_point(
         .get_closest_balance_projection(account, asset, new_watermark)
         .await?;
     let (snapshot, from_ts) = match closest {
-        // A cache point already covers this watermark: nothing to add.
+        // Storage-based debounce: a cache point within `debounce_ms` below the
+        // target already shortens the tail enough, so skip the fold. This is what
+        // collapses a per-read fan-out into at most one fold per debounce window.
+        Some(p) if new_watermark.saturating_sub(p.watermark) < debounce_ms => return Ok(()),
+        // A cache point already covers this watermark: nothing to add. (Also the
+        // debounce == 0 stop, so an exact-or-newer watermark is never duplicated.)
         Some(p) if p.watermark >= new_watermark => return Ok(()),
         Some(p) => (p.balance, Some(p.watermark.saturating_add(1))),
         None => (Cent::ZERO, None),
@@ -179,24 +193,39 @@ impl Ledger {
     /// Spawn a best-effort background append gated on `snapshot_interval` new
     /// credits/debits. Uses only the store handle and config, so it needs no
     /// `Arc<Self>` and can run from a `&self` read.
+    ///
+    /// Redundant spawns are cheap, not suppressed here: the storage-based debounce
+    /// inside [`append_cache_point`] (keyed on the newest shared cache-point row,
+    /// with `debounce_ms == grace`) returns before the fold when a recent cache
+    /// point already exists, so a hot account read at high QPS does at most one
+    /// fold per grace window, coordinated across every instance rather than in
+    /// this process's memory.
     fn spawn_append(&self, account: AccountId, asset: AssetId) {
         let store = Arc::clone(&self.store);
         let grace = self.projection_grace_ms;
         let min_new = self.snapshot_interval;
         tokio::spawn(async move {
-            let _ = append_cache_point(store.as_ref(), grace, min_new, &account, &asset).await;
+            if let Err(err) =
+                append_cache_point(store.as_ref(), grace, min_new, grace, &account, &asset).await
+            {
+                // Best effort: a failed append only lengthens a later read's tail.
+                // Log it so a projection that silently stops advancing is visible.
+                tracing::warn!(?account, ?asset, error = %err, "balance projection append failed");
+            }
         });
     }
 
     /// Append a cache point for one `(account, asset)` now (folding up to
     /// `now − grace`), unconditionally. The read path appends lazily in the
-    /// background; this forces one, exposed for tests and reconciliation.
-    /// Idempotent and append-only.
+    /// background; this forces one (no debounce), exposed for tests and
+    /// reconciliation. Append-only: a repeat within the same grace-adjusted
+    /// millisecond is a no-op (the watermark is already covered), otherwise it
+    /// adds a fresh row.
     pub async fn append_cache_point(
         &self,
         account: &AccountId,
         asset: &AssetId,
     ) -> Result<(), LedgerError> {
-        append_cache_point(self.store(), self.projection_grace_ms, 0, account, asset).await
+        append_cache_point(self.store(), self.projection_grace_ms, 0, 0, account, asset).await
     }
 }

+ 73 - 5
crates/kuatia/tests/projection.rs

@@ -46,6 +46,15 @@ async fn deposit(ledger: &Arc<Ledger>, to: i64, amount: i64) {
     ledger.commit(transfer).await.unwrap();
 }
 
+/// Current time as Unix milliseconds, the same clock the ledger stamps commits
+/// and cache-point watermarks with, so a test can pin a watermark to "now".
+fn unix_millis_now() -> i64 {
+    std::time::SystemTime::now()
+        .duration_since(std::time::UNIX_EPOCH)
+        .unwrap()
+        .as_millis() as i64
+}
+
 async fn pay(ledger: &Arc<Ledger>, from: i64, to: i64, amount: i64) {
     let transfer = TransferBuilder::new()
         .pay(account(from), account(to), usd(), Cent::from(amount))
@@ -231,6 +240,63 @@ async fn read_below_interval_appends_nothing() {
     );
 }
 
+/// Snapshot plus a genuinely non-empty tail. A cache point is pinned to the
+/// boundary between two commit batches, then more commits land strictly after
+/// its watermark, so the read must fold `snapshot + tail`: not a whole-history
+/// live sum (no snapshot) and not a snapshot that already holds everything (empty
+/// tail). This is the watermark boundary the grace-0 and no-snapshot tests never
+/// reach; an off-by-one at `watermark + 1` would drop or double-count a
+/// tail transfer here.
+#[tokio::test]
+async fn snapshot_plus_nonempty_tail_matches_live_sum() {
+    let ledger = Arc::new(Ledger::new(InMemoryStore::new()));
+    no_overdraft(&ledger, 1).await;
+    no_overdraft(&ledger, 2).await;
+    overdraft(&ledger, 99).await;
+
+    // Batch 1: the snapshot will cover exactly this.
+    deposit(&ledger, 1, 1000).await;
+    pay(&ledger, 1, 2, 250).await;
+
+    // Pin a cache point at the batch-1/batch-2 boundary. Its balance is the
+    // authoritative batch-1 sum (folded by the ledger, not by hand); its watermark
+    // is a time at or after every batch-1 commit. Appended directly so the
+    // watermark is controlled, independent of the lazy read-path grace.
+    let snapshot = ledger.compute_balance(&account(1), &usd()).await.unwrap();
+    let watermark = unix_millis_now();
+    ledger
+        .store()
+        .append_balance_projection(&account(1), &usd(), snapshot, watermark)
+        .await
+        .unwrap();
+
+    // Cross the millisecond boundary so every batch-2 commit is stamped strictly
+    // after the watermark and lands in the tail, never the snapshot.
+    tokio::time::sleep(std::time::Duration::from_millis(3)).await;
+
+    // Batch 2: folded onto the snapshot as a non-empty tail (both credits and
+    // debits for account 1).
+    pay(&ledger, 1, 2, 100).await;
+    pay(&ledger, 2, 1, 40).await;
+    deposit(&ledger, 1, 500).await;
+
+    // The read is snapshot + folded tail, and still equals the live sum.
+    let projected = ledger.balance(&account(1), &usd()).await.unwrap();
+    let authoritative = ledger.compute_balance(&account(1), &usd()).await.unwrap();
+    assert_eq!(
+        projected, authoritative,
+        "snapshot + non-empty tail diverged from the live sum"
+    );
+    // Guard the test itself: the snapshot must have been partial, so the tail
+    // actually carried value. Otherwise this silently degrades to the empty-tail
+    // case the other tests already cover.
+    assert_ne!(
+        snapshot, authoritative,
+        "the snapshot already held the full balance; the tail was empty"
+    );
+    assert_projection_matches(&ledger, &[1, 2]).await;
+}
+
 /// Deterministic xorshift64 PRNG so the property test is reproducible.
 struct Rng(u64);
 impl Rng {
@@ -256,11 +322,13 @@ impl Rng {
 /// produce.
 #[tokio::test]
 async fn projection_matches_live_sum_across_random_utxo_history() {
-    // A low interval so reads append cache points throughout the run, exercising
-    // the append-only cache points and closest-at-or-before selection. The
-    // default grace keeps each watermark safely in the past (so the tail folds
-    // every commit and no same-millisecond commit races an append); the invariant
-    // holds regardless of cache-point state.
+    // Default grace (60s) over a sub-second run keeps every watermark before all
+    // commits, so no cache point is appended and each read folds the full tail
+    // (equal to the live sum). That makes the run deterministic and free of
+    // same-millisecond append races; the invariant holds with or without a cache
+    // point. The snapshot-plus-non-empty-tail path (a real snapshot with commits
+    // folded on top) is covered deterministically by
+    // `snapshot_plus_nonempty_tail_matches_live_sum`.
     let ledger = Arc::new(Ledger::new(InMemoryStore::new()).with_snapshot_interval(4));
 
     // Accounts under test, including two subaccounts and both overdraft kinds.

+ 4 - 3
doc/adr/0019-cached-balance-projection.md

@@ -3,9 +3,10 @@
 * Status: accepted
 * Authors: Cesar Rodas
 * Date: 2026-07-17
-* Targeted modules: `kuatia` (`ledger/balance.rs`, commit path, projector),
-  `kuatia-storage` (`Store`), `kuatia-storage-sql` (a new projection + lease
-  table)
+* Targeted modules: `kuatia` (`ledger/balance.rs`, `ledger/projection.rs`),
+  `kuatia-storage` (`Store`), `kuatia-storage-sql` (a new `balance_projection`
+  table). The chosen outcome leaves the commit path untouched and adds no lease
+  table or projector service (see Decision Outcome).
 * Associated tickets/PRs: N/A
 
 ## Context and Problem Statement