Skip to content

fix(mesh): heartbeat is fire-and-forget — stop the callback leak that wedges the cache hub - #152

Merged
rbuergi merged 1 commit into
mainfrom
fix/heartbeat-fire-and-forget
Jul 1, 2026
Merged

rbuergi merged 1 commit into
mainfrom
fix/heartbeat-fire-and-forget

Conversation

@rbuergi

@rbuergi rbuergi commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

The wedge

Reproduced by a documentation crawl (and matching the prod wedge-reproduced-local-k3s-e2e-portal): after ~127 open sync streams, the cache hub accumulates 163 HeartBeatEvent callbacks pending >30 s ([STALE-CALLBACK]) → RoutingGrain TimeoutExceptions → /healthz probe times out (1 s) → pod goes 0/1 → 503s.

Root cause

The sync-stream keep-alive did Post and Observe the HeartBeatEvent. The Observe existed to catch a terminal NotFound and stop heart-beating a dead owner. But a healthy owner never acks a heartbeat, so that Observe registered a hub callback that never resolved — the Rx .Timeout completed the observable but left the underlying callback pending on the cache hub. One leak per heartbeat per live stream → the pile-up that wedges the hub. The heartbeat was waiting for an ack it must never wait for.

Fix — truly fire-and-forget

The code already claimed fire-and-forget (it had added the change-feed resubscribe as "the sole recycled-grain detector") but left the leaky Observe in. This completes that migration:

  • JsonSynchronizationStream — post the heartbeat, don't observe it. No callback, no leak. Keep-alive still works (the Post reaches live owners); recycled owners are re-detected by the change-feed resubscribe; a permanently-gone owner is heart-beaten into routing's ignore path each interval — a benign dropped post, not a storm.
  • RoutingGrain.PostFailureToSender — skip [CanBeIgnored] messages (HeartBeatEvent, Shutdown/Dispose): they're fire-and-forget with no awaiting sender, so a DeliveryFailure/NACK is meaningless and, for a permanently-gone owner, would be the NotFound storm. Makes the Orleans router agree with the monolith RoutingServiceBase, which already guarded [CanBeIgnored]. (The "when not deliverable ⇒ ignore, don't fail the delivery" half.)

Tests

HeartbeatStopsWhenOwnerDiesAfterSubscribeTest pinned the removed observe-and-stop behaviour → renamed to HeartbeatFireAndForgetTest and inverted: after the owner dies (NotFound reply), the fire-and-forget heartbeat ignores the reply and keeps flowing (count climbs) instead of freezing — goes RED if the leaky Observe returns. The storm (never-existent owner) + resubscribe (change-feed) tests are unaffected and still pass. Both changed projects build under Release -warnaserror.

🤖 Generated with Claude Code

… wedge the cache hub

Root cause of the cache-hub wedge (reproduced by a documentation crawl: 163
HeartBeatEvent callbacks pending >30s on the cache hub → RoutingGrain
TimeoutExceptions → /healthz probe times out → pod 0/1 → 503s):

The sync-stream keep-alive did `Post` AND `Observe` the HeartBeatEvent. The
Observe was there to catch a terminal NotFound and stop heart-beating a dead
owner. But a HEALTHY owner never acks a heartbeat, so that Observe registered a
hub callback that never resolved — the Rx `.Timeout` completed the observable
but left the underlying callback pending on the (cache) hub. Across many live
sync streams (e.g. one per open doc page) those leaked callbacks accumulated
until the hub's action block / liveness probe stalled. The heartbeat effectively
waited for an ack it must never wait for.

Fix — make it truly fire-and-forget (the code already claimed to be, and had
already added the change-feed resubscribe as "the sole recycled-grain detector",
but left the leaky Observe in):

- JsonSynchronizationStream: post the heartbeat, do NOT observe it. No callback,
  no leak. Keep-alive still works (the Post reaches live owners); recycled owners
  are re-detected by the existing change-feed resubscribe; a permanently-gone
  owner is heart-beaten into routing's ignore path every interval — a benign
  dropped post, not a storm — until the subscriber hub is itself collected.
- RoutingGrain.PostFailureToSender: skip [CanBeIgnored] messages (HeartBeatEvent,
  Shutdown/Dispose). They are fire-and-forget with no awaiting sender, so a
  DeliveryFailure/NACK for them is meaningless and — for a permanently-gone owner
  heart-beaten every interval — would BE the NotFound storm. This makes the
  Orleans router agree with the monolith RoutingServiceBase, which already
  guarded [CanBeIgnored]. This is the "when not deliverable ⇒ ignore, don't fail
  the delivery" half.

Test: HeartbeatStopsWhenOwnerDiesAfterSubscribeTest pinned the removed
observe-and-stop behaviour; renamed to HeartbeatFireAndForgetTest and inverted —
after the owner dies (NotFound reply), the fire-and-forget heartbeat IGNORES the
reply and keeps flowing (count climbs) instead of freezing. Goes RED if the leaky
Observe is reintroduced. Storm (never-existent owner) + resubscribe tests
unaffected and still pass. Both changed projects build under -warnaserror.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Test Results (shard 3)

1 055 tests  +922   1 055 ✅ +922   5m 36s ⏱️ + 4m 51s
   12 suites + 10       0 💤 ±  0 
   12 files   + 10       0 ❌ ±  0 

Results for commit 4d66f52. ± Comparison against base commit 02e02dd.

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Test Results (shard 1)

923 tests  +100   922 ✅ +100   3m 53s ⏱️ + 1m 12s
 13 suites ±  0     1 💤 ±  0 
 13 files   ±  0     0 ❌ ±  0 

Results for commit 4d66f52. ± Comparison against base commit 02e02dd.

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Test Results (shard 2)

   13 files  ± 0     13 suites  ±0   6m 59s ⏱️ -41s
1 244 tests  - 25  1 241 ✅  - 25  3 💤 ±0  0 ❌ ±0 
1 272 runs   - 25  1 269 ✅  - 25  3 💤 ±0  0 ❌ ±0 

Results for commit 4d66f52. ± Comparison against base commit 02e02dd.

This pull request removes 25 tests.
MeshWeaver.Persistence.Test.DataCubesDocBlocksTest ‑ EveryExecutableBlock_RendersAControl
MeshWeaver.Persistence.Test.DataCubesDocBlocksTest ‑ Page_HasTheExpectedExecutableBlocks
MeshWeaver.Persistence.Test.FileSystemObservableQueryTests ‑ ObserveQuery_CRUDWithMultipleSubscribers_AllReceiveNotifications
MeshWeaver.Persistence.Test.FileSystemObservableQueryTests ‑ ObserveQuery_CreateMultiple_EmitsBatchedNotification
MeshWeaver.Persistence.Test.FileSystemObservableQueryTests ‑ ObserveQuery_Create_EmitsAddedNotification
MeshWeaver.Persistence.Test.FileSystemObservableQueryTests ‑ ObserveQuery_Delete_EmitsRemovedNotification
MeshWeaver.Persistence.Test.FileSystemObservableQueryTests ‑ ObserveQuery_DisposalStopsNotifications
MeshWeaver.Persistence.Test.FileSystemObservableQueryTests ‑ ObserveQuery_FullCRUDCycle_EmitsCorrectNotifications
MeshWeaver.Persistence.Test.FileSystemObservableQueryTests ‑ ObserveQuery_MoveNode_EmitsDeleteAndCreate
MeshWeaver.Persistence.Test.FileSystemObservableQueryTests ‑ ObserveQuery_Read_EmitsInitialResults
…

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Test Results (shard 0)

1 522 tests  +1 522   1 522 ✅ +1 522   3m 23s ⏱️ + 3m 23s
   12 suites +   12       0 💤 ±    0 
   12 files   +   12       0 ❌ ±    0 

Results for commit 4d66f52. ± Comparison against base commit 02e02dd.

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Test Results

   50 files  +   22     50 suites  +22   19m 53s ⏱️ + 8m 45s
4 744 tests +2 519  4 740 ✅ +2 519  4 💤 ±0  0 ❌ ±0 
4 772 runs  +2 519  4 768 ✅ +2 519  4 💤 ±0  0 ❌ ±0 

Results for commit 4d66f52. ± Comparison against base commit 02e02dd.

This pull request removes 25 and adds 2544 tests. Note that renamed tests count towards both.
MeshWeaver.Persistence.Test.DataCubesDocBlocksTest ‑ EveryExecutableBlock_RendersAControl
MeshWeaver.Persistence.Test.DataCubesDocBlocksTest ‑ Page_HasTheExpectedExecutableBlocks
MeshWeaver.Persistence.Test.FileSystemObservableQueryTests ‑ ObserveQuery_CRUDWithMultipleSubscribers_AllReceiveNotifications
MeshWeaver.Persistence.Test.FileSystemObservableQueryTests ‑ ObserveQuery_CreateMultiple_EmitsBatchedNotification
MeshWeaver.Persistence.Test.FileSystemObservableQueryTests ‑ ObserveQuery_Create_EmitsAddedNotification
MeshWeaver.Persistence.Test.FileSystemObservableQueryTests ‑ ObserveQuery_Delete_EmitsRemovedNotification
MeshWeaver.Persistence.Test.FileSystemObservableQueryTests ‑ ObserveQuery_DisposalStopsNotifications
MeshWeaver.Persistence.Test.FileSystemObservableQueryTests ‑ ObserveQuery_FullCRUDCycle_EmitsCorrectNotifications
MeshWeaver.Persistence.Test.FileSystemObservableQueryTests ‑ ObserveQuery_MoveNode_EmitsDeleteAndCreate
MeshWeaver.Persistence.Test.FileSystemObservableQueryTests ‑ ObserveQuery_Read_EmitsInitialResults
…
Memex.Portal.Shared.Test.AgentSkillSyncServiceTest ‑ BaseInstructions_PointToMcp_VectorSearch_AndOnDemandSkillDiscovery
Memex.Portal.Shared.Test.AgentSkillSyncServiceTest ‑ RenderSkillCatalog_ListsAdvertisedInstructionSkills_WithDescriptionAndLoadPath
Memex.Portal.Shared.Test.AgentSkillSyncServiceTest ‑ RenderSkillCatalog_NoAdvertisedSkills_ReturnsEmpty
Memex.Portal.Shared.Test.OnboardingMiddlewareExclusionTest ‑ ExcludedPrefixes_AreCaseInsensitive(path: "/LOGIN")
Memex.Portal.Shared.Test.OnboardingMiddlewareExclusionTest ‑ ExcludedPrefixes_AreCaseInsensitive(path: "/STATIC/img.png")
Memex.Portal.Shared.Test.OnboardingMiddlewareExclusionTest ‑ ExcludedPrefixes_AreCaseInsensitive(path: "/_FRAMEWORK/blazor.js")
Memex.Portal.Shared.Test.OnboardingMiddlewareExclusionTest ‑ ExcludedPrefixes_SkipOnboardingCheck(path: "/_content/MeshWeaver.Blazor/css/app.css")
Memex.Portal.Shared.Test.OnboardingMiddlewareExclusionTest ‑ ExcludedPrefixes_SkipOnboardingCheck(path: "/_framework/blazor.js")
Memex.Portal.Shared.Test.OnboardingMiddlewareExclusionTest ‑ ExcludedPrefixes_SkipOnboardingCheck(path: "/auth/callback")
Memex.Portal.Shared.Test.OnboardingMiddlewareExclusionTest ‑ ExcludedPrefixes_SkipOnboardingCheck(path: "/favicon.ico")
…

@rbuergi
rbuergi merged commit 2ea6880 into main Jul 1, 2026
11 checks passed
@rbuergi
rbuergi deleted the fix/heartbeat-fire-and-forget branch August 5, 2026 10:45
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.

1 participant