pgbouncer icon indicating copy to clipboard operation
pgbouncer copied to clipboard

Support replication connections through PgBouncer

Open JelteF opened this issue 1 year ago • 6 comments

In session pooling mode PgBouncer is pretty much a transparent proxy, i.e. the client does normally not even need to know that PgBouncer is in the middle. This allows things like load balancing and failovers without the client needing to know about this at all. But as soon as replication connections are needed, this was not possible anymore, because PgBouncer would reject those instead of proxying them to the right server.

This PR fixes that by also proxying replication connections. They are handled pretty differently from normal connections though. A client and server replication connection will form a strong pair, as soon as one is closed the other is closed too. So, there's no caching of the server replication connections, like is done for regular connections. Reusing replication connections comes with a ton of gotchas. Postgres will throw errors in many cases when trying to do so. So simply not doing it seems like a good tradeoff for ease of implementation. Especially because replication connections are pretty much always very long lived. So re-using them gains pretty much no performance benefits.

Fixes #382

Depends on #945

JelteF avatar Jun 30 '23 09:06 JelteF

So re-using them gains pretty much no performance benefits.

Moreover, replication connections cannot be reliably reused at all. Attempting to do so will lead to various mysteries like

elog(ERROR, "no record found"); /* shouldn't happen */

in wal sender code. Wal sender implicitly assumes that it works with one wal receiver. (Based on my experience with a single cloud service where admins tried to use connection pooling for replication too)

Melkij avatar Jul 03 '23 11:07 Melkij

Moreover, replication connections cannot be reliably reused at all.

Thanks. I updated the wording in the PR to include that.

JelteF avatar Jul 03 '23 12:07 JelteF

I fixed all the things I wanted to in this PR. So I think it's ready for a more thorough review now.

JelteF avatar Jul 19 '23 13:07 JelteF

Isn't the cancel key change a requirement to this feature? I suggest you to create a separate commit for it.

eulerto avatar Sep 05 '23 23:09 eulerto

Isn't the cancel key change a requirement to this feature? I suggest you to create a separate commit for it.

Ops... I realized that you did it. Another suggestion is to explain in the commit message that it is a requirement for this feature.

eulerto avatar Sep 05 '23 23:09 eulerto

Ops... I realized that you did it. Another suggestion is to explain in the commit message that it is a requirement for this feature.

I added a commit message and moved it before the actual replication change, but while doing that I realized I thought the change was special enough to warrant its own PR, so I made a new one: https://github.com/pgbouncer/pgbouncer/pull/945

JelteF avatar Sep 06 '23 07:09 JelteF