Repository navigation
Patch ack by write identity, not emission counting (second acked-write-loss mode) - #635
Merged
Merged
Conversation
…gate + cold-store defer Residual acked-write-loss behind TwoSiloRecycleConvergenceTest (main run 30079395006, shard 2 - recurred AFTER the round-1 pending-save flush fix c43cc20; identical signature to run 30068597014: fast Success ack on the post-recycle write, store frozen at the pre-recycle version, WaitForPersistedBeyond timeout). Root cause (ApplyMeshNodePatchInTurn, MeshWeaver.Data/DataExtensions.cs): 1. The ack watcher was `stream.Skip(1).Take(1)` on the pathless per-node reduced stream - emission COUNTING, the exact pattern PR #584 banned ("write-echo = own stamped Version + target identity, never emission counting"). On a COLD activation (idle-recycle -> reactivate-on-write) the MeshNode init gate opens inside BuildInstanceCollection - on the storage-read emission, BEFORE the loaded collection commits to the primary stream - releasing the held PatchDataRequest into a window where the store is still empty. The first post-subscribe emissions are then the empty-store marker and the initial LOAD echo (the PRE-patch node): Skip(1).Take(1) took the load echo as "committed", IPostCommitFlush.Flush()ed the STALE state and posted PatchDataResponse SUCCESS, while the merge turn no-op'd AckOnce(false, NotFound) against the empty store (suppressed by the AckOnce Interlocked guard). A success-acked write that never existed anywhere - round 1's dispose-flush had nothing to save because nothing was ever committed in-RAM. 2. The merge turn insta-rejected NotFound on an empty store - wrong for a node that EXISTS in storage with its load in flight (the cold window above). Fix: - Write-identity ack: the merge lambda stamps (entityId, minted Version) Volatile/Interlocked BEFORE ApplyChanges; the ack watcher fires only on an emission that provably CONTAINS the write (ChangeContainsStampedWrite: same id, Version at-or-past the stamp). Load echoes, sibling-satellite churn, null drop-markers, and a concurrent writer's earlier commit can no longer ack this patch - restoring the read-after-write contract the flush-chained ack exists for. - Cold-store defer: on an empty collection the merge re-arms ONCE when the primary store first carries data (bounded 10s -> NotFound), so the in-flight initial load is awaited instead of raced. A genuinely absent node still NotFounds - now against the LOADED store. - Cold own-node resolution with satellites present (Count > 1): resolve the target by Path == hub.Address.Path instead of failing the Count==1 shortcut. Deterministic repro: PatchAckWriteIdentityTest scripts the exact interleaving (pre-stamp load echo -> late load echo -> satellite churn -> commit) through the production gate and pins the counterfactual: the old Skip(1).Take(1) shape selects the LOAD ECHO on that sequence. Scenario guard: ColdReactivationPatchReadYourWriteTest drives dispose -> immediate cross-hub Update -> read-your-write through the real pipeline. Verification (all on this worktree, real runs with restore): PatchAckWriteIdentityTest 6/6; ColdReactivationPatchReadYourWriteTest + DisposalPendingSaveFlushTest 2/2; TwoSiloRecycleConvergenceTest 1/1; CrossHubPatchAtomicityTest 1/1; MeshWeaver.Data.Test 287/287; VersionHistoryTest 5/5; WorkspaceCacheEvictionTest 5/5; MeshNodeVersionSyncTest 3/3; CI-exact `dotnet restore` + `dotnet build --no-restore -c Release -p:CIRun=true -warnaserror` clean (0 warnings, 0 errors). Note: the generic (non-MeshNode) deferred patch path retains Skip(1).Take(1); MeshNode never routes there when a primary stream exists, and non-MeshNode reduced streams have no cold load-echo shape - left unchanged deliberately. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012wP3c7DLHutUczcBWgs4h8
This was referenced Jul 25, 2026
This was referenced Aug 25, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Second, independent acked-write-loss mode (survived #627/#628; identical CI signature runs 30068597014 + 30079395006)
ApplyMeshNodePatchInTurn's ack watcher wasstream.Skip(1).Take(1)— emission counting, the pattern #584 banned. On cold reactivation (recycle → write reactivates the hub), the MeshNode init gate releases the heldPatchDataRequestinto an empty-store window; the watcher's "next emission" is the initial-load echo (the PRE-patch node) → it flushed stale state and acked Success, while the merge turn no-op'd against the empty store (suppressed by the AckOnce guard). Success-acked write, existing nowhere — the flush-chained ack fired on the wrong emission, so round 1's flush had nothing to save.Fix
(entityId, minted Version); the watcher acks only on an emission carrying the stamped id at/past that version (ChangeContainsStampedWrite). Load echoes / satellite churn / concurrent earlier commits can no longer ack this patch.Path == hub.Address.Pathwhen satellites are present.Verification (real restored runs)
PatchAckWriteIdentityTest6/6 (scripts the exact interleaving; includes the counterfactual proving the old shape selects the load echo).ColdReactivationPatchReadYourWriteTest+DisposalPendingSaveFlushTest2/2;TwoSiloRecycleConvergenceTest1/1;CrossHubPatchAtomicityTest1/1;MeshWeaver.Data.Testfull 287/287; VersionHistory/WorkspaceCacheEviction/MeshNodeVersionSync 13/13.🤖 Generated with Claude Code