From d529db55835f59abc0b84a128e35791c0a65b2eb Mon Sep 17 00:00:00 2001 From: Oleksandr Poliakov Date: Wed, 5 Mar 2025 14:19:37 -0800 Subject: [PATCH 1/4] CSHARP-5496: Reduce locks contention on server selection and connection checkout --- src/MongoDB.Driver/Core/Clusters/Cluster.cs | 42 ++++++------------- .../Core/Jira/CSharp3173Tests.cs | 3 +- .../Core/Jira/CSharp3302Tests.cs | 3 +- 3 files changed, 16 insertions(+), 32 deletions(-) diff --git a/src/MongoDB.Driver/Core/Clusters/Cluster.cs b/src/MongoDB.Driver/Core/Clusters/Cluster.cs index f7a5996c2c5..4e3ecf49eb2 100644 --- a/src/MongoDB.Driver/Core/Clusters/Cluster.cs +++ b/src/MongoDB.Driver/Core/Clusters/Cluster.cs @@ -46,9 +46,7 @@ internal abstract class Cluster : IClusterInternal private readonly TimeSpan _minHeartbeatInterval = __minHeartbeatIntervalDefault; private readonly IClusterClock _clusterClock = new ClusterClock(); private readonly ClusterId _clusterId; - private ClusterDescription _description; - private TaskCompletionSource _descriptionChangedTaskCompletionSource; - private readonly object _descriptionLock = new object(); + private Tuple> _descriptionWithChangedTaskCompletionSource; private readonly LatencyLimitingServerSelector _latencyLimitingServerSelector; protected readonly EventLogger _clusterEventLogger; protected readonly EventLogger _serverSelectionEventLogger; @@ -72,8 +70,7 @@ protected Cluster(ClusterSettings settings, IClusterableServerFactory serverFact _rapidHeartbeatTimerCallbackState = new InterlockedInt32(RapidHeartbeatTimerCallbackState.NotRunning); _clusterId = new ClusterId(); - _description = ClusterDescription.CreateInitial(_clusterId, _settings.DirectConnection); - _descriptionChangedTaskCompletionSource = new TaskCompletionSource(); + _descriptionWithChangedTaskCompletionSource = new(ClusterDescription.CreateInitial(_clusterId, _settings.DirectConnection), new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously)); _latencyLimitingServerSelector = new LatencyLimitingServerSelector(settings.LocalThreshold); _rapidHeartbeatTimer = new Timer(RapidHeartbeatTimerCallback, null, Timeout.InfiniteTimeSpan, Timeout.InfiniteTimeSpan); @@ -97,10 +94,7 @@ public ClusterDescription Description { get { - lock (_descriptionLock) - { - return _description; - } + return _descriptionWithChangedTaskCompletionSource.Item1; } } @@ -134,7 +128,7 @@ protected virtual void Dispose(bool disposing) var newClusterDescription = new ClusterDescription( _clusterId, - _description.DirectConnection, + _descriptionWithChangedTaskCompletionSource.Item1.DirectConnection, dnsMonitorException: null, ClusterType.Unknown, Enumerable.Empty()); @@ -293,22 +287,12 @@ public ICoreSessionHandle StartSession(CoreSessionOptions options) protected void UpdateClusterDescription(ClusterDescription newClusterDescription, bool shouldClusterDescriptionChangedEventBePublished = true) { - ClusterDescription oldClusterDescription = null; - TaskCompletionSource oldDescriptionChangedTaskCompletionSource = null; - - lock (_descriptionLock) - { - oldClusterDescription = _description; - _description = newClusterDescription; + var oldClusterDescription = Interlocked.Exchange(ref _descriptionWithChangedTaskCompletionSource, + Tuple.Create(newClusterDescription, new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously))); - oldDescriptionChangedTaskCompletionSource = _descriptionChangedTaskCompletionSource; - _descriptionChangedTaskCompletionSource = new TaskCompletionSource(); - } - - OnDescriptionChanged(oldClusterDescription, newClusterDescription, shouldClusterDescriptionChangedEventBePublished); + OnDescriptionChanged(oldClusterDescription.Item1, newClusterDescription, shouldClusterDescriptionChangedEventBePublished); - // TODO: use RunContinuationsAsynchronously instead once we require a new enough .NET Framework - Task.Run(() => oldDescriptionChangedTaskCompletionSource.TrySetResult(true)); + oldClusterDescription.Item2.TrySetResult(true); } private string BuildTimeoutExceptionMessage(TimeSpan timeout, IServerSelector selector, ClusterDescription clusterDescription) @@ -380,7 +364,7 @@ public SelectServerHelper(Cluster cluster, IServerSelector selector) { _cluster = cluster; - _connectedServers = new List(_cluster._description?.Servers?.Count ?? 1); + _connectedServers = new List(_cluster._descriptionWithChangedTaskCompletionSource.Item1?.Servers?.Count ?? 1); _connectedServerDescriptions = new List(_connectedServers.Count); _operationCountServerSelector = new OperationsCountServerSelector(_connectedServers); @@ -429,11 +413,9 @@ public void HandleException(Exception exception) public IServer SelectServer() { - lock (_cluster._descriptionLock) - { - _descriptionChangedTask = _cluster._descriptionChangedTaskCompletionSource.Task; - _description = _cluster._description; - } + var clusterDescription = _cluster._descriptionWithChangedTaskCompletionSource; + _descriptionChangedTask = clusterDescription.Item2.Task; + _description = clusterDescription.Item1; if (!_serverSelectionWaitQueueEntered) { diff --git a/tests/MongoDB.Driver.Tests/Core/Jira/CSharp3173Tests.cs b/tests/MongoDB.Driver.Tests/Core/Jira/CSharp3173Tests.cs index 488ed96d5d1..ba5d786f3be 100644 --- a/tests/MongoDB.Driver.Tests/Core/Jira/CSharp3173Tests.cs +++ b/tests/MongoDB.Driver.Tests/Core/Jira/CSharp3173Tests.cs @@ -289,7 +289,8 @@ private IServerSelector CreateWritableServerAndEndPointSelector(EndPoint endPoin private void ForceClusterId(MultiServerCluster cluster, ClusterId clusterId) { Reflector.SetFieldValue(cluster, "_clusterId", clusterId); - Reflector.SetFieldValue(cluster, "_description", ClusterDescription.CreateInitial(clusterId, __directConnection)); + Reflector.SetFieldValue(cluster, "_descriptionWithChangedTaskCompletionSource", + Tuple.Create(ClusterDescription.CreateInitial(clusterId, __directConnection), new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously))); } private void SetupServerMonitorConnection( diff --git a/tests/MongoDB.Driver.Tests/Core/Jira/CSharp3302Tests.cs b/tests/MongoDB.Driver.Tests/Core/Jira/CSharp3302Tests.cs index 624ae10883d..8181f000121 100644 --- a/tests/MongoDB.Driver.Tests/Core/Jira/CSharp3302Tests.cs +++ b/tests/MongoDB.Driver.Tests/Core/Jira/CSharp3302Tests.cs @@ -270,7 +270,8 @@ private IServerSelector CreateWritableServerAndEndPointSelector(EndPoint endPoin private void ForceClusterId(MultiServerCluster cluster, ClusterId clusterId) { Reflector.SetFieldValue(cluster, "_clusterId", clusterId); - Reflector.SetFieldValue(cluster, "_description", ClusterDescription.CreateInitial(clusterId, __directConnection)); + Reflector.SetFieldValue(cluster, "_descriptionWithChangedTaskCompletionSource", + Tuple.Create(ClusterDescription.CreateInitial(clusterId, __directConnection), new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously))); } private void SetupServerMonitorConnection( From 002e9fe1872b84556cb50b974ef623a3e36af037 Mon Sep 17 00:00:00 2001 From: Oleksandr Poliakov Date: Thu, 6 Mar 2025 16:56:33 -0800 Subject: [PATCH 2/4] PR --- src/MongoDB.Driver/Core/Clusters/Cluster.cs | 41 +++++++++++++------ .../Core/Jira/CSharp3173Tests.cs | 3 +- .../Core/Jira/CSharp3302Tests.cs | 3 +- 3 files changed, 31 insertions(+), 16 deletions(-) diff --git a/src/MongoDB.Driver/Core/Clusters/Cluster.cs b/src/MongoDB.Driver/Core/Clusters/Cluster.cs index 4e3ecf49eb2..ed156b53091 100644 --- a/src/MongoDB.Driver/Core/Clusters/Cluster.cs +++ b/src/MongoDB.Driver/Core/Clusters/Cluster.cs @@ -46,7 +46,7 @@ internal abstract class Cluster : IClusterInternal private readonly TimeSpan _minHeartbeatInterval = __minHeartbeatIntervalDefault; private readonly IClusterClock _clusterClock = new ClusterClock(); private readonly ClusterId _clusterId; - private Tuple> _descriptionWithChangedTaskCompletionSource; + private ClusterDescriptionChangeSource _descriptionWithChangedTaskCompletionSource; private readonly LatencyLimitingServerSelector _latencyLimitingServerSelector; protected readonly EventLogger _clusterEventLogger; protected readonly EventLogger _serverSelectionEventLogger; @@ -68,9 +68,8 @@ protected Cluster(ClusterSettings settings, IClusterableServerFactory serverFact Ensure.IsNotNull(eventSubscriber, nameof(eventSubscriber)); _state = new InterlockedInt32(State.Initial); _rapidHeartbeatTimerCallbackState = new InterlockedInt32(RapidHeartbeatTimerCallbackState.NotRunning); - _clusterId = new ClusterId(); - _descriptionWithChangedTaskCompletionSource = new(ClusterDescription.CreateInitial(_clusterId, _settings.DirectConnection), new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously)); + _descriptionWithChangedTaskCompletionSource = new(ClusterDescription.CreateInitial(_clusterId, _settings.DirectConnection)); _latencyLimitingServerSelector = new LatencyLimitingServerSelector(settings.LocalThreshold); _rapidHeartbeatTimer = new Timer(RapidHeartbeatTimerCallback, null, Timeout.InfiniteTimeSpan, Timeout.InfiniteTimeSpan); @@ -94,7 +93,7 @@ public ClusterDescription Description { get { - return _descriptionWithChangedTaskCompletionSource.Item1; + return _descriptionWithChangedTaskCompletionSource.ClusterDescription; } } @@ -128,7 +127,7 @@ protected virtual void Dispose(bool disposing) var newClusterDescription = new ClusterDescription( _clusterId, - _descriptionWithChangedTaskCompletionSource.Item1.DirectConnection, + _descriptionWithChangedTaskCompletionSource.ClusterDescription.DirectConnection, dnsMonitorException: null, ClusterType.Unknown, Enumerable.Empty()); @@ -287,12 +286,11 @@ public ICoreSessionHandle StartSession(CoreSessionOptions options) protected void UpdateClusterDescription(ClusterDescription newClusterDescription, bool shouldClusterDescriptionChangedEventBePublished = true) { - var oldClusterDescription = Interlocked.Exchange(ref _descriptionWithChangedTaskCompletionSource, - Tuple.Create(newClusterDescription, new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously))); + var oldClusterDescription = Interlocked.Exchange(ref _descriptionWithChangedTaskCompletionSource, new(newClusterDescription)); - OnDescriptionChanged(oldClusterDescription.Item1, newClusterDescription, shouldClusterDescriptionChangedEventBePublished); + OnDescriptionChanged(oldClusterDescription.ClusterDescription, newClusterDescription, shouldClusterDescriptionChangedEventBePublished); - oldClusterDescription.Item2.TrySetResult(true); + oldClusterDescription.TrySetChanged(); } private string BuildTimeoutExceptionMessage(TimeSpan timeout, IServerSelector selector, ClusterDescription clusterDescription) @@ -347,6 +345,25 @@ private void ThrowTimeoutException(IServerSelector selector, ClusterDescription } // nested classes + internal class ClusterDescriptionChangeSource + { + private readonly TaskCompletionSource _changedTaskCompletionSource; + private readonly ClusterDescription _clusterDescription; + + public ClusterDescriptionChangeSource(ClusterDescription clusterDescription) + { + _changedTaskCompletionSource = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + _clusterDescription = clusterDescription; + } + + public ClusterDescription ClusterDescription => _clusterDescription; + + public Task Changed => _changedTaskCompletionSource.Task; + + public bool TrySetChanged() + => _changedTaskCompletionSource.TrySetResult(true); + } + private class SelectServerHelper : IDisposable { private readonly Cluster _cluster; @@ -364,7 +381,7 @@ public SelectServerHelper(Cluster cluster, IServerSelector selector) { _cluster = cluster; - _connectedServers = new List(_cluster._descriptionWithChangedTaskCompletionSource.Item1?.Servers?.Count ?? 1); + _connectedServers = new List(_cluster._descriptionWithChangedTaskCompletionSource.ClusterDescription?.Servers?.Count ?? 1); _connectedServerDescriptions = new List(_connectedServers.Count); _operationCountServerSelector = new OperationsCountServerSelector(_connectedServers); @@ -414,8 +431,8 @@ public void HandleException(Exception exception) public IServer SelectServer() { var clusterDescription = _cluster._descriptionWithChangedTaskCompletionSource; - _descriptionChangedTask = clusterDescription.Item2.Task; - _description = clusterDescription.Item1; + _descriptionChangedTask = clusterDescription.Changed; + _description = clusterDescription.ClusterDescription; if (!_serverSelectionWaitQueueEntered) { diff --git a/tests/MongoDB.Driver.Tests/Core/Jira/CSharp3173Tests.cs b/tests/MongoDB.Driver.Tests/Core/Jira/CSharp3173Tests.cs index ba5d786f3be..bdf4438a008 100644 --- a/tests/MongoDB.Driver.Tests/Core/Jira/CSharp3173Tests.cs +++ b/tests/MongoDB.Driver.Tests/Core/Jira/CSharp3173Tests.cs @@ -289,8 +289,7 @@ private IServerSelector CreateWritableServerAndEndPointSelector(EndPoint endPoin private void ForceClusterId(MultiServerCluster cluster, ClusterId clusterId) { Reflector.SetFieldValue(cluster, "_clusterId", clusterId); - Reflector.SetFieldValue(cluster, "_descriptionWithChangedTaskCompletionSource", - Tuple.Create(ClusterDescription.CreateInitial(clusterId, __directConnection), new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously))); + Reflector.SetFieldValue(cluster, "_descriptionWithChangedTaskCompletionSource", new Cluster.ClusterDescriptionChangeSource(ClusterDescription.CreateInitial(clusterId, __directConnection))); } private void SetupServerMonitorConnection( diff --git a/tests/MongoDB.Driver.Tests/Core/Jira/CSharp3302Tests.cs b/tests/MongoDB.Driver.Tests/Core/Jira/CSharp3302Tests.cs index 8181f000121..2a2a3f024d2 100644 --- a/tests/MongoDB.Driver.Tests/Core/Jira/CSharp3302Tests.cs +++ b/tests/MongoDB.Driver.Tests/Core/Jira/CSharp3302Tests.cs @@ -270,8 +270,7 @@ private IServerSelector CreateWritableServerAndEndPointSelector(EndPoint endPoin private void ForceClusterId(MultiServerCluster cluster, ClusterId clusterId) { Reflector.SetFieldValue(cluster, "_clusterId", clusterId); - Reflector.SetFieldValue(cluster, "_descriptionWithChangedTaskCompletionSource", - Tuple.Create(ClusterDescription.CreateInitial(clusterId, __directConnection), new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously))); + Reflector.SetFieldValue(cluster, "_descriptionWithChangedTaskCompletionSource", new Cluster.ClusterDescriptionChangeSource(ClusterDescription.CreateInitial(clusterId, __directConnection))); } private void SetupServerMonitorConnection( From 2e8a7dd9fa0787fe765b147410b3196f9acddb47 Mon Sep 17 00:00:00 2001 From: Oleksandr Poliakov Date: Thu, 6 Mar 2025 17:20:47 -0800 Subject: [PATCH 3/4] Fix formatting --- src/MongoDB.Driver/Core/Clusters/Cluster.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/MongoDB.Driver/Core/Clusters/Cluster.cs b/src/MongoDB.Driver/Core/Clusters/Cluster.cs index ed156b53091..29f198c00fc 100644 --- a/src/MongoDB.Driver/Core/Clusters/Cluster.cs +++ b/src/MongoDB.Driver/Core/Clusters/Cluster.cs @@ -69,7 +69,7 @@ protected Cluster(ClusterSettings settings, IClusterableServerFactory serverFact _state = new InterlockedInt32(State.Initial); _rapidHeartbeatTimerCallbackState = new InterlockedInt32(RapidHeartbeatTimerCallbackState.NotRunning); _clusterId = new ClusterId(); - _descriptionWithChangedTaskCompletionSource = new(ClusterDescription.CreateInitial(_clusterId, _settings.DirectConnection)); + _descriptionWithChangedTaskCompletionSource = new (ClusterDescription.CreateInitial(_clusterId, _settings.DirectConnection)); _latencyLimitingServerSelector = new LatencyLimitingServerSelector(settings.LocalThreshold); _rapidHeartbeatTimer = new Timer(RapidHeartbeatTimerCallback, null, Timeout.InfiniteTimeSpan, Timeout.InfiniteTimeSpan); From 330996ced405ef6383773c59967744946fca76f2 Mon Sep 17 00:00:00 2001 From: Oleksandr Poliakov Date: Mon, 10 Mar 2025 16:38:17 -0700 Subject: [PATCH 4/4] pr --- src/MongoDB.Driver/Core/Clusters/Cluster.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/MongoDB.Driver/Core/Clusters/Cluster.cs b/src/MongoDB.Driver/Core/Clusters/Cluster.cs index 29f198c00fc..f1031387530 100644 --- a/src/MongoDB.Driver/Core/Clusters/Cluster.cs +++ b/src/MongoDB.Driver/Core/Clusters/Cluster.cs @@ -345,7 +345,7 @@ private void ThrowTimeoutException(IServerSelector selector, ClusterDescription } // nested classes - internal class ClusterDescriptionChangeSource + internal sealed class ClusterDescriptionChangeSource { private readonly TaskCompletionSource _changedTaskCompletionSource; private readonly ClusterDescription _clusterDescription;