Fix event counters around similar connection strings (#3485)

neyromant · Andrey.Kudashkin · web-flow · commit c451b2826f7a · 2021-01-28T13:15:37.000+01:00
Fixes #3480 Co-authored-by: Andrey.Kudashkin <andrey.kudashkin@russianpost.ru>
diff --git a/src/Npgsql/NpgsqlConnection.cs b/src/Npgsql/NpgsqlConnection.cs
@@ -195,7 +195,7 @@ void GetPoolAndSettings()
             {
                 // If the pool we created was the one that ended up being stored we need to increment the appropriate counter.
                 // Avoids a race condition where multiple threads will create a pool but only one will be stored.
-                NpgsqlEventSource.Log.PoolCreated();
+                NpgsqlEventSource.Log.PoolCreated(newPool);
             }
 
             _pool = PoolManager.GetOrAdd(_connectionString, _pool);
diff --git a/src/Npgsql/NpgsqlEventSource.cs b/src/Npgsql/NpgsqlEventSource.cs
@@ -1,4 +1,5 @@
 using System;
+using System.Collections.Generic;
 using System.Diagnostics;
 using System.Threading;
 using System.Diagnostics.Tracing;
@@ -42,7 +43,8 @@ sealed class NpgsqlEventSource : EventSource
         long _currentCommands;
         long _failedCommands;
 
-        int _pools;
+        readonly object _poolsLock = new();
+        readonly HashSet<ConnectorPool> _pools = new();
 
         long _multiplexingBatchesSent;
         long _multiplexingCommandsSent;
@@ -79,7 +81,13 @@ public void CommandStop()
 
         internal void CommandFailed() => Interlocked.Increment(ref _failedCommands);
 
-        internal void PoolCreated() => Interlocked.Increment(ref _pools);
+        internal void PoolCreated(ConnectorPool pool)
+        {
+            lock (_poolsLock)
+            {
+                _pools.Add(pool);
+            }
+        }
 
         internal void MultiplexingBatchSent(int numCommands, int waits, Stopwatch stopwatch)
         {
@@ -91,35 +99,42 @@ internal void MultiplexingBatchSent(int numCommands, int waits, Stopwatch stopwa
         }
 
 #if !NETSTANDARD2_0
-        static int GetIdleConnections()
+        int GetIdleConnections()
         {
             // Note: there's no attempt here to be coherent in terms of race conditions, especially not with regards
             // to different counters. So idle and busy and be unsynchronized, as they're not polled together.
-            var sum = 0;
-            foreach (var kv in PoolManager.Pools)
+            lock (_poolsLock)
             {
-                var pool = kv.Pool;
-                if (pool == null)
-                    return sum;
-                sum += pool.Statistics.Idle;
+                var sum = 0;
+                foreach (var pool in _pools)
+                {
+                    sum += pool.Statistics.Idle;
+                }
+                return sum;
             }
-            return sum;
         }
 
-        static int GetBusyConnections()
+        int GetBusyConnections()
         {
             // Note: there's no attempt here to be coherent in terms of race conditions, especially not with regards
             // to different counters. So idle and busy and be unsynchronized, as they're not polled together.
-            var sum = 0;
-            foreach (var kv in PoolManager.Pools)
+            lock (_poolsLock)
+            {
+                var sum = 0;
+                foreach (var pool in _pools)
+                {
+                    sum += pool.Statistics.Busy;
+                }
+                return sum;
+            }
+        }
+
+        int GetPoolsCount()
+        {
+            lock (_poolsLock)
             {
-                var pool = kv.Pool;
-                if (pool == null)
-                    return sum;
-                var (_, _, busy) = pool.Statistics;
-                sum += busy;
+                return _pools.Count;
             }
-            return sum;
         }
 
         protected override void OnEventCommand(EventCommandEventArgs command)
@@ -174,7 +189,7 @@ protected override void OnEventCommand(EventCommandEventArgs command)
                     DisplayUnits = "%"
                 };
 
-                _poolsCounter = new PollingCounter("connection-pools", this, () => _pools)
+                _poolsCounter = new PollingCounter("connection-pools", this, () => GetPoolsCount())
                 {
                     DisplayName = "Connection Pools"
                 };

Original file line number	Diff line number	Diff line change
`@@ -195,7 +195,7 @@ void GetPoolAndSettings()`
`195`	`195`	`{`
`196`	`196`	`// If the pool we created was the one that ended up being stored we need to increment the appropriate counter.`
`197`	`197`	`// Avoids a race condition where multiple threads will create a pool but only one will be stored.`
`198`		`- NpgsqlEventSource.Log.PoolCreated();`
	`198`	`+ NpgsqlEventSource.Log.PoolCreated(newPool);`
`199`	`199`	`}`
`200`	`200`
`201`	`201`	`_pool = PoolManager.GetOrAdd(_connectionString, _pool);`