npgsql: Connection Pool does not free idle connections in version 4.0
Hello,
It seems like the connection pool has changed in a way that seems undesired. In version 3, only as many connections as are needed are kept open, while in version 4, once a connection is opened the connection is kept alive indefinitely in normal circumstances.
For example, if you open 10 connections with “Connection Idle Lifetime=15” and then, in sequence, do “new NpgsqlConnection(blah).Open” every 100 ms, you will never drop down to 1 connection again (version 3 does drop down to 1 connection).
Since the parameter “Connection Idle Lifetime” exists and is now effectively useless as long as there is any load, I think that this a bug. Version 3 implemented the “Next idle connection” as a stack (List of connections with “removefirst” and “addFirst”). While version 4 takes a random idle connection. Where random is Thread.CurrentThread.ManagedThreadId % _max, effectively extending the a random connection by 15 seconds.
Code to replicate below, please not that since the ManagedThreadId is used as the random number generator, how you execute it might prevent the issue from being replicated. Works fine for me though and I hope the issue is clear enough 😃
static void Main(string[] args)
{
var tasks = new List<Task>();
for(int i = 0; i < 10; i++)
{
tasks.Add(Task.Run(() =>
{
using (var connection = new NpgsqlConnection("{connectionStringHere};Connection Idle Lifetime=15"))
{
connection.Open();
Console.WriteLine(Thread.CurrentThread.ManagedThreadId);
Thread.Sleep(1000);
}
}
));
}
Task.WaitAll(tasks.ToArray());
Console.WriteLine("Created 10 connections");
while (true)
{
//new Task is used in order to create a new ManagedTradeId which is used as a random number in ConnectionPool.cs. In IIS requests will generally have quite a spread of thread ids.
var task = Task.Run(() =>
{
using (var connection = new NpgsqlConnection("{ConnectionStringHere};Connection Idle Lifetime=15"))
{
connection.Open();
Console.WriteLine(Thread.CurrentThread.ManagedThreadId);
Thread.Sleep(100);
}
});
task.Wait();
}
}
Issue observed on: NPGSQL 4.0.3.0 (latest stable nuget package) Windows Server 2012 .NET 4.5.2
Cheers,
Tom
About this issue
- Original URL
- State: closed
- Created 6 years ago
- Reactions: 3
- Comments: 17 (10 by maintainers)
OK, thanks for the feedback. As this is a bug in the current behavior, and the fix is relatively risk-free (despite touching the pool…) I’ll try to do it for the next patch release. I’ll try to see about benchmarking the effects although I don’t have a good platform for that.
The description above is correct… The pool radically changed in version 4.0, rewritten to be completely lock-free to increase performance. Unfortunately this meant that we abandoned the previous stack design, which ensured we always return the most recently used connections, and therefore allow the least recently used connections to be pruned after idle time.
The idea of using the managed thread ID modulu _max was to start searching in random places in the array, so as to avoid “contention” at the start of the idle list, where many threads attempt to perform interlocked operations on the very same slot of the array.
We can indeed recreate the previous logic by having
Allocate()always start at index 0 and look for a connector moving forward, while havingRelease()start at the last slot and moving backwards. This would ensure that open attempts (usually!) return the last connection released back into the pool. The downside is a potential small slowdown as attempts contend on the same slots (open attempts always at the start, release attempts always at the end). I think I did a bit of benchmarking at some point and the random starting point wasn’t actually that important, so this should be OK - but we need to check again. Or we could think about doing something more fancy.@YohDeadfall @austindrenski any thoughts?
Hello, we have similar behavior on 4.0.3. Workaround for this bug is set pooling = false, but this thing made our app much slower. Or to reload app once a day, while we still have memory reserve on DB, this is no good idea… So we are waiting for 4.0.4 with fix.
@alex-namely can you please open a new issue for this? This issue is about pruning long-time idle connections, and not connection behavior when the database is down.
Just reiterating this problem as I ran into the same issue with too many connections being maintained in the pool .
As people move to containers and microservice architectures… you end up with more processes and the need for the client side connection pool to be more frugal about maintaining connections that are unnecessary.
The current implementation keeps many unnecessary connections open and active. All you need to reproduce is steady load and one spike or slowdown in the db to create a spiral of problems with too many connections.
Unless the load drops to basically 0 and goes idle, your process instance will tend to maintain its peak pool size since it starts looking for idle connections at a rather random point.
@TOTBWF thanks for reporting this, I will do my best to patch this for 4.0.8 although there is a lot going on at the moment.