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

Add support for locking table in pgx-driver #992

Merged
merged 3 commits into from
Dec 20, 2023

Conversation

tsfcode
Copy link

@tsfcode tsfcode commented Oct 19, 2023

A common pattern in postgres deployment is to use pgbouncer in transaction pooling mode. This is to support at large number of concurrent clients to work around the process based postgres connection model.
Unfortunately pgbouncer does not support advisory locks in transaction pooling mode, which is the most commonly used mode. To work around this we introduce this traditional table lock method. As used in the cockroach driver. We expose the feature behind a config flag and default the config to advisory lock.

fortnox-andreas and others added 2 commits September 20, 2023 09:59
In order to support running migrations through PgBouncer which does not
support advisory locks.
@coveralls
Copy link

coveralls commented Oct 19, 2023

Coverage Status

coverage: 59.183%. first build
when pulling 091ad5d on fortnox-andreas:master
into 691f687 on golang-migrate:master.

Copy link
Member

@dhui dhui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tsfcode Thanks for the PR and for explaining why it's needed!

Did you have plans to also make this change to the postgres driver? (You don't need to)

database/pgx/pgx.go Outdated Show resolved Hide resolved
database/pgx/pgx.go Show resolved Hide resolved
database/pgx/pgx.go Outdated Show resolved Hide resolved
@tsfcode
Copy link
Author

tsfcode commented Dec 20, 2023

@tsfcode Thanks for the PR and for explaining why it's needed!

Did you have plans to also make this change to the postgres driver? (You don't need to)

I don't have any plans to add it to the postgres driver, since we only use the pgx driver at the moment.

Defer rollback of transactions
Copy link
Member

@dhui dhui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tsfcode Thanks for addressing the PR feedback!

@dhui dhui merged commit 0815e2d into golang-migrate:master Dec 20, 2023
8 checks passed
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.

None yet

5 participants