Repository navigation
Activation seeds from durable truth — the write-loss family fixed at its root (#590 #648 #667) - #923
Merged
Merged
Conversation
…e prod sighting recorded on #919. ## Root cause — the activation seed raced durable truth and lost `MeshNodeTypeSource.Initialize` merged the durable storage read with the cache-backed routing legs (`Observable.Merge`), and `GetInitialValueAsync` takes the **first** accepted emission then disposes the subscription. On any async backend (PG — the prod portals) the synchronously-replaying stale routing snapshot reliably won the seed and the late durable row was **discarded unobserved**. Every subsequent write minted below durable truth → `MonotonicWriteGuard` refused each save → `AdoptDurableTruth` rebased the owner and **clobbered the acked write in RAM**. That is #590 (updates stop persisting after re-activation), #648 (acked-success loss on a stale second activation) and #667/#919 (deploy-window discards) — and why it never reproduced on in-memory (synchronous reads). ## Fixes - **Seed ordering**: `Merge` → **`Concat`** — the durable read settles first (row / null / fault-degrade), the routing leg follows; activation *holds* for one storage read (#667's "wait, don't Not-found"); version floor unchanged. - **Enrichment graft**: the durable row (authoritative for version/content) is grafted with the routing node's non-persistable `HubConfiguration` delegate — without it, durable-first seeding stripped the delegate and fired spurious activation compiles (caught as 2 suite regressions, both healed). - **InMemory delegate strip**: a durable store cannot hold an in-process delegate (FS/PG strip via serialization; InMemory retained the instance and latched stale overlay configs forever — deterministically reproduced and fixed via `OverlaySelfHealInstanceRecycleTest`). - **#648 invariant 1**: an all-keys-refused three-way merge NACKs **`Conflict`** — never acks Success. - **#648 invariant 2**: a disposing activation NACKs retryable (`OwnerDisposing`) at handler entry; direct-path NACKs now re-enqueue like the late path. - **#667**: new wire code **`OwnerNotReady`** — a cold store's defer-timeout is retryable and provably-not-applied, never a false `NotFound` (which also poisoned the stream cache's negative cache). ## Verification - Acceptance (unmodified): `StaleActivationSeedRollbackTest` **15/15 consecutive** on the final build. Honest note: the claimed 8/15 red on pristine main did **not** reproduce on this machine (in-memory is structurally green) — the deterministic pin below covers the mechanism instead. - New deterministic repros with a gated-read adapter (async-backend stand-in): `StaleActivationDurableFirstSeedTest` — **fail-without confirmed** (the acked write demonstrably destroyed by the refusal→rebase cycle), pass-with green; `SupersededActivationPatchNackTest` (never-Success across all reachable NACK shapes); an all-keys-refused ⇒ Conflict pin; an activation-is-a-read pin. - Suites: Hosting.Monolith **×3 consecutive 482/0** · Data 303 · NodeOperations 85 · Graph 820 · Query 361 · Orleans `TwoSiloRecycleConvergenceTest` 1/1. Independently re-verified **7/7** across the new + acceptance + overlay classes after merging current main. `-c Release -warnaserror` clean per project. ## Residual (documented) Quiescing-NACK branch not directly red/greened locally (Monolith passes Quiescing in <20 ms; matters most on Orleans). Activation now holds for the durable read — on PG a boot storm serializes on the `pg:{adapter}` pool; that is the requested trade. `Take(1)` seed semantics otherwise unchanged. Closes #590 Closes #648 Closes #667 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses a root-cause mechanism behind multiple silent write-loss issues (#590/#648/#667) by ensuring per-node-hub activation seeds from durable storage truth before any cache/routing replays, and by tightening owner-side ACK/NACK semantics so “provably not applied” patches are retried rather than falsely acknowledged.
Changes:
- Fix activation seeding by ordering durable reads ahead of routing/cache emissions (
Merge→Concat) and grafting routing-only, non-persistable enrichment (HubConfiguration) onto the durable seed. - Strengthen cross-hub patch handling: superseded activations NACK early, and all-keys-refused three-way merges NACK
Conflictinstead of ACKing success. - Add deterministic test coverage with a gated async-like storage adapter and new repro/pin tests for the activation/ACK invariants; align InMemory persistence behavior with serializing backends by stripping
HubConfigurationdelegates on write.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/MeshWeaver.Hosting.Monolith.Test/SupersededActivationPatchNackTest.cs | New deterministic pins for “superseded activation must NACK” and “all-keys-refused must not ACK success”. |
| test/MeshWeaver.Hosting.Monolith.Test/StaleActivationDurableFirstSeedTest.cs | New deterministic repro for durable-first activation seeding using a gated async-like read adapter. |
| test/MeshWeaver.Hosting.Monolith.Test/GatedReadStorageAdapter.cs | Test-only storage adapter decorator to force genuinely asynchronous/held reads per path. |
| src/MeshWeaver.Mesh.Contract/MeshNodeStreamExtensions.cs | Client-side re-enqueue logic extended to treat the new retryable owner NACK as safe to retry. |
| src/MeshWeaver.Hosting/Persistence/InMemoryStorageAdapter.cs | Strips non-serializable HubConfiguration delegate on write to match durable backends. |
| src/MeshWeaver.Graph/MeshNodeTypeSource.cs | Durable-first activation seeding (Concat) plus routing enrichment graft to preserve in-process delegate behavior. |
| src/MeshWeaver.Data/DataExtensions.cs | Owner-side patch handling: superseded activation early NACK, refused-merge Conflict NACK, and “store not initialized” NACK changed to retryable. |
| src/MeshWeaver.Data.Contract/MeshNodeError.cs | Introduces OwnerNotReady error code intended to represent “provably not applied; safe to retry”. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+63
to
+74
| OwnerDisposing = 7, | ||
| /// <summary> | ||
| /// The owning activation is up but has NOT finished loading its durable state | ||
| /// (activation seed in flight — a slow storage read, a NodeType assembly reload | ||
| /// window) — the patch was provably NEVER applied. 🚨 Deliberately distinct from | ||
| /// <see cref="NotFound"/>: answering "not found" for a node that exists poisons | ||
| /// every existence check built on the read (and the stream cache's negative | ||
| /// cache) — the #667 deploy-window write loss. Same retry contract as | ||
| /// <see cref="OwnerDisposing"/>: the write never happened, so the mirror may | ||
| /// safely re-enqueue the SAME update once the activation has loaded. | ||
| /// </summary> | ||
| OwnerNotReady = 8 |
Contributor
Test Results (shard 0)672 tests 672 ✅ 5m 0s ⏱️ Results for commit af7836c. |
Contributor
Test Results (shard 5)1 676 tests 1 485 ✅ 6m 1s ⏱️ Results for commit af7836c. |
Contributor
Test Results (shard 4)1 207 tests 1 202 ✅ 5m 33s ⏱️ Results for commit af7836c. |
Contributor
Test Results (shard 2)1 771 tests 1 769 ✅ 5m 45s ⏱️ Results for commit af7836c. |
Contributor
Test Results (shard 3) 11 files 11 suites 5m 59s ⏱️ Results for commit af7836c. |
Contributor
Test Results (shard 1)1 185 tests 1 185 ✅ 8m 30s ⏱️ Results for commit af7836c. |
Contributor
Test Results 65 files 65 suites 36m 50s ⏱️ Results for commit af7836c. |
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.
Fixes #590, #648, #667 — one mechanism behind all three, plus the live prod sighting recorded on #919.
Root cause — the activation seed raced durable truth and lost
MeshNodeTypeSource.Initializemerged the durable storage read with the cache-backed routing legs (Observable.Merge), andGetInitialValueAsynctakes the first accepted emission then disposes the subscription. On any async backend (PG — the prod portals) the synchronously-replaying stale routing snapshot reliably won the seed and the late durable row was discarded unobserved. Every subsequent write minted below durable truth →MonotonicWriteGuardrefused each save →AdoptDurableTruthrebased the owner and clobbered the acked write in RAM. That is #590 (updates stop persisting after re-activation), #648 (acked-success loss on a stale second activation) and #667/#919 (deploy-window discards) — and why it never reproduced on in-memory (synchronous reads).Fixes
Merge→Concat— the durable read settles first (row / null / fault-degrade), the routing leg follows; activation holds for one storage read (A deploy makes custom-NodeType per-node hubs answerNot foundinstead of waiting — in-flight writes are silently discarded #667's "wait, don't Not-found"); version floor unchanged.HubConfigurationdelegate — without it, durable-first seeding stripped the delegate and fired spurious activation compiles (caught as 2 suite regressions, both healed).OverlaySelfHealInstanceRecycleTest).Conflict— never acks Success.OwnerDisposing) at handler entry; direct-path NACKs now re-enqueue like the late path.Not foundinstead of waiting — in-flight writes are silently discarded #667: new wire codeOwnerNotReady— a cold store's defer-timeout is retryable and provably-not-applied, never a falseNotFound(which also poisoned the stream cache's negative cache).Verification
StaleActivationSeedRollbackTest15/15 consecutive on the final build. Honest note: the claimed 8/15 red on pristine main did not reproduce on this machine (in-memory is structurally green) — the deterministic pin below covers the mechanism instead.StaleActivationDurableFirstSeedTest— fail-without confirmed (the acked write demonstrably destroyed by the refusal→rebase cycle), pass-with green;SupersededActivationPatchNackTest(never-Success across all reachable NACK shapes); an all-keys-refused ⇒ Conflict pin; an activation-is-a-read pin.TwoSiloRecycleConvergenceTest1/1. Independently re-verified 7/7 across the new + acceptance + overlay classes after merging current main.-c Release -warnaserrorclean per project.Residual (documented)
Quiescing-NACK branch not directly red/greened locally (Monolith passes Quiescing in <20 ms; matters most on Orleans). Activation now holds for the durable read — on PG a boot storm serializes on the
pg:{adapter}pool; that is the requested trade.Take(1)seed semantics otherwise unchanged.Closes #590
Closes #648
Closes #667
🤖 Generated with Claude Code