Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Too many open connections with NpgSql 7.0.0 #1993

Closed
peterkiss1 opened this issue Aug 11, 2023 · 7 comments
Closed

Too many open connections with NpgSql 7.0.0 #1993

peterkiss1 opened this issue Aug 11, 2023 · 7 comments
Assignees
Labels
bug Something isn't working
Milestone

Comments

@peterkiss1
Copy link

What happened: upgraded from 6.0.2 to 7.0.0 and 1 application instance opens ~40 connections towards the database which cause issues as the number of connections are limited, with the old version it has opened ~2 connections.

What you expected to happen: should not have this many connection opened and left dangling.

How to reproduce it: grab an application (checked with .NET 7 and 8 Preview) add this package and start running the health checks.

Source code sample: no special code is required, plain vanilla ASP.NET health check setup.

Environment:

  • .NET Core version: 7 and 8 Preview
  • Healthchecks version: NpgSql 7.0.0
  • Operative system: Linux
@tyshkavets
Copy link

Experiencing the same thing. Seems to be due to an introduction of DbDataSource abstraction in .NET 7 (details here) and NpgSqlDataSource implementation for it in corresponding NpgSql version.

According to NpgSqlHealthCheckBuilderExtensions.cs, a new one is spawned every time we execute a health check, leading to excessive numbers of dangling connections. It happens because the code responsible for creating DbDataSource is simply packed into a delegate and passed into HealthCheckRegistration.Factory, to be invoked on every RunCheckAsync().

It's a rather unpleasant surprise, I'd (perhaps, naively) expect them to be pooled under the hood.

My workaround was to create a data source, stick it as a singleton into DI and use .AddNpgSql() overload that accepts Func<IServiceProvider, NpgsqlDataSource> rather than a connection string, effectively pooling it on my own. Should be fine if your connection string isn't prone to changing.

@sungam3r
Copy link
Collaborator

OK, got it. Need time to think how to deal with it.

@sungam3r sungam3r added the bug Something isn't working label Aug 15, 2023
@iam-black
Copy link

iam-black commented Sep 8, 2023

I encountered the same issue and used the same overload as @tyshkavets for the fix, but without explicit singleton registration:
var options = new NpgSqlHealthCheckOptions { DataSource = new NpgsqlDataSourceBuilder(connectionString).Build(), }; hcBuilder.AddNpgSql(options);

@kamisoft-fr
Copy link

kamisoft-fr commented Sep 12, 2023

Hi,
I had the same issue, and I finally ended up tweaking the connection string to avoid pooling:

            string connectionString = configuration.GetConnectionString("xxx") ;

            NpgsqlConnectionStringBuilder healthCheckConnectionStringBuilder = new(connectionString)
            {
                // Avoid long life pooled PG connections
                Pooling = false
            };

            healthCheckBuilder.AddNpgSql(connectionString: healthCheckConnectionStringBuilder.ToString(),
               ...);

@adamsitnik
Copy link
Collaborator

I would be happy to provide a fix and include it in the 8.0 version.

Is my understanding correct that the current API design leads the users into a trap of creating a new instance of NpgsqlDataSource every time the HC is invoked? And every data source instance has it's own pool of connections?

@adamsitnik adamsitnik self-assigned this Sep 21, 2023
@unaizorrilla
Copy link
Collaborator

I can get the fix and move down to 6.X and current 7.X versions!

@peterkiss1
Copy link
Author

7.1.0 works fine, closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

7 participants