Repository navigation
fix(#3432): a read shares one stream instead of minting a permanent sync hub per request - #3952
Conversation
…ub per request
Workspace._localStreamCache caches a plain reduce and deliberately not a configured one,
because a caller-supplied configuration is caller-SPECIFIC (client id, subscriber, init
callback). The read handlers passed a CONSTANT — x => x.ReturnNullWhenNotPresent() — so
every GetDataRequest took the uncached branch, and every uncached reduce constructs a
SynchronizationStream, hence a hosted sync/{id} sub-hub (~390 KB) registered for disposal
on its hub-lifetime parent: one permanent hub per read, released only when the owning node
hub dies.
Measured on memex.meshweaver.cloud 2026-09-10 with a read-only in-process census plus a
controlled experiment: six GetDataRequests against ONE node hub took it from 8 to 14 sync/
hubs (1:1), while the replica gained ~16 sync/ hubs a minute (about 985/h, about 385 MB/h)
across three readings on the same pod and pid. The same census re-scopes the previously
proposed mechanism (evicted UNLEASED streams) to 1 parked stream in 475, and shows the
parked set flat (474/475/475) while the total grew by 120.
The null-when-absent flag is a constant, so it is now part of the cache key and the read
path resolves IWorkspace.GetNullableStream. The population is bounded by DISTINCT
REFERENCES instead of by reads, and the bound is never worse than before: distinct
references <= calls. The interface member carries a default implementation forwarding to
the uncached overload, so an implementer outside this repo is unaffected.
Controls in both directions (test/MeshWeaver.Data.Test/ReadPathStreamMintingTest.cs):
five reads of one reference add 0 sync/ hubs (it fails with +5 on the pre-fix call site,
verified by reverting it); five genuinely-configured reduces still add 5, so the counter
is proven able to see growth; and a write after the reads is visible to the next read, so
the shared stream is a live mirror and not a snapshot.
ReadDuringDisposalWindowTest gains its other half: a read whose stream already exists is
now SERVED from the live mirror inside the disposal window, while a read whose stream
would have to be BUILT there still gets the transient ShuttingDown NACK naming
HubDisposingException. Real data is the opposite of #1470's fabricated absence, so both
halves are pinned and the NACK arm now uses a reference the owner never reduced.
Doc: Doc/Architecture/ReadPathStreamMinting (the census recipe, the decomposition, the
falsification, the experiment, and what is still open).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S25PJZbVeCbkWhCfMub9P7
There was a problem hiding this comment.
🟢 Approval recommended
The caching/keying change is localized and backed by targeted tests plus documentation that ties the fix to a measured production mechanism.
Pull request overview
Fixes the read-path behavior that minted a new SynchronizationStream (and thus a hosted sync/ hub) per GetDataRequest, by introducing a shared “nullable read stream” API and keying the null-when-absent flag into the local stream cache to bound hub population by distinct references rather than by reads.
Changes:
- Add
IWorkspace.GetNullableStream(...)and update read handlers to use it instead of an uncached configured reduce. - Key
Workspace._localStreamCacheby(WorkspaceReference, bool nullReturn)and route nullable reads through the cached path. - Add regression tests and supporting documentation + a “What’s New” fix entry describing the measured impact and the validated mechanism.
File summaries
| File | Description |
|---|---|
| test/MeshWeaver.Data.Test/ReadPathStreamMintingTest.cs | Adds regression coverage proving repeated reads of the same reference don’t mint new sync/ hubs, plus controls validating the counter and liveness. |
| test/MeshWeaver.Data.Test/ReadDuringDisposalWindowTest.cs | Extends teardown-window coverage to reflect the new shared-stream behavior (warmed reference served from live mirror vs. unwarmed reference NACK). |
| src/MeshWeaver.Data/IWorkspace.cs | Introduces GetNullableStream with a default implementation to avoid breaking implementers. |
| src/MeshWeaver.Data/Workspace.cs | Implements cached nullable stream resolution by keying the cache on (reference, nullReturn). |
| src/MeshWeaver.Data/DataExtensions.cs | Switches read paths to resolve streams via GetNullableStream to avoid uncached configured reduces. |
| src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-10-reads-no-longer-leave-a-hub-behind.md | Adds a user-facing fix entry describing the behavioral change and its operational impact. |
| src/MeshWeaver.Documentation/Data/Architecture/SyncHubPopulation.md | Updates architecture context to reflect that the read-path cause was experimentally established. |
| src/MeshWeaver.Documentation/Data/Architecture/ReadPathStreamMinting.md | Adds the detailed measurement + experiment write-up and documents remaining open questions. |
| src/MeshWeaver.Documentation/Data/Architecture/EvictedStreamRetention.md | Re-scopes the earlier hypothesis based on the new live census and cautions against an incorrect remedy. |
Review details
- Files reviewed: 9/9 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| private IObservable<InstanceCollection> ReadObservable( | ||
| IMessageHub client, CollectionReference reference) => | ||
| client.Observe(new GetDataRequest(reference), o => o.WithTarget(CreateHostAddress())) | ||
| .Select(d => (d.Message as GetDataResponse)?.Data as InstanceCollection) | ||
| .Where(c => c is not null) | ||
| .Select(c => c!); |
Test Results (shard 1)500 tests 500 ✅ 43s ⏱️ Results for commit 0ed8b24. ♻️ This comment has been updated with latest results. |
Test Results (shard 0)254 tests 254 ✅ 2m 0s ⏱️ Results for commit 0ed8b24. ♻️ This comment has been updated with latest results. |
Test Results (shard 3)389 tests 389 ✅ 56s ⏱️ Results for commit 0ed8b24. ♻️ This comment has been updated with latest results. |
…al=Pending #3593's hubs are also sync/ and also RunLevel=Started, but read Disposal=Pending with buffer=1: Dispose() was called and the action block never dequeued the ShutdownRequest. Same family, opposite mechanism — so the census asked directly instead of reasoning. Classified every sync/ hub on memex-cloud pod f9lb5 (uptime 123.5 min) by the same fields MessageHub.AppendDiagnostics prints (disposalStarted, DisposalSignalled, MessageService.GetQueueSnapshot): 4648 Started / Disposal=<not started> 1 Starting / Disposal=<not started> withBufferedMessage=0 executingATurn=0 snapshotUnreadable=0 Zero Pending, zero buffered, zero executing, and snapshotUnreadable=0 states the denominator: every one of the 4,649 was actually read. These hubs were never asked to dispose — nothing is wedged and nothing is retained against a teardown in progress. That rules the wedged-pump explanation out for this population rather than leaving it open, and #3593 keeps its own defect and its own fix. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S25PJZbVeCbkWhCfMub9P7
Test Results (shard 4)1 908 tests 1 908 ✅ 3m 33s ⏱️ Results for commit 0ed8b24. ♻️ This comment has been updated with latest results. |
Test Results (shard 2)1 252 tests 1 250 ✅ 3m 53s ⏱️ Results for commit 0ed8b24. ♻️ This comment has been updated with latest results. |
Test Results (shard 5) 6 files 6 suites 4m 55s ⏱️ Results for commit 0ed8b24. ♻️ This comment has been updated with latest results. |
Test Results 17 files 17 suites 16m 3s ⏱️ Results for commit 0ed8b24. ♻️ This comment has been updated with latest results. |
… the topic map CI red on shard 4, both failures caused by this diff and both guards correct: * TestTimeoutLiteralRatchetGuard: 389 vs baseline 386. The new disposal-window arm was written by copying its sibling, and copied three hand-written 30 s bounds with it. Converted THOSE THREE to TestTimeouts.Convergence — the pre-existing three in the other arm are the baseline's and stay untouched, so the ratchet still only goes down. * ArchitectureTopicMapTest: a new Architecture page must be listed in Data/Architecture.md under a theme; a working URL alone does not make it discoverable. Listed beside the two pages it corrects. Re-verified locally: the 8 Documentation guards pass, and ReadDuringDisposalWindowTest + ReadPathStreamMintingTest are 5/5 under the converted bounds. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S25PJZbVeCbkWhCfMub9P7
Closes the cause of #3432 — population MEASURED, cause NOT established — by measurement on a running replica, and fixes it.
What was measured
A read-only in-process census (a
Codenode runs inside the portal process, so it can walk the live hub tree and read the holders' own fields — no heap dump, no 106 s freeze, no replica restart). Onmemex.meshweaver.cloud, pod…-sd7p4, 2026-09-10:Trend, three readings on the same pod and pid: 4,647 (85.9 min) → 4,687 (89.8) → 4,767 (93.2) = ≈ +16 sync hubs/min ≈ +985/h ≈ +385 MB/h. Growing, not a high-water mark — the same order as the ~210 MB/h the issue reported.
What it falsifies
Doc/Architecture/EvictedStreamRetentionproposed that the parked set is filled by unleased streams, "LayoutAreaReferenceis the volume case". Classified live:One in 475 — and the parked count was flat (474/475/475) while the total grew by 120. That mechanism is real and is neither the bulk nor the growth. The 2026-09-09 comment on the issue was right: do NOT change the
ReclaimIfUnheldpredicate.The cause, pinned by experiment
Workspace._localStreamCachecaches a plain reduce and deliberately not a configured one, because a configuration is caller-specific. The read handlers passed a constant —x => x.ReturnNullWhenNotPresent()— so everyGetDataRequesttook the uncached branch, and an uncached reduce constructs aSynchronizationStream, hence a hostedsync/{id}hub (~390 KB) registered for disposal on its hub-lifetime parent.Run in-process against a node hub on the live replica:
The fix
The null-when-absent flag is a constant, so it belongs in the cache key.
_localStreamCacheis keyed(WorkspaceReference, bool NullReturn)and the read path resolvesIWorkspace.GetNullableStream. The population is bounded by distinct references instead of by reads, and the bound is never worse than before: distinct references ≤ calls. The interface member carries a default implementation forwarding to the uncached overload.Controls, both directions
ReadingTheSameReferenceRepeatedly_DoesNotMintASyncHubPerReadAnUncachedConfiguredReduce_MintsOneSyncHubPerCallTheSharedReadStream_StaysLiveReadDuringDisposalWindowTestgains its other half: a read whose stream already exists is now served from the live mirror inside the disposal window; a read whose stream would have to be BUILT there still gets the transientShuttingDownNACK namingHubDisposingException. Real data is the opposite of #1470's fabricated absence, so both halves are pinned.Local:
MeshWeaver.Data.Test500/500,DocumentationLinkIntegrityTestgreen, every touched project-c Release -warnaserrorwith0 Error(s).Implementers: IWorkspace.GetNullableStream — added WITH a default implementation forwarding to the existing GetStream overload, so no implementer must change; the only implementer in the fleet is MeshWeaver.Data.Workspace (checked core and MeshWeaver.Plugins origin/main), which overrides it.
Doc:
Doc/Architecture/ReadPathStreamMinting— the census recipe, the decomposition, the falsification, the experiment, and what is still open (the rest of the residual, the same shape one layer down inContentCollection.GetMarkdown, and the read subscription that is never unsubscribed).🤖 Generated with Claude Code
https://claude.ai/code/session_01S25PJZbVeCbkWhCfMub9P7