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

Use Postgres.conn rather than Postgres.db to handle table locking #3

Merged
merged 1 commit into from Jan 30, 2024

Conversation

bclarkx2
Copy link

@bclarkx2 bclarkx2 commented Jan 30, 2024

#2 makes it so that WithInstance in the pgx database driver simply grabs a connection from the pool and calls WithConnection on it.

However, golang-migrate#992 has added the ability to configure the pgx driver to use either advisory or table locks to lock the database for migrations.

This PR updates the functions that acquire and release those table locks to use Postgres.conn rather than Postgres.db, as following the changes in #2 all Postgres instances will have conn set but those instantiated with WithConnection may not have db set.

@bclarkx2 bclarkx2 self-assigned this Jan 30, 2024
@@ -370,7 +370,7 @@ func (p *Postgres) releaseTableLock() error {
}

query := "DELETE FROM " + pq.QuoteIdentifier(p.config.LockTable) + " WHERE lock_id = $1"
if _, err := p.db.Exec(query, aid); err != nil {
if _, err := p.conn.ExecContext(context.Background(), query, aid); err != nil {
Copy link
Author

Choose a reason for hiding this comment

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

I didn't change these functions to accept a context just because I didn't want to deviate too far from the current behavior in which these queries are not cancellable, mostly on the grounds that I'm not super familiar with the ramifications of making them cancellable.

If you or the upstream maintainers would prefer that these functions accept a context rather than use the background context, I'd be happy to make that change!

@bclarkx2 bclarkx2 merged commit 186a870 into master Jan 30, 2024
10 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
1 participant