Skip to content

[Server] Refactor MasterNodeManager NamespaceManagers Property to use ConcurrentDictionary for lock free lookups#3190

Merged
marcschier merged 4 commits into
OPCFoundation:masterfrom
romanett:feat/MasterNodeManagerNamespaceManagerLookup
Sep 11, 2025
Merged

[Server] Refactor MasterNodeManager NamespaceManagers Property to use ConcurrentDictionary for lock free lookups#3190
marcschier merged 4 commits into
OPCFoundation:masterfrom
romanett:feat/MasterNodeManagerNamespaceManagerLookup

Conversation

@romanett

@romanett romanett commented Sep 7, 2025

Copy link
Copy Markdown
Contributor

Proposed changes

  • Replaced array-based management of NamespaceManagers with a ConcurrentDictionary<int, IReadOnlyList>
  • Renamed m_readWriterLockSlim to m_namespaceManagersReadWriterLockSlim

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • Enhancement (non-breaking change which adds functionality)
  • Test enhancement (non-breaking change to increase test coverage)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected, requires version increase of Nuget packages)
  • Documentation Update (if none of the other choices apply)

Checklist

  • I have read the CONTRIBUTING doc.
  • I have signed the CLA.
  • I ran tests locally with my changes, all passed.
  • I fixed all failing tests in the CI pipelines.
  • I fixed all introduced issues with CodeQL and LGTM.
  • I have added tests that prove my fix is effective or that my feature works and increased code coverage.
  • I have added necessary documentation (if appropriate).
  • Any dependent changes have been merged and published in downstream modules.

Further comments

@romanett
romanett requested a review from Copilot September 7, 2025 14:42
Copilot AI reviewed Sep 7, 2025
View reviewed changes

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 refactors the MasterNodeManager to replace array-based namespace manager storage with a ConcurrentDictionary for improved thread-safe performance. The change aims to enable lock-free lookups while maintaining thread safety for modifications.

  • Replaces INodeManager[][] array structure with ConcurrentDictionary<int, IReadOnlyList<INodeManager>>
  • Removes read-write lock usage from the critical lookup path in GetManagerHandle method
  • Renames the lock field to be more specific about its purpose

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread Libraries/Opc.Ua.Server/NodeManager/MasterNodeManager.cs Outdated
Comment thread Libraries/Opc.Ua.Server/NodeManager/MasterNodeManager.cs Outdated
Comment thread Libraries/Opc.Ua.Server/NodeManager/MasterNodeManager.cs Outdated
@codecov

codecov Bot commented Sep 7, 2025 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 72.22222% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 57.58%. Comparing base (ca731c3) to head (1f9165d).
⚠️ Report is 4 commits behind head on master.

Files with missing lines Patch % Lines
...ies/Opc.Ua.Server/NodeManager/MasterNodeManager.cs 72.22% 7 Missing and 3 partials ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #3190      +/-   ##
==========================================
- Coverage   58.03%   57.58%   -0.45%     
==========================================
  Files         357      355       -2     
  Lines       78116    78078      -38     
  Branches    13820    13814       -6     
==========================================
- Hits        45335    44963     -372     
- Misses      28607    28872     +265     
- Partials     4174     4243      +69

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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

@romanett romanett changed the title [Server] Refactor MasterNodemManager NamespaceManagers Proerpty to use ConcurrentDictionary for lock free lookups [Server] Refactor MasterNodeManager NamespaceManagers Proerpty to use ConcurrentDictionary for lock free lookups Sep 8, 2025
@romanett romanett changed the title [Server] Refactor MasterNodeManager NamespaceManagers Proerpty to use ConcurrentDictionary for lock free lookups [Server] Refactor MasterNodeManager NamespaceManagers Property to use ConcurrentDictionary for lock free lookups Sep 8, 2025
@romanett
romanett marked this pull request as ready for review September 8, 2025 04:15
@romanett romanett added this to the 1.5.378 Async Server Update 2 milestone Sep 8, 2025
marcschier
marcschier approved these changes Sep 9, 2025
View reviewed changes
Comment thread Libraries/Opc.Ua.Server/NodeManager/MasterNodeManager.cs Outdated
@marcschier
marcschier merged commit 79dbd43 into OPCFoundation:master Sep 11, 2025
76 of 77 checks passed
@romanett
romanett deleted the feat/MasterNodeManagerNamespaceManagerLookup branch September 13, 2025 12:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments
@marcschier marcschier marcschier approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

1.5.378 Async Server Update / Telemet...

Development

Successfully merging this pull request may close these issues.

3 participants

@romanett @marcschier