Skip to content

Compile discovery: read sources directly instead of through the live synced query - #690

Merged
rbuergi merged 1 commit into
mainfrom
fix/discovery-direct-query
Jul 29, 2026
Merged

rbuergi merged 1 commit into
mainfrom
fix/discovery-direct-query

Conversation

@rbuergi

@rbuergi rbuergi commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

The measurement this is built on

Same box (memex), same data, the exact four queries a Store/Coupon compile issues:

route time
IMeshService.Query — what MCP search uses ~0.25 s (counts 16 / 4 / 1)
the cached synced query — what compiles used 45.20 s

45.20 s is one SyncStreamOptions.HeartbeatInterval to within 200 ms: the synced subscription misses its Initial and idles until the periodic heartbeat re-delivers. Roslyn itself is 2.83 s, so 97% of every compile was that wait — and types then crossed the 60 s settle window, which is how every plugin root came to serve the "did not settle" fallback on 2026-07-27.

A compile needs a point-in-time snapshot. It was taking it from a mechanism whose purpose is live updates — a guarantee it does not need and, on a large mesh, pays 45 s for.

What this changes

Direct read first, synced query kept only as a fallback when the direct read finds nothing. Never the other way round.

#682 kept the cache primary and raced a delayed probe against it. That probe never fired once in production. This ordering removes the dependency on winning a race — the fast path is unconditional.

Everything eliminated by experiment first

This is the part my earlier attempts skipped, and why I trust this one more:

hypothesis verdict
the compile path itself ❌ 0 ms discovery on a fresh mesh (#686)
topology (silo count) ❌ 0 ms with one silo and with two (real AdoNet cluster, both Active)
scale / ~90 schemas / data volume ❌ the same queries answer in 0.25 s on memex
heartbeat schedule (#684) ❌ shipped, measured no change (48 s → 57 s)
pre-warming (#687) ❌ enabled on memex, degraded 7 of 16 roots, rolled back

Testing

647/647 Graph; 8/8 Hosting.Monolith compile + pre-warm integration tests (these drive real Roslyn compiles through this exact path).

Deploy check

After rollout, the verdict is one measurement: a cold Store/Coupon compile should fall from ~48 s to a few seconds, and core #685's phase line will print the discovery figure directly — ⏱ Source discovery took Xms. If that number is still tens of seconds, this is wrong too and should be reverted rather than explained.

🤖 Generated with Claude Code

Copilot AI review requested due to automatic review settings July 28, 2026 06:09

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.

Pull request overview

This PR changes the NodeType compile “source snapshot” path to prefer a direct IMeshService.Query read (point-in-time snapshot) and only fall back to the cached synced query when the direct read yields no sources, addressing long stalls caused by missed Initial emissions in synced-query subscriptions on large meshes.

Changes:

  • Reordered compile source discovery to attempt a direct mesh read first, then fall back to the cached synced source query.
  • Updated the direct-probe helper to accept a configurable subscription delay and adjusted logging/error messaging for the new flow.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +508 to 511
return DirectSourceProbe(ntDef, selfPath, TimeSpan.Zero)
.Concat(Observable.Defer(() => ResolveSources(ntDef, selfPath, null).Take(1)))
.FirstAsync()
.Timeout(bound)
Comment on lines 557 to +561
/// compile never issues it. Failures complete EMPTY rather than erroring — this is a fallback
/// racing a primary, and it must never be the thing that fails a compile.
/// </summary>
private IObservable<IEnumerable<MeshNode>> DirectSourceProbe(
NodeTypeDefinition? ntDef, string selfPath)
NodeTypeDefinition? ntDef, string selfPath, TimeSpan? subscriptionDelay = null)
@github-actions

Copy link
Copy Markdown
Contributor

Test Results (shard 3)

   12 files  ±0     12 suites  ±0   4m 40s ⏱️ +14s
  974 tests ±0    970 ✅ ±0  4 💤 ±0  0 ❌ ±0 
1 037 runs  ±0  1 033 ✅ ±0  4 💤 ±0  0 ❌ ±0 

Results for commit d800d75. ± Comparison against base commit 799cc1f.

@github-actions

Copy link
Copy Markdown
Contributor

Test Results (shard 0)

959 tests  ±0   953 ✅ ±0   5m 26s ⏱️ +19s
  5 suites ±0     6 💤 ±0 
  5 files   ±0     0 ❌ ±0 

Results for commit d800d75. ± Comparison against base commit 799cc1f.

@github-actions

Copy link
Copy Markdown
Contributor

Test Results (shard 2)

1 129 tests  ±0   1 026 ✅ ±0   6m 10s ⏱️ +28s
   10 suites ±0     103 💤 ±0 
   10 files   ±0       0 ❌ ±0 

Results for commit d800d75. ± Comparison against base commit 799cc1f.

@github-actions

Copy link
Copy Markdown
Contributor

Test Results (shard 4)

1 998 tests  ±0   1 810 ✅ ±0   5m 15s ⏱️ +31s
   12 suites ±0     188 💤 ±0 
   12 files   ±0       0 ❌ ±0 

Results for commit d800d75. ± Comparison against base commit 799cc1f.

@github-actions

Copy link
Copy Markdown
Contributor

Test Results (shard 5)

1 142 tests  ±0   1 141 ✅ ±0   6m 47s ⏱️ +8s
   12 suites ±0       1 💤 ±0 
   12 files   ±0       0 ❌ ±0 

Results for commit d800d75. ± Comparison against base commit 799cc1f.

@github-actions

github-actions Bot commented Jul 28, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 1)

781 tests   - 403   781 ✅  - 402   2m 30s ⏱️ - 5m 6s
  9 suites  -   1     0 💤  -   1 
  9 files    -   1     0 ❌ ±  0 

Results for commit d800d75. ± Comparison against base commit 799cc1f.

This pull request removes 403 tests.
MeshWeaver.Hosting.Monolith.Test.AgentChatClientNoSuitableAgentTest ‑ AgentChatClient_OnHostedSubHub_DoesNotReturn_NoSuitableAgent
MeshWeaver.Hosting.Monolith.Test.AgentPickerProjectionPartitionedTest ‑ PartitionedPath_ObserveAgents_FromHostedSubHub_PopulatesCombobox
MeshWeaver.Hosting.Monolith.Test.AgentPickerProjectionPartitionedTest ‑ PartitionedPath_ObserveAgents_PopulatesCombobox
MeshWeaver.Hosting.Monolith.Test.AgentPickerProjectionPartitionedTest ‑ PartitionedPath_ObserveModels_PopulatesCombobox
MeshWeaver.Hosting.Monolith.Test.AgentPickerProjectionTest ‑ ObserveAgents_FromHostedSubHub_PopulatesCombobox
MeshWeaver.Hosting.Monolith.Test.AgentPickerProjectionTest ‑ ObserveAgents_FromMeshHub_PopulatesCombobox
MeshWeaver.Hosting.Monolith.Test.AgentPickerProjectionTest ‑ ObserveAgents_SpaceAndUserSet_SurfacesSpaceAgent_UserAgent_AndPlatform
MeshWeaver.Hosting.Monolith.Test.AgentPickerProjectionTest ‑ ObserveModels_FromHostedSubHub_PopulatesCombobox
MeshWeaver.Hosting.Monolith.Test.AgentPickerProjectionTest ‑ ObserveModels_FromMeshHub_PopulatesCombobox
MeshWeaver.Hosting.Monolith.Test.AgentSyncedQueryFromHostedHubTest ‑ HostedSubHub_GetQuery_AgentDropdownNamesAreNonEmpty
…

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Jul 28, 2026 •

Copy link
Copy Markdown
Contributor

Test Results

   60 files   -   1     60 suites   - 1   30m 51s ⏱️ - 3m 25s
6 983 tests  - 403  6 681 ✅  - 402  302 💤  - 1  0 ❌ ±0 
7 046 runs   - 403  6 744 ✅  - 402  302 💤  - 1  0 ❌ ±0 

Results for commit d800d75. ± Comparison against base commit 799cc1f.

This pull request removes 403 tests.
MeshWeaver.Hosting.Monolith.Test.AgentChatClientNoSuitableAgentTest ‑ AgentChatClient_OnHostedSubHub_DoesNotReturn_NoSuitableAgent
MeshWeaver.Hosting.Monolith.Test.AgentPickerProjectionPartitionedTest ‑ PartitionedPath_ObserveAgents_FromHostedSubHub_PopulatesCombobox
MeshWeaver.Hosting.Monolith.Test.AgentPickerProjectionPartitionedTest ‑ PartitionedPath_ObserveAgents_PopulatesCombobox
MeshWeaver.Hosting.Monolith.Test.AgentPickerProjectionPartitionedTest ‑ PartitionedPath_ObserveModels_PopulatesCombobox
MeshWeaver.Hosting.Monolith.Test.AgentPickerProjectionTest ‑ ObserveAgents_FromHostedSubHub_PopulatesCombobox
MeshWeaver.Hosting.Monolith.Test.AgentPickerProjectionTest ‑ ObserveAgents_FromMeshHub_PopulatesCombobox
MeshWeaver.Hosting.Monolith.Test.AgentPickerProjectionTest ‑ ObserveAgents_SpaceAndUserSet_SurfacesSpaceAgent_UserAgent_AndPlatform
MeshWeaver.Hosting.Monolith.Test.AgentPickerProjectionTest ‑ ObserveModels_FromHostedSubHub_PopulatesCombobox
MeshWeaver.Hosting.Monolith.Test.AgentPickerProjectionTest ‑ ObserveModels_FromMeshHub_PopulatesCombobox
MeshWeaver.Hosting.Monolith.Test.AgentSyncedQueryFromHostedHubTest ‑ HostedSubHub_GetQuery_AgentDropdownNamesAreNonEmpty
…

♻️ This comment has been updated with latest results.

…synced query

A compile needs a POINT-IN-TIME snapshot of its sources. It was taking that snapshot
from the cached SYNCED query, which exists to deliver LIVE updates — a guarantee a
compile does not need and, on a large mesh, pays 45 seconds for.

Measured on memex, same box, same data, same four queries a Store/Coupon compile
issues:

    via IMeshService.Query (what MCP `search` uses)    ~0.25s   counts 16 / 4 / 1
    via the cached synced query (what compiles used)    45.20s

45.20s is one SyncStreamOptions.HeartbeatInterval to within 200ms: the synced
subscription misses its Initial and idles until the periodic heartbeat re-delivers.
Roslyn itself is 2.83s, so 97% of every compile was that wait — and types then
crossed the 60s settle window, which is how every plugin root came to serve the
"did not settle" fallback on 2026-07-27.

Everything else was eliminated by experiment BEFORE writing this, which is the part
the earlier attempts skipped:

  - the compile path itself     0ms discovery on a fresh mesh (issue #686)
  - topology                    0ms with ONE silo and with TWO (real AdoNet cluster,
                                both silos Active) — silo count is not the cause
  - scale / ~90 schemas         the same queries answer in 0.25s ON memex
  - the heartbeat schedule      #684 shipped and measured no change (48s → 57s)
  - pre-warming                 enabled on memex, degraded 7 of 16 roots, rolled back

So: direct read FIRST, synced query kept only as a fallback when the direct read
finds nothing. Never the other way round. #682 kept the cache primary and raced a
delayed probe against it — that probe never fired once in production, and this
ordering is exactly what removes the dependency on winning a race.

647/647 Graph, 8/8 Hosting.Monolith compile/pre-warm integration tests.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@rbuergi
rbuergi force-pushed the fix/discovery-direct-query branch from d800d75 to e004eed Compare July 29, 2026 00:02
@rbuergi
rbuergi merged commit de86b03 into main Jul 29, 2026
1 check passed
@rbuergi
rbuergi deleted the fix/discovery-direct-query branch July 29, 2026 00:05
rbuergi added a commit that referenced this pull request Jul 29, 2026
Second half of unbreaking main. #690 (de86b03) made MeshWeaver.Hosting.Monolith.Test
exceed CI's 6m wall-clock cap (exit=124) on every run since it landed.

CI reports that cap as "likely fixture/init hang". It is not a hang — the suite
completes; it just got slower. Measured on one box, CompileLeafStabilityTest:

    before #690 (cached query primary)     4s
    #690 as merged (probe, then cached)   52s     ← 13×
    this change (true race)                4s

    full suite: 7m50s -> 3m13s (CI cap 6m)

WHY IT GOT SLOWER. #690 chained the direct probe ahead of the synced query with
Concat. Concat is SEQUENTIAL: the synced query is not even subscribed until the
probe COMPLETES. So every compile paid the probe's full latency up front — and the
probe cannot answer faster than SourceProbeQuietWindow, because each leg has to
watch its chunked Initial go quiet before it may emit. That is a ~1s floor per
compile that the previous code never paid: the probe was DelaySubscription'd and on
a healthy mesh never ran at all.

That floor lands on far more calls than "compiles". The disk cache is keyed by
effectiveLastModified, which comes from DiscoverSourceMaxLastModified ->
SnapshotSources — so source discovery runs BEFORE the cache can be consulted, and
a cache HIT pays it too. In a suite that touches the same NodeTypes repeatedly the
cache-hit path is the common case, which is why a 1s floor cost minutes.

Merge starts both sources NOW and FirstAsync takes whichever answers first, so:
  * healthy synced query  -> answers immediately, probe latency never on the path
  * stalled synced query  -> probe answers in a read + quiet window, not 45s
#690's production win is kept; neither source is privileged, so the fast path does
not depend on winning a race (#690's stated objection to #682).

Merging is safe precisely because of #690's own `.Where(count > 0)`: a probe that
finds nothing stays SILENT and completes, so it can never beat the cached query
with an empty set and compile against no sources. That filter is also the likelier
reason #682's probe "never fired" in production — not the 3s delay alone.

NOT done: shrinking SourceProbeQuietWindow to 50ms, which also takes 52s -> 7s.
That window exists so a chunked Initial can settle; the code's own comment warns a
partial source set "compiles WRONG (missing files -> phantom errors)". Nor raising
the 6m cap — the cap did not change, the compile path got 13x slower.

Verified: Hosting.Monolith.Test 419/421 in 3m13s (2 skipped, 0 failed), including
CompileSourceSnapshotWedgeTest which covers the stalled-snapshot path #690 exists
to fix; Graph builds clean under -c Release -warnaserror.

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