Question: pool.Reset() alternative for v4

[hbagdi]

In v3, there is a pool.Reset() to reset the connection pool in case of a change in networking configuration (or rather for any reason).

I don't see anything similar in v4.
Is there a way to achieve a similar effect in v4?

[jackc]

I don't remember if that was removed intentionally or it was just lost when the pool was rewritten.

It can be more or less duplicated, but not as conveniently.

Use AcquireAllIdle and close all idle connections. Presumably everything that is already checked out will already be dead because of the network outage. A bit a hackery with an AfterRelease hook could probably handle closing checked out connections if you needed to be absolutely sure.

[hbagdi]

Presumably everything that is already checked out will already be dead because of the network outage.

What if there is no network outage but the server transitioned from a primary to read-only secondary?
In that case, the connections will be active but any write operation will fail.
Resetting all connections is the only way to be sure that all connections (and hopefully with a new DNS resolution) in the pool are for the updated primary db node.

[jackc]

I suppose it would be worth re-implementing. The v3 version was messier than I liked. Maybe it can be done more simply this time.

[cristaloleg]

Funny enough, literally yesterday I was upset that this method was removed. I need a way to recreate connections after aws autora failover.

[hbagdi]

Any specific high-level pointers? I could try getting something together in the next couple of weeks.

[jackc]

It's a little subtle. Broadly speaking there are two approaches.

The most obvious approach is to swap the underlying puddle pool. Unfortunately, this would require guarding all accesses to that pool with a mutex or using atomics. It may or may not make a significant performance difference but adding an synchronization to every pool access regardless of whether reset would ever be used is unfortunate.

The other approach is to add Reset to the puddle pool. The way this was done in pgx v3 is a good guide. The pool has an internal version counter and every connection / resource includes the pool version it was created by. When Reset is called all checked in resources are destroyed. When a resource is released it is destroyed if it is not the current version.

But this approach is also complicated because I wasn't anticipating any further changes to puddle as used by pgx v4. The master branch of puddle now is using generics which I don't want to require for pgx v4. So if this change was done for pgx v4 we would need to branch puddle.

[rajkong]

I have a similar problem as well.
I had an additional requirement of needing to perform a custom SQL health check on the connections received from AcquireAllIdle and if more than a few of them fail, then to swap-out the underlying pgx.Pool. So I am using Read lock for all queries to the pgx.Pool access and a write lock only when a number of connections fail and I need to clean out the pool.
What do you think about this?
Ideally, I would like to include this healthCheck that I am performing as an additional check within this code

pgx/pgxpool/pool.go

Line 456 in 91c9e84

func (p *Pool) checkConnsHealth() bool {

so that I am not wasting an additional go routine on the check.

[jackc]

Regarding Reset(), I reimplemented it in the development versions of pgx v5 and puddle v2. I'm expecting those to get a general release in a month or two.

It could probably be ported back to pgx v4 and puddle v1 without too much trouble, but I don't know if or when I will get to it.

[rajkong]

Thanks for the update on Reset. That is awesome! What are your thoughts on adding a user over-ridable function pointer for custom validation in checkConnsHealth()? I am happy to write one if you think it is worth adding. I have seen these in many of the JDBC drivers/pools implementations and did not find anything equivalent in Go, so the request

[jackc]

Your existing use of AcquireAllIdle for a custom health check is exactly why that method exists. Goroutines are pretty cheap in Go.

What are your thoughts on adding a user over-ridable function pointer for custom validation in checkConnsHealth()?

I'm open to it in theory, but in practice I suspect it might be tricky. The health check code is rather subtle in that it tries to avoid avoid even temporarily going over max conns, tries to minimize going under min conns, and tries to minimize multiple connections being closed or opened in parallel. I don't know of an obvious, clean way to integrate a user level check into that system.

---

One other subtlety that might affect this is there are different types of health checks. Checks such as detecting a connection is too old, or has been idle for too long, or there are too few connections in the pool detect sub-optimal but non-error conditions. If a connection is used a few seconds longer than its max age it doesn't really matter.

But there are also errors such as a network failure or a database schema change that can immediately render all existing connections broken.

The existing internal health check system is entirely focused on the former. The latter left to the application to handle with AcquireAllIdle() and Reset().

[rajkong]

Those are both good points. Just for the sake of consistency it may not be a good idea to club 2 things whose behaviour and consequences may be different. On the trickiness aspect, I can take a look. Let me contemplate on this a bit more and if I am able to solve it well (IMHO), then I'll open a PR. I am away on vacation till August 19th. Looking forward to picking this up when I am back. I think the reset is still a great add.

[rajkong]

One of the reasons why I felt that it would be a good idea to implement it within core pgx Pool implementations was the integration with ORMs. As you know these days most people end up using an ORM (I am not a fan) and some of them have built integration with Pgx, so it may make it easier.
I can't for the life of me understand, how Go screwed up so badly in their db Pool implementation.