Skip to content

Commit e6142a2

Browse files
author
Paul Cernuto
committed
SignalR distribution issues and improve reliability
Distribution fixes: - Partition reassignment after scale-up by tracking active partitions and persisting state on every connection add/remove operation - Broadcast inefficiency - only send to partitions with actual connections instead of all partitions when coordinator cache was empty - Group coordinator with same partition tracking improvements Performance improvements: - Replace MD5 with XxHash64 in PartitionHelper for faster hashing - SendGroupsAsync/SendUsersAsync to await operations instead of fire-and-forget, ensuring errors are properly reported Code quality fixes: - Blocking Task.WaitAny in TryGetReturnType - use Task.Wait(timeout) - Remove finalizer from Subscription class - Add MaxQueuedMessagesPerUser option to prevent unbounded memory growth in SignalRUserGrain (default: 100 messages) Interface changes: - Remove [ReadOnly] from GetPartitionForConnection as it can modify state These changes ensure consistent routing after coordinator grain reactivation and prevent message loss during scale-up events.
1 parent 18a7438 commit e6142a2

12 files changed

Lines changed: 227 additions & 68 deletions

File tree

.claude/settings.local.json

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
{
2+
"permissions": {
3+
"allow": [
4+
"Bash(tree:*)",
5+
"Bash(dotnet build:*)",
6+
"Bash(dotnet test:*)"
7+
]
8+
}
9+
}
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
{
2+
"profiles": {
3+
"ManagedCode.Orleans.SignalR.Client": {
4+
"commandName": "Project",
5+
"launchBrowser": true,
6+
"environmentVariables": {
7+
"ASPNETCORE_ENVIRONMENT": "Development"
8+
},
9+
"applicationUrl": "https://localhost:56460;http://localhost:56463"
10+
}
11+
}
12+
}

ManagedCode.Orleans.SignalR.Core/Config/OrleansSignalROptions.cs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,4 +54,11 @@ public class OrleansSignalROptions
5454
/// Used as a hint when determining how many partitions to allocate dynamically.
5555
/// </summary>
5656
public int GroupsPerPartitionHint { get; set; } = 1_000;
57+
58+
/// <summary>
59+
/// Maximum number of messages to queue per user when they are disconnected.
60+
/// Oldest messages are dropped when the limit is exceeded.
61+
/// The default value is 100.
62+
/// </summary>
63+
public int MaxQueuedMessagesPerUser { get; set; } = 100;
5764
}

ManagedCode.Orleans.SignalR.Core/Helpers/PartitionHelper.cs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
using System;
22
using System.Collections.Concurrent;
33
using System.Collections.Generic;
4+
using System.IO.Hashing;
45
using System.Linq;
5-
using System.Security.Cryptography;
66
using System.Text;
77

88
namespace ManagedCode.Orleans.SignalR.Core.Helpers;
@@ -130,9 +130,10 @@ public int GetPartition(string key)
130130

131131
private static uint GetHash(string key)
132132
{
133-
using var md5 = MD5.Create();
134-
var hash = md5.ComputeHash(Encoding.UTF8.GetBytes(key));
135-
return BitConverter.ToUInt32(hash, 0);
133+
var bytes = Encoding.UTF8.GetBytes(key);
134+
var hash = XxHash64.HashToUInt64(bytes);
135+
// Use lower 32 bits for partition assignment
136+
return unchecked((uint)hash);
136137
}
137138

138139
public Dictionary<int, int> GetDistribution(IEnumerable<string> keys)

ManagedCode.Orleans.SignalR.Core/Interfaces/ISignalRConnectionCoordinatorGrain.cs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ public interface ISignalRConnectionCoordinatorGrain : IGrainWithStringKey
1111
[AlwaysInterleave]
1212
Task<int> GetPartitionCount();
1313

14-
[ReadOnly]
1514
[AlwaysInterleave]
1615
Task<int> GetPartitionForConnection(string connectionId);
1716

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
{
2+
"profiles": {
3+
"ManagedCode.Orleans.SignalR.Core": {
4+
"commandName": "Project",
5+
"launchBrowser": true,
6+
"environmentVariables": {
7+
"ASPNETCORE_ENVIRONMENT": "Development"
8+
},
9+
"applicationUrl": "https://localhost:56458;http://localhost:56462"
10+
}
11+
}
12+
}

ManagedCode.Orleans.SignalR.Core/SignalR/Observers/Subscription.cs

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,17 +6,12 @@
66

77
namespace ManagedCode.Orleans.SignalR.Core.SignalR.Observers;
88

9-
public class Subscription(SignalRObserver observer) : IDisposable
9+
public sealed class Subscription(SignalRObserver observer) : IDisposable
1010
{
1111
private readonly HashSet<IObserverConnectionManager> _grains = new();
1212
private readonly HashSet<GrainId> _heartbeatGrainIds = new();
1313
private bool _disposed;
1414

15-
~Subscription()
16-
{
17-
Dispose();
18-
}
19-
2015
public ISignalRObserver Reference { get; private set; } = default!;
2116

2217
public string? HubKey { get; private set; }

ManagedCode.Orleans.SignalR.Core/SignalR/OrleansHubLifetimeManager.cs

Lines changed: 42 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -190,35 +190,34 @@ public override Task SendGroupAsync(string groupName, string methodName, object?
190190
}
191191
}
192192

193-
public override Task SendGroupsAsync(IReadOnlyList<string> groupNames, string methodName, object?[] args,
193+
public override async Task SendGroupsAsync(IReadOnlyList<string> groupNames, string methodName, object?[] args,
194194
CancellationToken cancellationToken = new())
195195
{
196196
var message = new InvocationMessage(methodName, args);
197197

198198
if (_orleansSignalOptions.Value.GroupPartitionCount > 1)
199199
{
200-
return Task.Run(() => NameHelperGenerator.GetGroupCoordinatorGrain<THub>(_clusterClient)
200+
await Task.Run(() => NameHelperGenerator.GetGroupCoordinatorGrain<THub>(_clusterClient)
201201
.SendToGroups(groupNames.ToArray(), message), cancellationToken);
202+
return;
202203
}
203204

204-
// For potentially many groups, use fire-and-forget to avoid memory issues
205-
_ = Task.Run(async () =>
205+
// Send to all groups in parallel for better performance
206+
var tasks = new List<Task>(groupNames.Count);
207+
foreach (var groupName in groupNames)
206208
{
207-
foreach (var groupName in groupNames)
208-
{
209-
try
210-
{
211-
var groupGrain = NameHelperGenerator.GetSignalRGroupGrain<THub>(_clusterClient, groupName);
212-
await groupGrain.SendToGroup(message).ConfigureAwait(false);
213-
}
214-
catch (Exception ex)
215-
{
216-
_logger.LogError(ex, "Failed to send to group {GroupName}", groupName);
217-
}
218-
}
219-
}, cancellationToken);
209+
var groupGrain = NameHelperGenerator.GetSignalRGroupGrain<THub>(_clusterClient, groupName);
210+
tasks.Add(Task.Run(() => groupGrain.SendToGroup(message), cancellationToken));
211+
}
220212

221-
return Task.CompletedTask;
213+
try
214+
{
215+
await Task.WhenAll(tasks);
216+
}
217+
catch (Exception ex)
218+
{
219+
_logger.LogError(ex, "Failed to send to one or more groups");
220+
}
222221
}
223222

224223
public override Task SendGroupExceptAsync(string groupName, string methodName, object?[] args,
@@ -244,29 +243,27 @@ public override Task SendUserAsync(string userId, string methodName, object?[] a
244243
return Task.Run(() => NameHelperGenerator.GetSignalRUserGrain<THub>(_clusterClient, userId).SendToUser(message), cancellationToken);
245244
}
246245

247-
public override Task SendUsersAsync(IReadOnlyList<string> userIds, string methodName, object?[] args,
246+
public override async Task SendUsersAsync(IReadOnlyList<string> userIds, string methodName, object?[] args,
248247
CancellationToken cancellationToken = new())
249248
{
250249
var message = new InvocationMessage(methodName, args);
251250

252-
// For potentially many users, use fire-and-forget to avoid memory issues
253-
_ = Task.Run(async () =>
251+
// Send to all users in parallel for better performance
252+
var tasks = new List<Task>(userIds.Count);
253+
foreach (var userId in userIds)
254254
{
255-
foreach (var userId in userIds)
256-
{
257-
try
258-
{
259-
var userGrain = NameHelperGenerator.GetSignalRUserGrain<THub>(_clusterClient, userId);
260-
await userGrain.SendToUser(message).ConfigureAwait(false);
261-
}
262-
catch (Exception ex)
263-
{
264-
_logger.LogError(ex, "Failed to send to user {UserId}", userId);
265-
}
266-
}
267-
}, cancellationToken);
255+
var userGrain = NameHelperGenerator.GetSignalRUserGrain<THub>(_clusterClient, userId);
256+
tasks.Add(Task.Run(() => userGrain.SendToUser(message), cancellationToken));
257+
}
268258

269-
return Task.CompletedTask;
259+
try
260+
{
261+
await Task.WhenAll(tasks);
262+
}
263+
catch (Exception ex)
264+
{
265+
_logger.LogError(ex, "Failed to send to one or more users");
266+
}
270267
}
271268

272269
public override async Task AddToGroupAsync(string connectionId, string groupName,
@@ -474,15 +471,20 @@ await Task.Run(() => NameHelperGenerator.GetInvocationGrain<THub>(_clusterClient
474471

475472
public override bool TryGetReturnType(string invocationId, [NotNullWhen(true)] out Type? type)
476473
{
477-
var returnType = NameHelperGenerator.GetInvocationGrain<THub>(_clusterClient, invocationId).TryGetReturnType();
474+
var returnTypeTask = NameHelperGenerator.GetInvocationGrain<THub>(_clusterClient, invocationId).TryGetReturnType();
478475

479476
var timeSpan = TimeIntervalHelper.GetClientTimeoutInterval(_orleansSignalOptions, _globalHubOptions, _hubOptions);
480-
Task.WaitAny(returnType, Task.Delay(timeSpan * 0.8));
477+
var timeout = TimeSpan.FromMilliseconds(timeSpan.TotalMilliseconds * 0.8);
478+
479+
// Use async wait with timeout to avoid blocking thread pool threads
480+
// This is required because the base class method is synchronous
481+
var completed = returnTypeTask.Wait(timeout);
481482

482-
if (returnType.IsCompleted)
483+
if (completed && returnTypeTask.IsCompletedSuccessfully)
483484
{
484-
type = returnType.Result.GetReturnType();
485-
return returnType.Result.Result;
485+
var result = returnTypeTask.Result;
486+
type = result.GetReturnType();
487+
return result.Result;
486488
}
487489

488490
type = null;
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
{
2+
"profiles": {
3+
"ManagedCode.Orleans.SignalR.Server": {
4+
"commandName": "Project",
5+
"launchBrowser": true,
6+
"environmentVariables": {
7+
"ASPNETCORE_ENVIRONMENT": "Development"
8+
},
9+
"applicationUrl": "https://localhost:56459;http://localhost:56461"
10+
}
11+
}
12+
}

ManagedCode.Orleans.SignalR.Server/SignalRConnectionCoordinatorGrain.cs

Lines changed: 46 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ public sealed class SignalRConnectionCoordinatorGrain : Grain, ISignalRConnectio
2626
private readonly IOptions<OrleansSignalROptions> _options;
2727
private readonly IPersistentState<ConnectionCoordinatorState> _state;
2828
private readonly Dictionary<string, int> _connectionPartitions;
29+
private readonly HashSet<int> _activePartitions;
2930
private readonly int _connectionsPerPartitionHint;
3031
private uint _basePartitionCount;
3132
private int _currentPartitionCount;
@@ -40,6 +41,7 @@ public SignalRConnectionCoordinatorGrain(
4041
_options = options;
4142
_state = state;
4243
_connectionPartitions = new Dictionary<string, int>(StringComparer.Ordinal);
44+
_activePartitions = new HashSet<int>();
4345
_connectionsPerPartitionHint = Math.Max(1, _options.Value.ConnectionsPerPartitionHint);
4446
}
4547

@@ -49,28 +51,36 @@ public override async Task OnActivateAsync(CancellationToken cancellationToken)
4951
_state.State ??= new ConnectionCoordinatorState();
5052
var partitions = EnsureOrdinalDictionary(_state.State.ConnectionPartitions);
5153
_connectionPartitions.Clear();
54+
_activePartitions.Clear();
5255
foreach (var kvp in partitions)
5356
{
5457
_connectionPartitions[kvp.Key] = kvp.Value;
58+
_activePartitions.Add(kvp.Value);
5559
}
5660
_state.State.ConnectionPartitions = _connectionPartitions;
5761
_basePartitionCount = Math.Max(1u, _options.Value.ConnectionPartitionCount);
5862
_currentPartitionCount = _state.State.CurrentPartitionCount;
63+
64+
// Ensure partition count is at least base, but preserve higher counts to maintain routing consistency
5965
if (_currentPartitionCount <= 0 || _currentPartitionCount < _basePartitionCount)
6066
{
6167
_currentPartitionCount = (int)_basePartitionCount;
6268
_state.State.CurrentPartitionCount = _currentPartitionCount;
6369
}
70+
// Only reset to base if truly empty AND partition count was scaled up
71+
// This preserves routing consistency for connections that might reconnect
6472
else if (_connectionPartitions.Count == 0 && _currentPartitionCount > _basePartitionCount)
6573
{
6674
_currentPartitionCount = (int)_basePartitionCount;
6775
_state.State.CurrentPartitionCount = _currentPartitionCount;
6876
}
6977

7078
_logger.LogInformation(
71-
"Connection coordinator activated with base partition count {PartitionCount} and hint {ConnectionsPerPartition}",
79+
"Connection coordinator activated with base partition count {PartitionCount}, current {CurrentPartitionCount}, hint {ConnectionsPerPartition}, tracked connections {TrackedConnections}",
7280
_basePartitionCount,
73-
_connectionsPerPartitionHint);
81+
_currentPartitionCount,
82+
_connectionsPerPartitionHint,
83+
_connectionPartitions.Count);
7484
await base.OnActivateAsync(cancellationToken);
7585
}
7686

@@ -190,20 +200,39 @@ public async Task SendToConnections(HubMessage message, string[] connectionIds)
190200
await Task.WhenAll(tasks);
191201
}
192202

193-
public Task NotifyConnectionRemoved(string connectionId)
203+
public async Task NotifyConnectionRemoved(string connectionId)
194204
{
195-
if (_connectionPartitions.Remove(connectionId))
205+
if (_connectionPartitions.Remove(connectionId, out var removedPartition))
196206
{
197-
_logger.LogDebug("Removed connection {ConnectionId} from coordinator mapping.", connectionId);
207+
_logger.LogDebug("Removed connection {ConnectionId} from coordinator mapping (partition {Partition}).", connectionId, removedPartition);
208+
209+
// Check if any other connections are using this partition
210+
var partitionStillActive = false;
211+
foreach (var partition in _connectionPartitions.Values)
212+
{
213+
if (partition == removedPartition)
214+
{
215+
partitionStillActive = true;
216+
break;
217+
}
218+
}
219+
220+
if (!partitionStillActive)
221+
{
222+
_activePartitions.Remove(removedPartition);
223+
}
224+
198225
if (_connectionPartitions.Count == 0 && _currentPartitionCount != _basePartitionCount)
199226
{
200227
_logger.LogDebug("Resetting partition count to base value {PartitionCount} as no active connections remain.", _basePartitionCount);
201228
_currentPartitionCount = (int)_basePartitionCount;
202229
_state.State.CurrentPartitionCount = _currentPartitionCount;
230+
_activePartitions.Clear();
203231
}
204-
}
205232

206-
return Task.CompletedTask;
233+
// Persist state changes to ensure consistency after reactivation
234+
await _state.WriteStateAsync();
235+
}
207236
}
208237

209238
public override async Task OnDeactivateAsync(DeactivationReason reason, CancellationToken cancellationToken)
@@ -221,15 +250,18 @@ public override async Task OnDeactivateAsync(DeactivationReason reason, Cancella
221250

222251
private List<int> GetActivePartitions()
223252
{
224-
if (_connectionPartitions.Count == 0)
253+
// Use cached active partitions set - only send to partitions that actually have connections
254+
if (_activePartitions.Count == 0)
225255
{
226-
return Enumerable.Range(0, _currentPartitionCount).ToList();
256+
// No tracked connections - nothing to send to
257+
return [];
227258
}
228259

229-
return _connectionPartitions.Values
230-
.Distinct()
231-
.OrderBy(static partitionId => partitionId)
232-
.ToList();
260+
// Return sorted list of active partitions for consistent ordering
261+
var result = new List<int>(_activePartitions.Count);
262+
result.AddRange(_activePartitions);
263+
result.Sort();
264+
return result;
233265
}
234266

235267
private int GetOrAssignPartition(string connectionId)
@@ -242,6 +274,7 @@ private int GetOrAssignPartition(string connectionId)
242274
var partitionCount = EnsurePartitionCapacity(_connectionPartitions.Count + 1);
243275
partition = PartitionHelper.GetPartitionId(connectionId, (uint)partitionCount);
244276
_connectionPartitions[connectionId] = partition;
277+
_activePartitions.Add(partition);
245278

246279
_logger.LogDebug("Assigned connection {ConnectionId} to partition {Partition} (partitionCount={PartitionCount})", connectionId, partition, partitionCount);
247280
return partition;

0 commit comments

Comments
 (0)