CSHARP-5496: Reduce locks contention on server selection and connection checkout #1617
Conversation
sanych-sun
requested review from
BorisDog and
papafe
and removed request for
a team
| private ClusterDescription _description; | ||
| private TaskCompletionSource<bool> _descriptionChangedTaskCompletionSource; | ||
| private readonly object _descriptionLock = new object(); | ||
| private Tuple<ClusterDescription, TaskCompletionSource<bool>> _descriptionWithChangedTaskCompletionSource; |
There was a problem hiding this comment.
I've decided to use Tuple instead of 2 fields, to let me remove lock in UpdateClusterDescription method.
| oldClusterDescription = _description; | ||
| _description = newClusterDescription; | ||
| var oldClusterDescription = Interlocked.Exchange(ref _descriptionWithChangedTaskCompletionSource, | ||
| Tuple.Create(newClusterDescription, new TaskCompletionSource<bool>(TaskCreationOptions.RunContinuationsAsynchronously))); |
There was a problem hiding this comment.
We can use TaskCreationOptions.RunContinuationsAsynchronously and remove old workaround with explicitly running new task (see line 311 before the changes).
There was a problem hiding this comment.
minor: new(newClusterDescription, new(TaskCreationOptions..)) ?
| TrackInUseConnection(result); | ||
| } | ||
|
|
||
| // This connection can be expired and not disposed by Prune. Dispose if needed |
There was a problem hiding this comment.
It looks like there is no way to get expired connection here, because we just checked that in line 763.
sanych-sun
changed the title
| private ClusterDescription _description; | ||
| private TaskCompletionSource<bool> _descriptionChangedTaskCompletionSource; | ||
| private readonly object _descriptionLock = new object(); | ||
| private Tuple<ClusterDescription, TaskCompletionSource<bool>> _descriptionWithChangedTaskCompletionSource; |
| _clusterId = new ClusterId(); | ||
| _description = ClusterDescription.CreateInitial(_clusterId, _settings.DirectConnection); | ||
| _descriptionChangedTaskCompletionSource = new TaskCompletionSource<bool>(); | ||
| _descriptionWithChangedTaskCompletionSource = new(ClusterDescription.CreateInitial(_clusterId, _settings.DirectConnection), new TaskCompletionSource<bool>(TaskCreationOptions.RunContinuationsAsynchronously)); |
There was a problem hiding this comment.
minor:
_descriptionWithChangedTaskCompletionSource = new(ClusterDescription.CreateInitial(_clusterId, _settings.DirectConnection), new(TaskCreationOptions.RunContinuationsAsynchronously));
| oldClusterDescription = _description; | ||
| _description = newClusterDescription; | ||
| var oldClusterDescription = Interlocked.Exchange(ref _descriptionWithChangedTaskCompletionSource, | ||
| Tuple.Create(newClusterDescription, new TaskCompletionSource<bool>(TaskCreationOptions.RunContinuationsAsynchronously))); |
There was a problem hiding this comment.
minor: new(newClusterDescription, new(TaskCreationOptions..)) ?
|
|
||
| // TODO: use RunContinuationsAsynchronously instead once we require a new enough .NET Framework | ||
| Task.Run(() => oldDescriptionChangedTaskCompletionSource.TrySetResult(true)); | ||
| oldClusterDescription.Item2.TrySetResult(true); |
There was a problem hiding this comment.
Consider defining a 2 liner private class to improve readability of Item1 and Item2.
| } | ||
|
|
||
| // nested classes | ||
| internal class ClusterDescriptionChangeSource |
|
|
||
| public ClusterDescriptionChangeSource(ClusterDescription clusterDescription) | ||
| { | ||
| _changedTaskCompletionSource = new TaskCompletionSource<bool>(TaskCreationOptions.RunContinuationsAsynchronously); |
There was a problem hiding this comment.
Nice to hide this ceremony of source creation.
3 participants
No description provided.