From 98321150d6cef8765ae8fd0757fb7646b8ebf748 Mon Sep 17 00:00:00 2001 From: unaizorrilla Date: Thu, 21 Sep 2023 10:12:48 +0200 Subject: [PATCH 1/2] Added new version for check readme file on nuget --- .../workflows/healthchecks_azurekeyvault_secrets_cd_preview.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/healthchecks_azurekeyvault_secrets_cd_preview.yml b/.github/workflows/healthchecks_azurekeyvault_secrets_cd_preview.yml index f5c4f1c2e9..02bba8b833 100644 --- a/.github/workflows/healthchecks_azurekeyvault_secrets_cd_preview.yml +++ b/.github/workflows/healthchecks_azurekeyvault_secrets_cd_preview.yml @@ -12,6 +12,6 @@ jobs: secrets: inherit with: BUILD_CONFIG: Release - VERSION_SUFFIX_PREFIX: rc1 + VERSION_SUFFIX_PREFIX: rc1.1 PROJECT_PATH: ./src/HealthChecks.Azure.KeyVault.Secrets/HealthChecks.Azure.KeyVault.Secrets.csproj PACKAGE_NAME: AspNetCore.HealthChecks.Azure.KeyVault.Secrets \ No newline at end of file From 418903efe363a420728bfd59d2bf9e52a0e6f188 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Tue, 3 Oct 2023 15:27:21 +0200 Subject: [PATCH 2/2] Npgsql: don't create more than one DataSource (#2045) * add a failing test * provide simple fix, non-thread safe fix * provide thread-safe fix * bump Npgsql version * add .NET 7 to list of supported tfms to avoid "System.TypeLoadException : Could not load type 'System.Data.Common.DbDataSource' from assembly 'Npgsql, Version=7.0.6.0, Culture=neutral, PublicKeyToken=5d8b90d52f46fda7'." thrown for .NET 7 test runner * bump the version * add test for scenario when there is pre-registered DataSource in the DI * use NpgsqlDataSource registered in the DI when the connection string match * reorder the usings to satisfy dotnet format * one more fix --- build/versions.props | 2 +- .../NpgSqlHealthCheckBuilderExtensions.cs | 55 +++++++++++++-- .../HealthChecks.NpgSql.csproj | 4 +- .../DependencyInjection/RegistrationTests.cs | 68 +++++++++++++++++++ 4 files changed, 121 insertions(+), 8 deletions(-) diff --git a/build/versions.props b/build/versions.props index 550a58a2d6..d1faed5b97 100644 --- a/build/versions.props +++ b/build/versions.props @@ -39,7 +39,7 @@ 7.0.0 7.0.0 7.0.0 - 7.0.0 + 7.1.0 7.1.0 7.0.0 7.0.0 diff --git a/src/HealthChecks.NpgSql/DependencyInjection/NpgSqlHealthCheckBuilderExtensions.cs b/src/HealthChecks.NpgSql/DependencyInjection/NpgSqlHealthCheckBuilderExtensions.cs index fbcecaf63d..fb0dd34653 100644 --- a/src/HealthChecks.NpgSql/DependencyInjection/NpgSqlHealthCheckBuilderExtensions.cs +++ b/src/HealthChecks.NpgSql/DependencyInjection/NpgSqlHealthCheckBuilderExtensions.cs @@ -95,16 +95,61 @@ public static IHealthChecksBuilder AddNpgSql( { Guard.ThrowIfNull(dbDataSourceFactory); + NpgsqlDataSource? dataSource = null; + NpgSqlHealthCheckOptions options = new() + { + CommandText = healthQuery, + Configure = configure, + }; + return builder.Add(new HealthCheckRegistration( name ?? NAME, sp => { - var options = new NpgSqlHealthCheckOptions + // The Data Source needs to be created only once, + // as each instance has it's own connection pool. + // See https://github.com/Xabaril/AspNetCore.Diagnostics.HealthChecks/issues/1993 for more details. + + // Perform an atomic read of the current value. + NpgsqlDataSource? existingDataSource = Volatile.Read(ref dataSource); + if (existingDataSource is null) { - DataSource = dbDataSourceFactory(sp), - CommandText = healthQuery, - Configure = configure, - }; + // Create a new Data Source + NpgsqlDataSource fromFactory = dbDataSourceFactory(sp); + // Try to resolve the Data Source from DI. + NpgsqlDataSource? fromDI = sp.GetService(); + + if (fromDI is not null && fromDI.ConnectionString.Equals(fromFactory.ConnectionString)) + { + // If they are using the same ConnectionString, we can reuse the instance from DI. + // So there is only ONE NpgsqlDataSource per the whole app and ONE connection pool. + + if (!ReferenceEquals(fromDI, fromFactory)) + { + // Dispose it, as long as it's not the same instance. + fromFactory.Dispose(); + } + Interlocked.Exchange(ref dataSource, fromDI); + options.DataSource = fromDI; + } + else + { + // Perform an atomic exchange, but only if the value is still null. + existingDataSource = Interlocked.CompareExchange(ref dataSource, fromFactory, null); + if (existingDataSource is not null) + { + // Some other thread has created the data source in the meantime, + // we dispose our own copy, and use the existing instance. + fromFactory.Dispose(); + options.DataSource = existingDataSource; + } + else + { + options.DataSource = fromFactory; + } + } + } + return new NpgSqlHealthCheck(options); }, failureStatus, diff --git a/src/HealthChecks.NpgSql/HealthChecks.NpgSql.csproj b/src/HealthChecks.NpgSql/HealthChecks.NpgSql.csproj index 062dfa184f..f29c7f2da2 100644 --- a/src/HealthChecks.NpgSql/HealthChecks.NpgSql.csproj +++ b/src/HealthChecks.NpgSql/HealthChecks.NpgSql.csproj @@ -1,14 +1,14 @@ - netstandard2.0 + netstandard2.0;net7.0 $(PackageTags);Beat;Postgress HealthChecks.NpgSql is a health check for Postgress Sql. $(HealthCheckNpgSql) - + diff --git a/test/HealthChecks.Npgsql.Tests/DependencyInjection/RegistrationTests.cs b/test/HealthChecks.Npgsql.Tests/DependencyInjection/RegistrationTests.cs index b897f56df3..1d3252e4cd 100644 --- a/test/HealthChecks.Npgsql.Tests/DependencyInjection/RegistrationTests.cs +++ b/test/HealthChecks.Npgsql.Tests/DependencyInjection/RegistrationTests.cs @@ -1,4 +1,6 @@ +using System.Reflection; using HealthChecks.NpgSql; +using Npgsql; namespace HealthChecks.Npgsql.Tests.DependencyInjection; @@ -62,4 +64,70 @@ public void add_health_check_with_connection_string_factory_when_properly_config check.ShouldBeOfType(); factoryCalled.ShouldBeTrue(); } + + [Fact] + public void factory_is_called_only_once() + { + ServiceCollection services = new(); + int factoryCalls = 0; + services.AddHealthChecks() + .AddNpgSql(_ => + { + Interlocked.Increment(ref factoryCalls); + return "Server=localhost"; + }, name: "my-npg-1"); + + using var serviceProvider = services.BuildServiceProvider(); + + var options = serviceProvider.GetRequiredService>(); + + var registration = options.Value.Registrations.Single(); + + for (int i = 0; i < 10; i++) + { + _ = registration.Factory(serviceProvider); + } + + factoryCalls.ShouldBe(1); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public void factory_reuses_pre_registered_datasource_when_possible(bool sameConnectionString) + { + const string connectionString = "Server=localhost"; + ServiceCollection services = new(); + + services.AddSingleton(serviceProvider => + { + var dataSourceBuilder = new NpgsqlDataSourceBuilder(connectionString); + return dataSourceBuilder.Build(); + }); + + int factoryCalls = 0; + services.AddHealthChecks() + .AddNpgSql(_ => + { + Interlocked.Increment(ref factoryCalls); + return sameConnectionString ? connectionString : $"{connectionString}2"; + }, name: "my-npg-1"); + + using var serviceProvider = services.BuildServiceProvider(); + + var options = serviceProvider.GetRequiredService>(); + + var registration = options.Value.Registrations.Single(); + + for (int i = 0; i < 10; i++) + { + var healthCheck = (NpgSqlHealthCheck)registration.Factory(serviceProvider); + var fieldInfo = typeof(NpgSqlHealthCheck).GetField("_options", BindingFlags.Instance | BindingFlags.NonPublic); + var npgSqlHealthCheckOptions = (NpgSqlHealthCheckOptions)fieldInfo!.GetValue(healthCheck)!; + + Assert.Equal(sameConnectionString, ReferenceEquals(serviceProvider.GetRequiredService(), npgSqlHealthCheckOptions.DataSource)); + } + + factoryCalls.ShouldBe(1); + } }