[Server] Implement Service Members of IAsyncNodeManager in MasterNodeManager#3204
Conversation
…AsyncNodeManager Refactor MasterNodeManager to allow access to IAsyncNodeManager from NodeManagers & NamespaceManagers Properties
There was a problem hiding this comment.
Pull Request Overview
This PR extends the IAsyncNodeManager interface with new async methods and modernizes the MasterNodeManager to use asynchronous operations throughout. The changes create better async compatibility across the node management system while maintaining backward compatibility through adapter patterns.
Key changes:
- Extended
IAsyncNodeManagerwith core service methods (CreateAddressSpaceAsync, DeleteAddressSpaceAsync, GetNodeMetadataAsync, etc.) - Replaced synchronous operations in
MasterNodeManagerwith async equivalents using ValueTask - Changed from
LocktoSemaphoreSlimfor async-compatible thread safety during startup/shutdown
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| MasterNodeManager.cs | Core implementation converting sync operations to async, replacing lock with SemaphoreSlim, updating data structures to store tuple pairs |
| INodeManager.cs | Extended IAsyncNodeManager interface with new async method signatures |
| AsyncNodeManagerAdapter.cs | Added implementation for new async methods, maintaining backward compatibility |
| StandardServer.cs | Updated to call async startup/shutdown methods with GetAwaiter().GetResult() |
| ServerInternalData.cs | Updated session closing to use async method |
| ContinuationPoint.cs | Changed Manager property type to IAsyncNodeManager |
| Test files | Updated test assertions to handle new tuple data structure in NamespaceManagers |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #3204 +/- ##
==========================================
+ Coverage 57.46% 57.59% +0.13%
==========================================
Files 355 355
Lines 78541 78549 +8
Branches 13855 13849 -6
==========================================
+ Hits 45133 45240 +107
+ Misses 29172 29069 -103
- Partials 4236 4240 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
romanett
changed the title
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Proposed changes
Related Issues
Types of changes
Checklist
TODO
In the next PR the Management methods for MonitoredItems will be updated for async compatiblity.
Once the IAsyncNodeManager interface is complete it will be possible to register a NodeManager that only implements the IAsyncNodeManager and not the INodeManager Interface.
At a later point in time when CustomNodeManager3 (IAsyncNodeManager) is implemented, only IAsyncNodeManager could be exposed from the MasterNodeManager (all existing INodeManagers would be exposed via the AsyncNodeManagerAdapter).