Delete the Orleans fixture statics that made the assembly un-parallelisable by rbuergi · Pull Request #1010 · Systemorph/MeshWeaver

rbuergi · 2026-08-09T12:42:44Z

Fixes #999

The defect

SharedOrleansFixture kept the test cluster's mesh-node store and its chat-client factory in process-wide statics, and wiped them with ResetSharedState() at the start of every cluster init:

internal static readonly ConcurrentDictionary<string, MeshNode> SharedNodes
internal static readonly ConcurrentDictionary<string, List<object>> SharedPartitionObjects
internal static SwappableChatClientFactory SwappableFactory { get; } = new()

internal static void ResetSharedState()
{
    SharedNodes.Clear();
    SharedPartitionObjects.Clear();
    SwappableFactory.Reset();
}

That wipe was the per-class isolation, and it is exactly why the assembly could not be parallelised: with two classes in flight, B's init calls SharedNodes.Clear() while A is mid-test, deleting A's nodes — the eight No node found at 'TestUser/_Thread/…' failures in the issue. The fixture documented its own precondition: "Safe because classes run strictly sequentially."

AGENTS.md names this shape precisely: "the moment you add a Clear() method 'for test isolation', that method IS the proof of the bug." So the fix is to delete the statics, not to lock them — a lock would re-serialise the very thing we want parallel.

Why they existed, and what replaced them

Orleans instantiates ISiloConfigurator / IHostConfigurator types via new(), so a configurator can never be handed an instance. The issue called that constraint "the real problem to solve".

Most of it dissolves once you notice a silo's DI container is already per-cluster:

The silo's InMemoryStorageAdapter is a singleton of that silo's own container. Per-cluster by construction, disposed with the cluster. Nothing to inject.
Same for the chat factory, once SharedSiloConfigurator registers SwappableChatClientFactory as a type instead of an instance. The silo's container builds it; SharedOrleansFixture.ChatFactory reads it back out.

What was genuinely missing was a way to point the Orleans client at what the silo already owns. TestCluster.InitializeClientAsync offers no hook, but TestClusterHostFactory.CreateClusterClient takes a post-configure Action<IHostBuilder> — a closure. So OrleansTestCluster.DeployAsync sets InitializeClientOnDeploy = false and builds the client host itself, and ShareSiloNodeStore hands it the silo's adapter.

ResetSharedState() and SwappableChatClientFactory.Reset() are deleted, along with the eight try/finally blocks that existed only to put the chat factory back.

Two consequences, both deliberate

1. Cluster.Client is null on these clusters. The client host belongs to OrleansTestClusterHost; callers use ClientServices / ClusterClient. This also closes a teardown gap in passing: TestCluster.DisposeAsync disposes the client through IHost.DisposeAsync, which never runs StopAsync — the new drain always stops the client host first, before the silos, before anything is disposed.

2. The cluster declares ConnectionTransportType.TcpSocket. TestCluster's default transport is InMemory, wired through an InMemoryTransportConnectionHub that is a private field of TestCluster. A client we construct ourselves can never join that hub — it would dial the silos' TCP endpoints and get ConnectionRefused. Real loopback sockets are what TestCluster used before the in-memory transport existed. Measured cost: 131.1s → 139.8s serial for the whole assembly, locally.

TwoSiloCacheUpdateFixture is the one place a silo genuinely needs an injected instance (two silos, one store, no client to route through). It takes TestCluster's only silo-host hook, CreateSiloAsync, with the transport consequence spelled out at the call site — that setter flips ConnectionTransport to TcpSocket because replacing DefaultCreateSiloAsync also drops the in-memory transport wiring.

Parallelism

#979 left this project out, noting it "boots a real cluster per test class, where port collisions are a genuine risk". Port collisions were never the mechanism: the paths in all eight failures are GUID-suffixed, so nothing could collide by name, and Orleans' port allocator already handles concurrent clusters. The statics were.

18-core Mac, Release, native xUnit v3 host, whole assembly:

run	tests	time
serial, before this branch	136	131.1s
serial, after	136	139.8s
parallel ×4	136	91.9s
parallel ×4	136	91.6s
parallel ×4	136	77.9s

All eight tests named in #999 pass in all three parallel runs.

CI then answered the question the local box could not — with more variance. MeshWeaver.Hosting.Orleans.Test on the 4-vCPU ubuntu-latest runner:

	tests	time
main, serial (31313253809)	136	221.7s
this PR, parallel ×4 (31314161999)	136	155.4s
this PR, parallel ×4 (31315000994)	136	197.6s

136/136 green on CI every time — that is the claim #999 actually rests on. The speed-up is not the 1.5× the 18-core box suggested: two parallel samples span 155.4–197.6s against one 221.7s serial sample, i.e. 1.43× on the low sample and 1.12× on the high one. A shared runner has enough variance that no single number here is worth quoting, so read the CI gain as real but modest and under 1.5×.

The shard-assign.sh weight is left at 239. Re-weighting wants more than three samples, and a stale-high weight only under-fills that shard — it never mis-assigns one.

Out of scope

OrleansClusterDisposal.Pending is still a static readonly ConcurrentBag. It is an append-only completion sink drained once at assembly teardown — never read as state, never cleared, and parallel-safe — but it is a static collection. Making it an instance means threading the assembly fixture through ~50 test class constructors, which does not belong in this change.

CI red is not this branch's

Run 31314161999 (7a4909ab) was green on all six shards. The later run 31315000994 (9e8fc86c, which differs only by a comment edit and a restored ILogger argument) is red on shards 2 and 4:

WhatsNewEntryIntegrityTest fails on 2026-08-09-check-a-deployment-is-current.md, an entry that came from main. This branch adds no What's New entry, and main is red on the identical assertion (31314807609).
FutuReAnalysisTest.EuropeRe_LineOfBusiness_Search_ShouldReturn8LoBs misses a MonolithMeshTestBase teardown quiescing budget by 289 ms (2289 ms against 2.00 s), in a different assembly and shard. This branch touches only test/MeshWeaver.Hosting.Orleans.Test/.

The Orleans shard is green in both runs.

What's New

Skipped — test infrastructure only, no user-visible effect.

🤖 Generated with Claude Code

…g could run in parallel SharedOrleansFixture kept the test cluster's in-memory store and its chat-client factory in process-wide statics, and wiped them with ResetSharedState() at the START of every cluster init. That wipe WAS the per-class isolation, and it is what made the assembly un-parallelisable: with two classes in flight, B's init calls SharedNodes.Clear() while A is mid-test, deleting A's nodes — the eight "No node found at 'TestUser/_Thread/...'" failures in #999. The fixture said so itself: "Safe because classes run strictly sequentially". AGENTS.md names this exact shape: a Clear() added for test isolation IS the proof of the bug. The fix is not to lock the statics — that would re-serialise the very thing we want parallel — but to delete them. Why they existed: Orleans instantiates ISiloConfigurator/IHostConfigurator via new(), so a configurator can never be handed an instance, and a static was the only channel the silo and client DI containers could both reach. That constraint has an exit, though. Both InProcessSiloHandle.CreateAsync and TestClusterHostFactory.CreateClusterClient take a post-configure Action<IHostBuilder> — a CLOSURE, which captures whatever the deploying fixture owns. Using it for the client means creating the client host ourselves (InitializeClientOnDeploy = false), so OrleansTestClusterHost now owns it and Cluster.Client is null on these clusters; callers read ClientServices / ClusterClient instead. With the channel in place the state becomes what the rule asks for: OrleansTestBackingStore is an instance owned by the fixture that deployed the cluster and dies with it, and each fixture's SwappableChatClientFactory likewise. Nothing is process-wide, so ResetSharedState() and SwappableChatClientFactory.Reset() are deleted rather than made thread-safe — and the eleven try/finally blocks that existed only to put the chat factory back go with them. Owning the client host also closes a teardown gap in passing: TestCluster.DisposeAsync disposes the client through IHost.DisposeAsync, which never runs StopAsync. The new drain always stops the client host first, before the silos, before anything is disposed. Refs #999 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…nt host honestly The first cut handed both hosts a fixture-owned store through TestClusterBuilder's CreateSiloAsync hook. That was wrong twice over, and the suite said so: - CreateSiloAsync REPLACES TestCluster.DefaultCreateSiloAsync, which is the only thing that wires the default in-memory connection transport — and its setter quietly flips Options.ConnectionTransport to TcpSocket to compensate. A silo hook is therefore never free. - The client I built myself carried no gateway list. TestCluster.InitializeClientAsync computes one from the live silos and adds it as configuration; without it the client starts and then dies with "Could not find any gateway in StaticGatewayListProvider" (8 tests, plus a run that never terminated). Most of the silo hook was unnecessary anyway. A silo's InMemoryStorageAdapter is a singleton of that silo's OWN container: already per-cluster, already disposed with the cluster. The same holds for the chat factory once SharedSiloConfigurator registers SwappableChatClientFactory as a TYPE rather than an instance — the silo's container builds it, and the fixture reads it back out. So no silo needs to be handed anything; only the CLIENT has to be pointed at what the silo already owns, which is what ShareSiloNodeStore does. Owning the client host still costs something, and it is stated rather than discovered: the in-memory transport hub is a private field of TestCluster, so a client we construct cannot join it and would dial the silos' TCP endpoints into ConnectionRefused. The cluster therefore declares TcpSocket. Real loopback sockets — what TestCluster used before the in-memory transport existed — measured at 139.8s against 131.1s for the whole assembly, 136/136 green. TwoSiloCacheUpdateFixture is the one place a SILO genuinely needs an injected instance: two silos, one store, no client to route through. It takes the CreateSiloAsync hook with the transport consequence spelled out at the call site. Refs #999 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

#979 deliberately left this project out, noting it "boots a real cluster per test class, where port collisions are a genuine risk". Port collisions were never the mechanism: the paths in the eight failures are GUID-suffixed, so nothing could collide by name, and Orleans' port allocator has always handled concurrent clusters. What actually broke was the process-wide statics being wiped mid-test, fixed in the preceding commits. Measured on this machine (18 cores), Release, native xUnit v3 host, whole assembly: serial, before this branch 136 tests 0 failed 131.1s serial, after 136 tests 0 failed 139.8s (TCP transport) parallel x4 136 tests 0 failed 91.9s parallel x4 136 tests 0 failed 91.6s parallel x4 136 tests 0 failed 77.9s The eight tests #999 names — Delegation_CreateCellsThenThread_ExecutionStartsAndCompletes, PortalFlow_CreateThread_CreateCells_Submit_ExecutionCompletes, RapidSubmits_PileUpAndAllIngest, AutoExecute_UpdateThreadMessageContent_RoutesToResponseGrain, Delegation_NodeChanges_PropagateFromSubThread, ThreadHub_LocalWorkspaceWrite_VisibleViaGetDataRequest, SubmitChat_FromSidePanel_CellsAppearAndStream, DelegationSubThread_RoutesAfterCreate — pass in all three parallel runs. 1.5x on 18 cores is sub-linear because the cost is per-class cluster boot, which is already multi-core. A 4-vCPU ubuntu-latest runner is therefore NOT predicted by this measurement, and that is what the PR's own CI run has to answer. If Orleans is red or slower there, the right response is to drop this file, not to widen a timeout. Refs #999 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…114160c7129

…back chat factory Both stopped being true: OrleansSharedTestBase builds one per test class, and the factory is now a per-cluster DI singleton that a test never has to restore. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Deleting the wrapper de-indented one method's body by four columns; that method's opening lines were already mis-indented before, so the two anomalies compounded. Formatting only. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…114160c7129

Copilot

Pull request overview

This PR removes process-wide static state from the Orleans test fixtures (and the corresponding “clear at init” isolation mechanism) so MeshWeaver.Hosting.Orleans.Test can safely opt into intra-project parallelism without cross-test interference.

Changes:

Introduces a per-cluster deployment helper (OrleansTestCluster) that builds/owns the Orleans client host (when needed) and enables sharing per-cluster instances via closures instead of statics.
Refactors Orleans test fixtures/bases to use per-cluster DI singletons (and ClientServices / ClusterClient), deleting the reset/restore patterns that depended on global statics.
Opts MeshWeaver.Hosting.Orleans.Test into parallel execution via a project-local xunit.runner.json.

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
test/MeshWeaver.Hosting.Orleans.Test/xunit.runner.json	Enables per-project xUnit parallelism settings for this test project.
test/MeshWeaver.Hosting.Orleans.Test/TwoSiloRecycleConvergenceTest.cs	Updates persisted-version polling to read via the new per-fixture backing store API.
test/MeshWeaver.Hosting.Orleans.Test/TwoSiloCacheUpdateFixture.cs	Replaces process-wide shared dictionaries with an instance-owned backing store and updated cluster deployment.
test/MeshWeaver.Hosting.Orleans.Test/SharedOrleansFixture.cs	Removes shared statics/reset logic; uses `OrleansTestClusterHost` and per-cluster `SwappableChatClientFactory`.
test/MeshWeaver.Hosting.Orleans.Test/OrleansTestSeedProvider.cs	Updates agent description text to match the new per-cluster factory wiring.
test/MeshWeaver.Hosting.Orleans.Test/OrleansTestIdentity.cs	Adds host-based identity seeding for clusters where `Cluster.Client` is intentionally null.
test/MeshWeaver.Hosting.Orleans.Test/OrleansTestCluster.cs	Adds new deployment infrastructure to create/own the Orleans client host and share per-cluster instances without statics.
test/MeshWeaver.Hosting.Orleans.Test/OrleansTestBase.cs	Migrates the per-test cluster base to the new host-owned client model and removes static reset dependency.
test/MeshWeaver.Hosting.Orleans.Test/OrleansResubmitDeadlockTest.cs	Updates documentation to reflect instance-owned fixture state rather than cleared process-wide statics.
test/MeshWeaver.Hosting.Orleans.Test/OrleansPortalFlowTest.cs	Removes per-test `try/finally` reset of the global factory in favor of the per-cluster fixture factory.
test/MeshWeaver.Hosting.Orleans.Test/OrleansNodeChangePropagationTest.cs	Same factory refactor: uses `Fixture.ChatFactory` without global reset.
test/MeshWeaver.Hosting.Orleans.Test/OrleansGrainTeardownStragglerTest.cs	Switches management-grain access to the new `ClusterClient` surface.
test/MeshWeaver.Hosting.Orleans.Test/OrleansDelegationStartTest.cs	Removes global reset and uses the per-cluster chat factory.
test/MeshWeaver.Hosting.Orleans.Test/OrleansClusterDisposal.cs	Adds disposal support for clusters whose client host is owned outside `TestCluster`.
test/MeshWeaver.Hosting.Orleans.Test/OrleansChatHistoryTest.cs	Removes global reset and uses the per-cluster chat factory.
test/MeshWeaver.Hosting.Orleans.Test/OrleansAutoExecuteTest.cs	Removes global reset and uses the per-cluster chat factory.
test/MeshWeaver.Hosting.Orleans.Test/DelegationCompletionTest.cs	Removes global reset and relies on per-cluster factory lifetime semantics.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

…existed Copilot review on #1010. Each host constructs its own InMemoryStorageAdapter over the shared dictionaries, so each has its own Changes Subject — which is the prod shape, not a gap: a prod silo's Subject is per-process too, fed by PG LISTEN/NOTIFY, and a cross-silo signal travels through IMeshChangeFeed. TwoSiloRecycleConvergenceTest delivers that signal explicitly for exactly this reason. The claim predates this branch; it is corrected rather than carried forward. Also restores the adapter's ILogger, dropped when Register() switched to an instance registration — it is what emits the [InMemoryAdapter#id] Read/Write hit/miss lines that make a store-of-record disagreement debuggable. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

github-actions · 2026-08-09T13:02:32Z