Skip to content

Fix the Sessions LoadTest fixture and the server bottlenecks it exposed - #4749

Merged
marcschier merged 3 commits into
masterfrom
romanett/server-load-test-perf-750c87
Oct 13, 2026
Merged

marcschier merged 3 commits into
masterfrom
romanett/server-load-test-perf-750c87

Conversation

@romanett

@romanett romanett commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

The [Explicit] Sessions LoadTest fixture has not started since #4541 (also on the 2.0.0 tag): it set MaxChannelCount = MaxSessionCount = 10100, and resource isolation refuses to start unless the channel cap covers the sessions plus the bootstrap and reconnect floors plus one. Every LoadTest case failed in OneTimeSetUp. Running the fixture again, and profiling it and the perf/ServerLoadHarness, found several bottlenecks that shipped in 2.0.

Changes

Test

  • MaxChannelCount = MaxSessionCount + 3, the same margin as the framework defaults (100 / 103).
  • Notifications are counted with per-node Interlocked counters. The shared ConcurrentDictionary<NodeId, int>, updated by 750 subscriptions, took 13 % of the process CPU in bucket-lock contention.
  • After the writer stops, and after the last event, the tests wait until the received count stops growing instead of a fixed delay, and log the drain time.

Server / client

  • Detached monitored item recovery ([Server] Add live NodeManager lifecycle support #4094): every node a NodeManager added (AddPredefinedNodeAsync, recursively for children) scanned every monitored item of every subscription for detached items. CreateSubscription adds ~30 diagnostics nodes, so with 450k monitored items subscription creation was quadratic: 69 % of CPU, and CreateSubscription timed out. MonitoredItem now keeps an exact process-wide count of live detached or deleted items, and MasterNodeManager skips recovery while it is zero. All flag changes go through one helper under the item lock; a disposed item does not count, because it cannot be attached again.
  • Client channel gate (Pluggable symmetric cryptography, an awaitable asymmetric path, registrable security policies, and PubSub crypto offboarding (#4206, #4207, #4208, #4210) #4232): UaSCUaBinaryClientChannel.SendRequestAsync entered the gate with a blocking Gate.Enter() inside an async method, and SemaphoreSlim.Wait spun for 19 % of CPU with many requests in flight on one channel. It now awaits Gate.EnterAsync(ct), like the server side.
  • ResourceManager
    • Status code and symbolic id translation looked up their maps under the manager lock and translated while holding it. Sessions that request diagnostics do this for every notification: 83 % of lock contention. The maps are copy-on-write and read without the lock.
    • Translation tables were SortedDictionary<string, string> with the default, culture-sensitive comparer: up to 1 µs per Browse display name, 22 % of Browse server CPU. They now use an ordinal Dictionary.
  • Namespace default permissions: every permission check read DefaultAccessRestrictions / DefaultRolePermissions / DefaultUserRolePermissions of the node's namespace through two server-wide locks and deep-copied the role permission array, per node. That was 99 % of lock contention for Read/Write of 100 values. The values are now read once per namespace per service request and remembered on the OperationContext. Contexts that are not a service request (monitored item and session contexts, which can live long) remember nothing, so a changed default applies to them at once.

Results

perf/ServerLoadHarness, paired A/B against master (3 alternating rounds, medians, 20 sessions, 24 cores). The harness numbers predate the recovery, client gate and status code translation commits, which those scenarios do not exercise:

Scenario Throughput Server CPU / op Server alloc / op Lock contention
Read, 100 values +19 % −28 % −18 % −96 %
Read, 100 values, SignAndEncrypt +27 % −26 % −18 % −98 %
Write, 100 values noise ¹ −19 % −17 % −88 %
Browse (Server object) +3 % −16 % −10 % −2 % ²
Read, 1 value +12 % −4 % ±0 ±0

¹ Throughput varied run to run on the shared machine; per-op CPU and allocation are the reliable signal.
² Browse contention is the Server node's browse lock in NodeState.CreateBrowser (all sessions browse the same node); not changed here.

LoadTest (opc.tcp, Release, in-process client and server):

Case master this PR
ServerReadLoadTestAsync setup failure passes (50M value reads, 0 errors)
ServerManySessionsLoadTestAsync(500) setup failure passes
ServerSubscribeLoadTest_Classic setup failure; on 2026-09-26 (before #4541): CreateSubscription BadRequestTimeout 76–85 % received, fails (needs 99 %)
ServerSubscribeLoadTest_ManagedSessionV2 setup failure; 2026-09-26: 1,885 notifications received 35–45 M notifications, 77–85 % received, fails
ServerEventSubscribeLoadTestAsync setup failure; 2026-09-26: 34.6 % passes: 100 % delivered, drained ~40 s after the last event

The subscribe cases now lose notifications rather than deliver them late: delivery stops ~2 s after the last write. Every monitored item has queue size 1 and the writer is not paced by delivery, so whatever exceeds the publish rate (~585k notifications/s here, SignAndEncrypt with the client in the same process) is overwritten.

The event test lost nothing either: it waited a fixed 2 s after the last event and counted the backlog as lost. It now waits until delivery stops. Its throughput is bound by garbage collection: the event consumer of the Server node spends ~90 % of its wall time suspended for workstation GCs of the shared test process. With DOTNET_gcServer=1 it generates 702 instead of ~210 events/s and drains in 7 s instead of ~40 s, and the subscribe cases reach 87 % (Classic) and 93 % (ManagedSessionV2) at 78–106 M notifications.

Testing

  • Opc.Ua.Server.Tests 8468 passed / 5 skipped, Opc.Ua.Client.Tests 2788 passed, Opc.Ua.Sessions.Tests 851 passed / 379 skipped, Opc.Ua.Core.Tests Tcp/Channel 797 passed (net10.0).
  • New OperationContextDefaultPermissionsCacheTests.
  • Opc.Ua.Server builds for net48; dotnet format whitespace clean.

🤖 Generated with Claude Code

Copilot AI balanced review requested due to automatic review settings October 12, 2026 17:56
The LoadTest fixture set MaxChannelCount equal to MaxSessionCount, which
resource isolation rejects at startup since #4541, so every LoadTest case
failed in OneTimeSetUp. Running it again exposed:

- detached monitored item recovery scanned every monitored item of every
  subscription for each node a NodeManager added, which made
  CreateSubscription quadratic in the monitored item count
- the client request path entered the channel gate with a blocking wait
- status code and symbolic id translation took the ResourceManager lock
  for every notification of sessions that request diagnostics
- translation tables used the culture-sensitive string comparer
- namespace default permissions were read and deep-copied per node

The test counts notifications with per-node interlocked counters and waits
until delivery stops instead of a fixed delay.

Co-Authored-By: Claude Opus 5.5 <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.

🟡 Changes recommended

The permission optimization omits synchronous node managers, and the recovery gate lacks regression coverage.

2 open findings
What changed in this PR

Fixes the Sessions load fixture and optimizes server/client hot paths exposed by load testing.

Changes:

  • Repairs load-test resource limits and notification accounting/draining.
  • Reduces permission, translation, and detached-item recovery overhead.
  • Makes client channel-gate acquisition asynchronous.
File Description
tests/​Opc.Ua.Sessions.Tests/​LoadTest.cs Repairs setup and notification measurement.
tests/​Opc.Ua.Server.Tests/​OperationContextDefaultPermissionsCacheTests.cs Tests permission-cache behavior.
src/​Opc.Ua.Server/​Subscription/​MonitoredItem/​MonitoredItem.cs Tracks recoverable items globally.
src/​Opc.Ua.Server/​Server/​OperationContext.cs Adds per-request permission caching.
src/​Opc.Ua.Server/​NodeManager/​ResourceManager.cs Introduces lock-free translation snapshots.
src/​Opc.Ua.Server/​NodeManager/​MasterNodeManager.cs Skips unnecessary recovery scans.
src/​Opc.Ua.Server/​NodeManager/​AsyncCustomNodeManager.cs Uses cached namespace permissions.
src/​Opc.Ua.Core/​Stack/​Tcp/​UaSCBinaryClientChannel.cs Awaits channel-gate entry.

🧠 Review effort: Balanced


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

Comment thread src/Opc.Ua.Server/NodeManager/AsyncCustomNodeManager.cs
Comment thread src/Opc.Ua.Server/Subscription/MonitoredItem/MonitoredItem.cs
The event test waited 20 publishing intervals after the last event and
counted everything still queued as lost. It now waits until the received
count stops growing and logs the drain time; every event is delivered.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 12, 2026 •

Copy link
Copy Markdown

Code coverage

✅ Coverage gate passed.

Check Result Threshold
✅ Project line rate 88.61% (311594/351645 lines) >= 70.00%
✅ Project branch rate 79.02% >= 60.00%
✅ Patch coverage 97.92% (94/96 changed lines) >= 60.00% (<= 100 changed lines, advisory)
ℹ️ Baseline delta (advisory) +15.01 pp 73.60% recorded
Uncovered changed lines
  • src/Opc.Ua.Server/NodeManager/ResourceManager.cs: 980, 1006

Coverage is above the recorded baseline - consider ratcheting coverage-thresholds.json.

Thresholds live in coverage-thresholds.json. Whole report before exclusions: line 87.76%, branch 78.24%.

@codecov

codecov Bot commented Oct 12, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.80460% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.82%. Comparing base (0dff94b) to head (5fe7cb3).
⚠️ Report is 5 commits behind head on master.

Files with missing lines Patch % Lines
src/Opc.Ua.Server/NodeManager/ResourceManager.cs 80.00% 2 Missing and 3 partials ⚠️
src/Opc.Ua.Server/NodeManager/CustomNodeManager.cs 93.33% 0 Missing and 1 partial ⚠️
src/Opc.Ua.Server/NodeManager/MasterNodeManager.cs 50.00% 0 Missing and 1 partial ⚠️
...Server/Subscription/MonitoredItem/MonitoredItem.cs 92.85% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff            @@
##           master    #4749    +/-   ##
========================================
  Coverage   82.82%   82.82%            
========================================
  Files        2300     2304     +4     
  Lines      351259   351645   +386     
  Branches    61305    61392    +87     
========================================
+ Hits       290928   291251   +323     
- Misses      39857    39880    +23     
- Partials    20474    20514    +40     
Flag Coverage Δ
actions 82.82% <90.80%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...c/Opc.Ua.Core/Stack/Tcp/UaSCBinaryClientChannel.cs 64.59% <100.00%> (ø)
...pc.Ua.Server/NodeManager/AsyncCustomNodeManager.cs 87.92% <100.00%> (-0.13%) ⬇️
src/Opc.Ua.Server/Server/OperationContext.cs 86.66% <100.00%> (+3.80%) ⬆️
src/Opc.Ua.Server/NodeManager/CustomNodeManager.cs 70.84% <93.33%> (+0.28%) ⬆️
src/Opc.Ua.Server/NodeManager/MasterNodeManager.cs 80.67% <50.00%> (-0.16%) ⬇️
...Server/Subscription/MonitoredItem/MonitoredItem.cs 90.51% <92.85%> (+0.06%) ⬆️
src/Opc.Ua.Server/NodeManager/ResourceManager.cs 93.75% <80.00%> (-0.25%) ⬇️

... and 60 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

…ecovery count

- CustomNodeManager2.SetDefaultPermissions reads the namespace defaults once
  per namespace and service request through the same OperationContext cache
  as AsyncCustomNodeManager.
- Regression tests that every detach state transition of a MonitoredItem
  (deleted, detached, rebound, restored, disposed) keeps the recoverable
  count exact.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@romanett
romanett requested a review from marcschier October 13, 2026 03:55
@marcschier
marcschier merged commit 63adef0 into master Oct 13, 2026
113 checks passed
@marcschier
marcschier deleted the romanett/server-load-test-perf-750c87 branch October 13, 2026 04:26
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.

3 participants