Acked-success write loss via stale second per-node activation (MergeGuard refuses keys, flushes stale state, acks Success) #648

Activity

1. rbuergi commented on Jul 27, 2026

   

   rbuergi

   on Jul 27, 2026

   ContributorAuthor

   More actions

   A reproduced instance of this class, with the store dump — and it is a rollback, not just a lost ack

   Chasing TwoSiloRecycleConvergenceTest as a CI flake led straight back here. It is not a flake: an owner grain's idle-recycle durably rolls the node back to its creation state. Captured live at the moment of failure:

   [pre-recycle] mirror at ApiKey=sk-v6, node.Version=12   ← 6 writes acked; store verified >= v12
   [recycle]     disposed 1 owner hosted hub(s) on silo 0
   [PROBE] UpdateApiKeyResilient returned after 93 ms      ← the write reports SUCCESS
   [PROBE] STORE-DUMP present=True version=2 apiKey=sk-v0  ← store is back at the CREATION state
   

   Version=12 / sk-v6 → Version=2 / sk-v0. Six acked writes destroyed. WaitForPersistedBeyond then waits for a version > 12 that can never come — which is why every occurrence is a 30 s timeout at TwoSiloRecycleConvergenceTest.cs:240 (from :172), ~30.8–31.4 s, and never the convergence assertion or a harness gate.

   PR #647 / #635 closed the ack holes; this is the rollback hole, still open.

   Failure identity (trx pulled, not inferred from annotations)

   Run	Signature
   PR #656 attempt 1 (30219296051, shard 2)	TimeoutException @ :240, 31.11 s
   main 30219896359 (shard 2)	same, 30.94 s
   main 30070596702 (shard 2)	same, 30.82 s

   Six sampled passing runs are deterministically ~3.0 s, always v12 → v13. Failures are ~0.9 s of setup plus the full 30 s wait: the write returns fast, the store simply never advances.

   Reproduction

   Needs CPU contention (CI runners have 4 cores). 48 spinner processes + the test in a loop: 1 failure / 11 runs on origin/main. In isolation it passes 10/10, and the whole project passes 3/3 — test/xunit.runner.json sets maxParallelThreads: 1, so a shared-fixture theory is dead.

   Mechanism, as far as it is pinned

   MeshNodeTypeSource enqueues adds into _pendingSaves holding the node as it looked at enqueue time; later updates deliberately bypass that queue, so a queued entry is never refreshed, while ResetDebounceTimer() re-arms the one-shot 200 ms timer on every UpdateImpl. A create-time entry can therefore survive many newer acked writes and be drained by FlushOnDispose at teardown — i.e. exactly at the recycle. v2 is the hub clock at create; sk-v0 is the create content.

   Two invariant repairs for that route are in PR #660 (adds-branch MeshNode.NextVersion floor, which Doc/Architecture/MeshNodeVersioning.md requires and the code omitted; plus flush-current-state). They do not fix it — still 1/40 under load vs 1/11 baseline, not a significant difference (Fisher exact p ≈ 0.36). They are cleanup, offered as such.

   The surviving route, and the systemic gap

   1. A reactivated hub seeds its own node from a cache, not from durable storage. MessageHubGrain.OnActivateAsync builds sourceStream = Observable.Merge(pathResolver.ResolvePath(...), streamCache.GetStream(...)) (MessageHubGrain.cs:245-251) to learn the HubConfiguration, then hands that same stream to the hub as its authoritative own-node data source via WithOwnNodeStream (:377). Both legs are caches — PathResolutionService keeps a positive-only resolved-node cache invalidated only by the per-silo change feed, and MeshNodeStreamCache replays its last value. MeshNodeTypeSource.Initialize (:495-500) prefers that stream over _persistenceCore.Read. So a reactivation can adopt an arbitrarily stale snapshot as live state, and the persistence sampler writes it back. This contradicts Doc/Architecture/MeshNodeVersioning.md, which states the node loads its persisted Version verbatim on activation.

   2. No layer refuses a backward write. InMemoryStorageAdapter.Write and the SaveMeshNode paths overwrite unconditionally; the only write guard anywhere is the "delete wins" tombstone. A monotonic guard would have turned this silent rollback into a loud rejection — and would bound the blast radius of every future race of this shape, not just this one.

   Suggested split: (1) seed own-node from durable storage on activation, using the routing stream only for subsequent live updates and dropping emissions whose Version regresses; (2) a monotonicity guard in the storage layer, independently valuable.

   Also noticed: this test's XML doc still cites MeshDataSource.WithOwnerVersionBaseline / SynchronizationStream.OwnerVersionFloor as the mechanism under test. cc64f0d59 removed both (replaced by a resubscribe accept-latch), so the doc sends readers down a dead end.
2. rbuergi commented on Jul 27, 2026

   

   rbuergi

   on Jul 27, 2026

   ContributorAuthor

   More actions

   The rollback half of this is now fixed on main — PR #665, merged as 1aefc7c93 on a green run.

   Two changes: MeshNodeTypeSource seeds its own node from durable storage (merged with the routing stream, never blocking activation) with a monotonicity gate, and a new MonotonicWriteGuardStorageAdapter refuses any write that would move a node's durable Version backward — verify-before-refuse, logs at Error, returns the stored winner rather than throwing.

   Deterministic proof rather than a load harness: StaleActivationSeedRollbackTest builds the post-recycle state by construction and fails on the pre-fix code with the store reading version 3 against a durable 7000; MonotonicWriteGuardTests' two backward-write cases fail pre-fix (store reads back 2 instead of 12). All pass after.

   Worth noting what unblocked it: the same branch failed CI twice on ThreadAgentIntegrationTest before this landed. That turned out to be a separate production bug — deferred deliveries silently abandoned when a hub disposes mid-activation (PR #672, c118bf472) — i.e. a page that spins forever on any recycle path. With that fixed underneath, this merged clean.

   Leaving the issue open for the maintainer to judge whether the ack half and the remaining hardening (a monotonicity guard pushed down into the per-adapter SQL, rather than a decorator) are in scope here or belong in follow-ups. Two things found en route and deliberately not fixed: AddPartitionStorageHubs services.Replaces the IStorageAdapter binding and would bypass both this guard and the pre-existing VersionWritingStorageAdapter (it has no callers today and PartitionStorageRouter.cs:72 calls it dead code, but the decoration must move with it if it is ever activated), and VersionWritingStorageAdapter silently drops Postgres's batched ReadMany by not overriding it, turning one batched query into N.
3. rbuergi

   closed this as completedin af7836con Aug 8, 2026
4. rbuergi

   added a commit that references this issue on Aug 8, 2026

   Merge pull request #923 from Systemorph/fix/648-activation-durable-fi…

   ...

   23d62b9
5. rbuergi

   added a commit that references this issue on Aug 26, 2026

   Revert the #2305 patch-merge change — it broke two load-bearing invar…

   ...

   469b878