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

Support for PgBouncer transaction pooling #2447

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

vgogolev1703
Copy link

@vgogolev1703 vgogolev1703 commented Jan 16, 2024

We've had some issues using atlasgo with PgBouncer, which are caused by the fact that our PgBouncer runs in transactional mode, which is widely used in production (as we do in our production).

Sometimes, after successful migration, atlasgo can't release advisory lock. We migrate our schema using the same connection that the application uses (throught pgbouncer)

Error: sql/postgres: failed releasing lock 2686470616

We found out, that atlasgo using session-level advisory locks, which is not correctly working with transactional mode..

To prevent such behavior, atlasgo can use pg_advisory_xact_lock (which is transactional) with some bonus - you don't need to release it, cause it unlocks right after transaction ends. Also, it should work correctly with PgBouncer in session mode and without PgBouncer at all.
https://www.postgresql.org/docs/current/functions-admin.html#FUNCTIONS-ADVISORY-LOCKS

Here's the same problem that flyway had a few years ago, which was resolved by using advisory locks in transactional mode by default.
flyway/flyway#2895

@a8m
Copy link
Member

a8m commented Jan 16, 2024

Thanks for the contribution, @vgogolev1703. Unfortunately, I cannot easily merge this because locks in Atlas are taken in multiple places, including both by sessions/connections and transactions. Therefore, the code there should be more dynamic.

I will try to give it a look in the upcoming days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants