Skip to content

docs(#3432): the sync/ hub population is the SynchronizationStream population - #3636

Merged
rbuergi merged 2 commits into
mainfrom
docs/3432-the-sync-hub-population-is-the-stream-population
Sep 7, 2026
Merged

rbuergi merged 2 commits into
mainfrom
docs/3432-the-sync-hub-population-is-the-stream-population

Conversation

@rbuergi

@rbuergi rbuergi commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

What holds the 6,925 Started sync/ hubs — narrowed to a named mechanism, from src/ alone

#3432 is a measured population with an unestablished cause. This PR is the narrowing, committed
as Doc/Architecture/SyncHubPopulation. No code changes, no dump taken, no replica touched — the
last attempt on this issue established that dumping a fat replica restarts it, and everything here is
read out of the tree with file citations and checked against the issue's own arithmetic.

1. One of the issue's two open questions is now closed

"What holds them. Unknown. Nothing establishes that the Started half has a single holder, or the
same one."

It does, and it is one field of a stream. SynchronizationStream<T>'s constructor mints exactly one
sync/ hub unconditionally, and one cross-hub subscription builds two streams — the client's
(CreateExternalClient) and the owner's twin (SubscribeToClient → ReduceStream(… WithClientId(request.StreamId))), same ClientId, different parents.

The check that makes this more than a story is the issue's own numbers:

6,925 Started sync/  +  1,495 Dead sync/  =  8,420
SynchronizationStream total               =  8,461     (0.5 % apart)

So there is no independent sync-hub population; the question reduces to "why are there 8,461 live
SynchronizationStreams"
, and ~390 KB/hub is a multiplier on that count.

2. The issue's actual question — missing reaper, or one that does not fire?

Present, and structurally unable to fire on exactly the paths that generate the garbage.

Three things retire a sync/ hub; only the MeshNodeStreamCache idle sweep reaches a Started
hub under a live parent (Dispose() reached 11; the #3427 parent-teardown hook is what produced
the 1,485 Dead). MeshNodeStreamHandle.DetachUpstreams() is reachable from nowhere else, and it
is also the only collector of change-feed-parked predecessors.

Its predicate is !evicted && subscribers == 0 && now - lastActiveAt >= 10 min, and
MeshNodeStreamCacheOptions' own summary names what refreshes it: "Every GetStream subscription,
unsubscription and Update on the path refreshes the window."

🚨 The loop: EvictForPath runs on every change-feed event and parks the entry's stream;
ReclaimIfUnheld declines to dispose it because the cache's hydration is a declared holder for the
entry's whole life
; nothing re-points that hydration ("the sweep only ever CLOSES idle entries; it
never re-subscribes anything"
); the next reader builds a fresh one. Two live streams — four
sync/ hubs — where the design intends one, and the same Update that orphaned the first also
Touch()es the entry and pushes the 10-minute window out.

3. A correction this page exists to record

Workspace.cs warns that counting Rx subscribers "CANNOT answer 'is anyone still reading this' …
never reaches zero — implemented, measured and reverted"
. That is a different, abandoned
mechanism at the workspace layer. MeshNodeStreamCache.Entry.subscribers is a purpose-built refcount
moved only from SharedView, which the internal hydration bypasses — it genuinely can reach
zero
. Reasoning from the Workspace comment alone blames the wrong conjunct, and the next reader
of this issue would have.

4. Deliberately NOT a fix, and what stays open

Whether those 8,461 are garbage at all is still open — thousands of open circuits holding live
layout-area streams is what a healthy portal this size looks like. #3321's history is that two
sessions proposed a fix from a histogram and both were wrong until a referrer walk named the holder,
so this stops at the narrowing.

What it adds is a cheaper discriminator than the one the issue currently turns on. The plan calls
for a ClrMD referrer walk, which needs the dump that restarts the replica. §1's identity makes a
one-line reading decisive, off the dumpheap -stat that plan already takes:

count MeshNodeStreamCache+Entry alongside SynchronizationStream

streams ≈ 2× entries ⇒ the orphan floor dominates, and the fix has a named code path (release the
entry's lease on eviction so ReclaimIfUnheld collects it). ≈ 1× ⇒ the leak framing is
falsified and this is per-hub DI cost — the issue's own third outcome row. ≫ 2× ⇒ look at the
Identity dimension of _remoteStreamCache's key first.

Verification

  • DocumentationLinkIntegrityTest 1/1 (MeshWeaver.Documentation.Test, Release -warnaserror,
    0 Warning(s) / 0 Error(s), exit read directly).
  • Every code claim carries a file citation and was re-read in the tree rather than taken from a
    summary; the two load-bearing ones (the constructor's GetHostedHub, and Update refreshing the
    idle window) are quoted verbatim on the page.
  • Registered in the Architecture index; sibling links use ../.

Pairs-with: none — documentation only, no src/ code touched.

Refs #3432

🤖 Generated with Claude Code

…pulation

A sync/ hub is one field of a SynchronizationStream — the constructor mints one
unconditionally — and one cross-hub subscription builds two streams (client plus
the owner's twin, same ClientId). The dump's own arithmetic agrees: 6,925 Started
+ 1,495 Dead = 8,420 against 8,461 streams. So there is no separate sync-hub
population to explain, and #3432's "what holds them" is answered.

Of the three things that retire one, only the MeshNodeStreamCache idle sweep
reaches a Started hub under a live parent, and DetachUpstreams is reachable from
nowhere else. Its predicate is a conjunction whose window is refreshed by every
Update on the path — while EvictForPath, driven by the change feed that same
Update raises, parks the entry's leased stream that only that sweep can collect.
The act that creates the garbage postpones its only collector.

Narrowing, not a diagnosis: whether those streams are garbage at all is still
open, and the page ends with a discriminator that reads off the histogram the
existing plan already collects — SynchronizationStream count against
MeshNodeStreamCache+Entry count — instead of the referrer walk that needs a dump
which restarts the replica.

Co-Authored-By: Claude Opus 5 (1M context) <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

Documentation-only change with internally consistent citations and correct registration in the architecture index.

Pull request overview

Adds a new architecture documentation page that narrows the observed sync/ hub population (#3432) to SynchronizationStream<T> instances and documents the (code-defined) retirement paths—explicitly positioning the result as a narrowing and leaving the remaining “is this actually garbage?” question open.

Changes:

  • Add Doc/Architecture/SyncHubPopulation, explaining why sync/ hubs are “one per stream” and why a cross-hub subscription yields two.
  • Document the three retirement paths for sync/ hubs and highlight that the MeshNodeStreamCache idle sweep is the only collector for Started hubs under a live parent.
  • Register the new page in the Architecture topic map.
File summaries
File Description
src/MeshWeaver.Documentation/Data/Architecture/SyncHubPopulation.md New architecture page capturing the stream↔hub relationship and the idle-sweep-based retirement constraints, with code citations.
src/MeshWeaver.Documentation/Data/Architecture.md Adds the new page to the “Reading & writing nodes” topic map entry.
Review details
  • Files reviewed: 2/2 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 7, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 4)

1 666 tests   1 472 ✅  2m 5s ⏱️
    4 suites    194 💤
    4 files        0 ❌

Results for commit 8c2eecd.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Sep 7, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 0)

251 tests   251 ✅  2m 17s ⏱️
  1 suites    0 💤
  1 files      0 ❌

Results for commit 8c2eecd.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Sep 7, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 1)

492 tests   492 ✅  38s ⏱️
  1 suites    0 💤
  1 files      0 ❌

Results for commit 8c2eecd.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Sep 7, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 3)

1 061 tests   1 061 ✅  1m 12s ⏱️
    2 suites      0 💤
    2 files        0 ❌

Results for commit 8c2eecd.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Sep 7, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 5)

    5 files      5 suites   2m 39s ⏱️
1 586 tests 1 586 ✅ 0 💤 0 ❌
1 587 runs  1 587 ✅ 0 💤 0 ❌

Results for commit 8c2eecd.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Sep 7, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 2)

1 149 tests   1 149 ✅  4m 19s ⏱️
    3 suites      0 💤
    3 files        0 ❌

Results for commit 8c2eecd.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Sep 7, 2026 •

Copy link
Copy Markdown
Contributor

Test Results

   16 files     16 suites   13m 13s ⏱️
6 205 tests 6 011 ✅ 194 💤 0 ❌
6 206 runs  6 012 ✅ 194 💤 0 ❌

Results for commit 8c2eecd.

♻️ This comment has been updated with latest results.

@meshweaver-cloud
meshweaver-cloud Bot added this pull request to the merge queue Sep 7, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch Sep 7, 2026
…b-population-is-the-stream-population

# Conflicts:
#	src/MeshWeaver.Documentation/Data/Architecture.md
@rbuergi
rbuergi enabled auto-merge September 7, 2026 22:14
@rbuergi
rbuergi added this pull request to the merge queue Sep 7, 2026
Merged via the queue into main with commit a257d77 Sep 7, 2026
30 checks passed
@rbuergi
rbuergi deleted the docs/3432-the-sync-hub-population-is-the-stream-population branch September 13, 2026 10:33
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.

2 participants