Skip to content

Commit fc0a675

Browse files
NinoFlorisNinoFloris
authored
Move TryTrackDataSource to constructor and enable metrics more accurately (#6329)
1 parent a8b9564 commit fc0a675

6 files changed

Lines changed: 42 additions & 31 deletions

File tree

‎src/Npgsql/MetricsReporter.cs‎

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -178,20 +178,16 @@ static IEnumerable<Measurement<int>> GetConnectionUsage()
178178
{
179179
var reporter = Reporters[i];
180180

181-
if (reporter._dataSource is PoolingDataSource poolingDataSource)
182-
{
183-
var stats = poolingDataSource.Statistics;
184-
185-
measurements.Add(new Measurement<int>(
186-
stats.Idle,
187-
reporter._poolNameTag,
188-
new KeyValuePair<string, object?>("state", "idle")));
189-
190-
measurements.Add(new Measurement<int>(
191-
stats.Busy,
192-
reporter._poolNameTag,
193-
new KeyValuePair<string, object?>("state", "used")));
194-
}
181+
var connectionStats = reporter._dataSource.Statistics;
182+
measurements.Add(new Measurement<int>(
183+
connectionStats.Idle,
184+
reporter._poolNameTag,
185+
new KeyValuePair<string, object?>("state", "idle")));
186+
187+
measurements.Add(new Measurement<int>(
188+
connectionStats.Busy,
189+
reporter._poolNameTag,
190+
new KeyValuePair<string, object?>("state", "used")));
195191
}
196192

197193
return measurements;

‎src/Npgsql/MultiHostDataSourceWrapper.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
namespace Npgsql;
99

1010
sealed class MultiHostDataSourceWrapper(NpgsqlMultiHostDataSource wrappedSource, TargetSessionAttributes targetSessionAttributes)
11-
: NpgsqlDataSource(CloneSettingsForTargetSessionAttributes(wrappedSource.Settings, targetSessionAttributes), wrappedSource.Configuration)
11+
: NpgsqlDataSource(CloneSettingsForTargetSessionAttributes(wrappedSource.Settings, targetSessionAttributes), wrappedSource.Configuration, reportMetrics: false)
1212
{
1313
internal NpgsqlMultiHostDataSource WrappedSource { get; } = wrappedSource;
1414

‎src/Npgsql/NpgsqlDataSource.cs‎

Lines changed: 27 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -92,11 +92,11 @@ private protected readonly Dictionary<Transaction, List<NpgsqlConnector>> _pendi
9292
readonly SemaphoreSlim _setupMappingsSemaphore = new(1);
9393

9494
readonly INpgsqlNameTranslator _defaultNameTranslator;
95-
IDisposable? _eventSourceEvents;
95+
readonly IDisposable? _eventSourceEvents;
9696

9797
internal NpgsqlDataSource(
9898
NpgsqlConnectionStringBuilder settings,
99-
NpgsqlDataSourceConfiguration dataSourceConfig)
99+
NpgsqlDataSourceConfiguration dataSourceConfig, bool reportMetrics)
100100
{
101101
Settings = settings;
102102
ConnectionString = settings.PersistSecurityInfo
@@ -145,7 +145,21 @@ internal NpgsqlDataSource(
145145
}
146146

147147
Name = name ?? ConnectionString;
148-
MetricsReporter = new MetricsReporter(this);
148+
149+
// TODO this needs a rework, but for now we just avoid tracking multi-host data sources directly.
150+
if (reportMetrics)
151+
{
152+
MetricsReporter = new MetricsReporter(this);
153+
if (!NpgsqlEventSource.Log.TryTrackDataSource(Name, this, out _eventSourceEvents))
154+
_connectionLogger.LogDebug("NpgsqlEventSource could not start tracking a DataSource, " +
155+
"this can happen if more than one data source uses the same connection string.");
156+
}
157+
else
158+
{
159+
// This is not accessed anywhere currently for multi-host data sources.
160+
// Connectors which handle the metrics always access their nonpooling/pooling data source instead.
161+
MetricsReporter = null!;
162+
}
149163
}
150164

151165
/// <inheritdoc cref="DbDataSource.CreateConnection" />
@@ -315,10 +329,6 @@ internal async Task Bootstrap(
315329
serializerOptions: serializerOptions,
316330
dbTypeResolver: new ChainDbTypeResolver(resolvers));
317331

318-
if (!NpgsqlEventSource.Log.TryTrackDataSource(Name, this, out _eventSourceEvents))
319-
_connectionLogger.LogDebug("NpgsqlEventSource could not start tracking a DataSource, " +
320-
"this can happen if more than one data source uses the same connection string.");
321-
322332
IsBootstrapped = true;
323333
}
324334
finally
@@ -523,8 +533,11 @@ protected virtual void DisposeBase()
523533
}
524534

525535
_periodicPasswordProviderTimer?.Dispose();
526-
MetricsReporter.Dispose();
527-
_eventSourceEvents?.Dispose();
536+
if (MetricsReporter is not null)
537+
{
538+
MetricsReporter.Dispose();
539+
_eventSourceEvents?.Dispose();
540+
}
528541

529542
// We do not dispose _setupMappingsSemaphore explicitly, leaving it to finalizer
530543
// Due to possible concurrent access, which might lead to deadlock
@@ -555,8 +568,11 @@ protected virtual async ValueTask DisposeAsyncBase()
555568
if (_periodicPasswordProviderTimer is not null)
556569
await _periodicPasswordProviderTimer.DisposeAsync().ConfigureAwait(false);
557570

558-
MetricsReporter.Dispose();
559-
_eventSourceEvents?.Dispose();
571+
if (MetricsReporter is not null)
572+
{
573+
MetricsReporter.Dispose();
574+
_eventSourceEvents?.Dispose();
575+
}
560576
// We do not dispose _setupMappingsSemaphore explicitly, leaving it to finalizer
561577
// Due to possible concurrent access, which might lead to deadlock
562578
// See issue #6115

‎src/Npgsql/NpgsqlMultiHostDataSource.cs‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
using System.Collections.Generic;
55
using System.Diagnostics;
66
using System.Diagnostics.CodeAnalysis;
7-
using System.Linq;
87
using System.Threading;
98
using System.Threading.Tasks;
109
using System.Transactions;
@@ -31,7 +30,7 @@ public sealed class NpgsqlMultiHostDataSource : NpgsqlDataSource
3130
volatile int _roundRobinIndex = -1;
3231

3332
internal NpgsqlMultiHostDataSource(NpgsqlConnectionStringBuilder settings, NpgsqlDataSourceConfiguration dataSourceConfig)
34-
: base(settings, dataSourceConfig)
33+
: base(settings, dataSourceConfig, reportMetrics: false)
3534
{
3635
var hosts = settings.Host!.Split(',');
3736
_pools = new NpgsqlDataSource[hosts.Length];
@@ -53,7 +52,7 @@ internal NpgsqlMultiHostDataSource(NpgsqlConnectionStringBuilder settings, Npgsq
5352
: new UnpooledDataSource(poolSettings, dataSourceConfig);
5453
}
5554

56-
var targetSessionAttributeValues = Enum.GetValues<TargetSessionAttributes>().ToArray();
55+
var targetSessionAttributeValues = Enum.GetValues<TargetSessionAttributes>();
5756
var highestValue = 0;
5857
foreach (var value in targetSessionAttributeValues)
5958
if ((int)value > highestValue)

‎src/Npgsql/PoolingDataSource.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ internal sealed override (int Total, int Idle, int Busy) Statistics
7474
internal PoolingDataSource(
7575
NpgsqlConnectionStringBuilder settings,
7676
NpgsqlDataSourceConfiguration dataSourceConfig)
77-
: base(settings, dataSourceConfig)
77+
: base(settings, dataSourceConfig, reportMetrics: true)
7878
{
7979
if (settings.MaxPoolSize < settings.MinPoolSize)
8080
throw new ArgumentException($"Connection can't have 'Max Pool Size' {settings.MaxPoolSize} under 'Min Pool Size' {settings.MinPoolSize}");

‎src/Npgsql/UnpooledDataSource.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
namespace Npgsql;
88

99
sealed class UnpooledDataSource(NpgsqlConnectionStringBuilder settings, NpgsqlDataSourceConfiguration dataSourceConfig)
10-
: NpgsqlDataSource(settings, dataSourceConfig)
10+
: NpgsqlDataSource(settings, dataSourceConfig, reportMetrics: true)
1111
{
1212
volatile int _numConnectors;
1313

0 commit comments

Comments
 (0)