Repository navigation
MessageService: hot-path Debug logs summarize, never serialize the payload - #608
Merged
Merged
Conversation
…yload The per-delivery Debug logs (start-processing, deserializing) rendered the delivery through LogText — a full JSON serialization of the message payload. LoggingSerializerOptions only strips properties explicitly marked [PreventLogging], so a burst of cross-hub patches carrying fat unmarked payloads serialized the whole node into a log string on EVERY delivery. Proven with CrossHubPatchAtomicityTest run in the full-suite bulk (the way CI shard 0 runs it) with Debug logging enabled: +3.9 GiB managed / ~14k gen-0 GCs inside the single test, 2.7 GB of live log strings in a mid-storm heap dump (69% of the heap), 40 action-block threads inside LogText's serializer, and the test watchdog-killed at 104-113s in 5/5 runs. Whatever test shares the process gets starved too — the 'different victim each run' CI shard flake. Hot-path Debug lines now log a cheap type/id/sender→target/state summary (LogSummary); the full LogText payload render stays on the rare error path where it is worth its cost. With the fix the same bulk run is green, the test takes 16s (was 104-113s), the suite drops 5m45s → 3m22s, and zero memory- pressure watchdog events fire. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NGgWKSgL8pgrbg86xqfa6o
Contributor
There was a problem hiding this comment.
Pull request overview
Updates MessageService hot-path Debug logging to avoid expensive JSON payload serialization on every delivery, reducing allocations/GC pressure during high-throughput message bursts.
Changes:
- Introduces a cheap
LogSummary(IMessageDelivery)helper for per-delivery Debug logs. - Switches “Start processing …” and “Deserializing …” Debug logs from
LogText(...)(JSON serialization) toLogSummary(...). - Keeps full
LogText(...)payload rendering on the exception/error path.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+161
to
+163
| private static string LogSummary(IMessageDelivery delivery) => | ||
| $"{delivery.Message?.GetType().Name ?? "(null)"} (ID: {delivery.Id}, " | ||
| + $"{delivery.Sender} -> {delivery.Target}, {delivery.State})"; |
Contributor
Contributor
Contributor
Contributor
Contributor
Contributor
Contributor
This was referenced Jul 23, 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.
The flake (CI shard 0, 2026-07-21:
ExportImportAccessControlTestwatchdog-killed at 60s; locally:CrossHubPatchAtomicityTestkilled at 104–113s in 5 out of 5 bulk runs)Debugged per the
/debugskill — bulk repro, phase-trace timeline, then a mid-stormdotnet-stack+ 10 GBdotnet-dumpof the xUnit v3 test process:Start processing …,Deserializing …) rendered the delivery viaLogText— a full JSON serialization of the message payload.LoggingSerializerOptionsonly strips properties explicitly marked[PreventLogging]; everything else serializes whole.CrossHubPatchAtomicityTest's 1,152-write cross-hub burst carries a node whose dict grows to 288 entries — the discarded log JSON grows quadratically. Measured inside the one test: +3.9 GiB managed, ~14k gen-0 GCs; the mid-storm heap dump shows 2.7 GB of liveSystem.String(69% of the heap) and 40 action-block threads insideLogText's serializer.Fix
Hot-path Debug lines now log a cheap
LogSummary(type, id, sender → target, state). The fullLogTextpayload render stays on the rare error path, where it's worth its cost.Verification (same bulk run + Debug logging that reproduced 5/5)
CrossHubPatchAtomicityTestMeshWeaver.AI.TestsuiteMEM_CRITICAL6.5 GiBNote: a rarer, distinct silent-stall flake at Warning level (the CI
ExportImportAccessControlTestinstance — flat memory, zero activity, matches theSubscribeRequest-race theMeshNodeStreamCachecomments document) is not addressed here; this PR removes the dominant, proven storm.🤖 Generated with Claude Code
https://claude.ai/code/session_01NGgWKSgL8pgrbg86xqfa6o