Skip to content

fix(cache): the change-feed reset releases the upstream too — a finished activity stops heart-beating itself awake - #3116

Merged
meshweaver-cloud[bot] merged 1 commit into
mainfrom
fix/3110-change-feed-reset-releases-upstream
Sep 2, 2026
Merged

meshweaver-cloud[bot] merged 1 commit into
mainfrom
fix/3110-change-feed-reset-releases-upstream

Conversation

@rbuergi

@rbuergi rbuergi commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Fixes #3110.

What #3110 actually is

A system HeartBeatEvent from cache/… was still routing to
Crm/Stage/_Activity/compile-202609021024566204c48055472004402b49f4d77fe64bb89
49 minutes after that compile finished Succeeded in about a second — and would have
gone on doing so for the pod's lifetime. The OutOfMemoryException in the recorded stack is
where the exhausted heap surfaced, not the defect.

The issue's own hypothesis — "whatever activates hubs for _Activity nodes does not release
them at terminal status"
— is not what is broken. That release exists and works
(#1435/#1324): ActivityLogAppender.Append fires IMeshNodeStreamCache.ReleaseIfUnwatched on
the terminal write, and NodeTypeCompilationActivity (which owns every compile-<ts> node) is
on that path. What is broken is one layer down, and it is why the release found nothing to
release.

Root cause — the FOURTH faulted-entry eviction drifted from the other three

#1202 established that evicting a faulted cache entry must reach two layers — the cache's
own Entry, and the cacheHub workspace's remote-stream cache holding the
ISynchronizationStream — and centralised that in MeshNodeStreamCache.EvictFaultedEntry,
whose remarks call it "the single teardown behind all three re-probe triggers … so they cannot
drift"
.

There is a fourth trigger. ResetFailureState — the change-feed invalidation, which runs on
every mesh change event for a path, including the activity's own terminal write — kept a
hand-rolled copy that disposed only the Rx hydration and emitted
ReadStreamEviction(path, UpstreamReleased: false, "invalidate") with the flag hard-coded.

Its prose justified that: "a terminally errored upstream has already torn down its own
keep-alive"
. That is true of a NotFound — the one fault class that disposes the stream's
keepAlive, which is what made the claim look right. The class that actually matters is the
transient one the cache documents two screens earlier: an Orleans-idle-collected grain or a
60 s request timeout, where the node exists and is momentarily unreachable. There the stream
stays live and its 45 s heartbeat keeps firing.

Orphan one of those and nothing can ever reap it, because both reclaimers of a warm mirror are
keyed on the cache entry
:

reclaimer how it finds the mirror
the 10-minute idle sweep iterates _streams
ReleaseIfUnwatched (the terminal-activity event, #1435) looks the path up in _streams

Remove the entry and leave the stream attached and both are blind to it. The only escape left is
someone reading that exact path again, so a fresh Entry re-adopts the stream and a later
release disposes it. A finished _Activity/compile-… is never read again — so "never" is the
process lifetime, and every compile storm (100+ in a quarter of an hour on a reimport)
multiplied the set of stale-but-routable activity hubs, each with a periodic heartbeat pointed
at it.

The fix

ResetFailureState calls EvictFaultedEntry(path, "invalidate") — the same gate it applied
inline, plus the two things the copy was missing: the upstream detach + disposal, and the
pair-exact claim that re-parks what it detached on a lost race. Disposal is safe here for the
same reason it is safe at the other three sites, which EvictFaultedEntry already argues: the
detach happens FIRST, so a concurrent read builds a fresh stream, and the UnsubscribeRequest
a disposal posts carries the OLD StreamId.

No timer, no watchdog, no shortened window, no new bound.

Verified

  • New test/MeshWeaver.Hosting.Test/ChangeFeedResetReleasesUpstreamTest.cs. A hijacked owner
    (IRoutingService.RegisterStream, the harness MeshNodeStreamCacheFaultedEntryReprobeTest
    already uses for MeshNodeStreamCache permanently poisons a path after one failed subscribe — same cached TimeoutException replayed forever (memex-cloud, ci.2926) #1202) answers the SubscribeRequest with the verbatim production timeout
    banner, producing the real transient fault — asserted as such via
    IsTransientOwnerFailure / !IsMissingNodeFailure. A change event must then release the
    upstream.
  • The control arm is what makes it non-vacuous: the same faulted-with-live-upstream state,
    released through the already-correct ReleaseIfUnwatched, must report
    UpstreamReleased: true. If the scenario ever stops leaving a stream behind, the control goes
    red rather than the subject going quietly green.
  • Measured both ways: with the one-line change reverted the subject arm fails on exactly that
    assertion while the control arm passes; with it, both pass.
  • dotnet build -c Release -warnaserror, 0 Error(s) each: MeshWeaver.Hosting,
    MeshWeaver.Hosting.Orleans, MeshWeaver.Connection.Orleans, MeshWeaver.Documentation,
    MeshWeaver.Hosting.Test, MeshWeaver.Documentation.Test.
  • Full unfiltered runs: MeshWeaver.Hosting.Test 302/302, MeshWeaver.Documentation.Test
    202/202. (TestTimeoutLiteralRatchetGuard caught three 30.Seconds() literals in the
    first draft — converted to TestTimeouts.Convergence; the baseline is untouched.)

Notes for review

  • test/MeshWeaver.Hosting.Test gains a ProjectReference to
    MeshWeaver.Hosting.Monolith.TestBase: the defect needs a real mesh, and this is the assembly
    MeshWeaver.Hosting already grants InternalsVisibleTo for the cache's eviction seam
    (ReadStreamEvictions / IsReadStreamLive). The cache's own suite moved to
    MeshWeaver.Plugins with the 17 mesh suites (MeshWeaver.AI leaves the platform: carve out the three non-AI surfaces, then move the engine to MeshWeaver.Plugins #2276), so core had no other home for it.
  • That Plugins suite (MeshNodeStreamCacheRecycleResetTest,
    MeshNodeStreamCacheFaultedEntryReprobeTest, MeshNodeStreamCacheIdleReleaseTest,
    CompileActivityHubRetentionTest) covers this path from the other side. Reviewed statically
    against the change: the recycle-reset test's organic arm now disposes an already-dead NotFound
    stream instead of leaving it re-adoptable — the same disposal the storm-breaker branch it
    passes today already performs — and the retention tests exercise seams this diff does not
    touch.
  • A separate finding, deliberately not in this PR: several terminal _Activity writers do
    not go through ActivityLogAppender.Append at all, so they never fire ReleaseIfUnwatched —
    ActivityLogLogger.PublishSnapshotLocked (every kernel/script/markdown/test run — the
    highest-volume one), CodeNodeType.FailActivity, and BuildProtocolDriver.FinishActivity.
    Those leave a warm mirror to the 10-minute idle sweep rather than orphaning it, so they are a
    retention rather than the unbounded leak fixed here. Worth its own issue.

🤖 Generated with Claude Code

…hed activity stops heart-beating itself awake

#3110. A `HeartBeatEvent` from `cache/…` was still routing to
`Crm/Stage/_Activity/compile-…` 49 minutes after that compile finished
`Succeeded` in about a second, and would have gone on for the pod's life.

Root cause — the FOURTH faulted-entry eviction drifted from the other three.
#1202 established that an eviction must reach TWO layers (the cache `Entry`
AND the cacheHub workspace's remote-stream cache) and centralised it in
`EvictFaultedEntry`, whose own remarks call it "the single teardown behind all
three re-probe triggers … so they cannot drift". `ResetFailureState` — the
change-feed invalidation, which runs on EVERY mesh change event for a path —
kept a hand-rolled copy that disposed only the Rx hydration and reported
`ReadStreamEviction(UpstreamReleased: false)` hard-coded.

Its prose justified that with "a terminally errored upstream has already torn
down its own keep-alive". True of a NotFound, which is the one fault class that
disposes the stream's `keepAlive`. The class that matters is the TRANSIENT one
— an idle-collected grain, a 60 s request timeout — where the node EXISTS, the
stream stays live, and its 45 s heartbeat keeps firing. Dropping the entry then
orphans that stream, and BOTH reclaimers of a warm mirror are keyed on the
entry: the idle sweep ITERATES `_streams`, and `ReleaseIfUnwatched` (which
`ActivityLogAppender` fires on the terminal activity write, #1435/#1324) LOOKS
THE PATH UP in it. So nothing could reach the stream again unless that exact
path was read once more — and a finished compile activity never is. Every
compile storm multiplied the set of stale-but-routable activity hubs.

The reset now calls `EvictFaultedEntry` like the other three, which also brings
the pair-exact claim and the re-park-on-lost-race the copy was missing.

- `src/MeshWeaver.Hosting/MeshNodeStreamCache.cs` — the call, and the stale
  remarks replaced with why disposal here is safe.
- `test/MeshWeaver.Hosting.Test/ChangeFeedResetReleasesUpstreamTest.cs` — a
  hijacked owner produces the real transient fault (node exists, owner
  unreachable), then a change event must release the upstream. A CONTROL arm
  releases the same state through the already-correct `ReleaseIfUnwatched` and
  asserts an upstream really was disposed, so the subject arm cannot pass
  vacuously. Fails before / passes after.
- `Doc/Architecture/MeshNodeStreamCache.md` + a What's New entry (Fix).

Verified: `MeshWeaver.Hosting`, `MeshWeaver.Hosting.Orleans`,
`MeshWeaver.Connection.Orleans`, `MeshWeaver.Documentation` and both touched
test projects build `-c Release -warnaserror` with `0 Error(s)`;
`MeshWeaver.Hosting.Test` 302/302 and `MeshWeaver.Documentation.Test` 202/202
green unfiltered; the new test red with the one-line change reverted.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The fix reuses the existing shared eviction teardown to close the leak, and the new test plus docs provide strong coverage for the reported production failure mode.

Pull request overview

Fixes a cache-eviction teardown gap in MeshNodeStreamCache where change-feed invalidation could drop a faulted cache entry without detaching/disposing its upstream sync streams, allowing orphaned keep-alive heartbeats to continue routing to completed activity hubs indefinitely (issue #3110).

Changes:

  • Route change-feed invalidation eviction through the shared EvictFaultedEntry(...) teardown to ensure upstream streams are detached and disposed.
  • Add a regression test that reproduces the “faulted entry + live upstream” transient-owner failure shape and asserts UpstreamReleased: true on change-feed eviction (with a control arm via ReleaseIfUnwatched).
  • Add/extend documentation explaining the fourth eviction trigger and the production symptom/impact.
File summaries
File Description
test/MeshWeaver.Hosting.Test/MeshWeaver.Hosting.Test.csproj Adds monolith test base dependency needed to drive a real mesh for the new cache-eviction test.
test/MeshWeaver.Hosting.Test/ChangeFeedResetReleasesUpstreamTest.cs New regression test pinning that change-feed invalidation releases upstream streams for faulted entries (plus non-vacuous control arm).
src/MeshWeaver.Hosting/MeshNodeStreamCache.cs Updates ResetFailureState to call EvictFaultedEntry(path, "invalidate") so upstreams are detached/disposed consistently.
src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-02-finished-work-stops-keeping-itself-awake.md What’s New entry describing the user-visible fix and impact.
src/MeshWeaver.Documentation/Data/Architecture/MeshNodeStreamCache.md Documentation update: change-feed invalidation is the 4th eviction trigger; explains orphan-heartbeat failure mode.
Review details
  • Files reviewed: 5/5 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Test Results (shard 1)

435 tests   435 ✅  54s ⏱️
  1 suites    0 💤
  1 files      0 ❌

Results for commit 32714b2.

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Test Results (shard 5)

    3 files      3 suites   1m 15s ⏱️
1 228 tests 1 036 ✅ 192 💤 0 ❌
1 229 runs  1 037 ✅ 192 💤 0 ❌

Results for commit 32714b2.

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Test Results (shard 3)

620 tests   620 ✅  49s ⏱️
  2 suites    0 💤
  2 files      0 ❌

Results for commit 32714b2.

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Test Results (shard 4)

1 831 tests   1 831 ✅  2m 9s ⏱️
    3 suites      0 💤
    3 files        0 ❌

Results for commit 32714b2.

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Test Results (shard 0)

234 tests   234 ✅  2m 22s ⏱️
  1 suites    0 💤
  1 files      0 ❌

Results for commit 32714b2.

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Test Results (shard 2)

584 tests   582 ✅  2m 46s ⏱️
  4 suites    2 💤
  4 files      0 ❌

Results for commit 32714b2.

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Test Results

   14 files     14 suites   10m 17s ⏱️
4 932 tests 4 738 ✅ 194 💤 0 ❌
4 933 runs  4 739 ✅ 194 💤 0 ❌

Results for commit 32714b2.

@rbuergi
rbuergi marked this pull request as ready for review September 2, 2026 14:27
@meshweaver-cloud
meshweaver-cloud Bot merged commit bcdc808 into main Sep 2, 2026
29 checks passed
@rbuergi
rbuergi deleted the fix/3110-change-feed-reset-releases-upstream branch September 13, 2026 10:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Compile activity hubs are not retired when the compile finishes — keep-alive heartbeats still route to them and OOM under compile-storm hub churn

2 participants