Add 'UseMigrationLock' flag to acquire exclusive lock while performing migrations for Postgres#596
Add 'UseMigrationLock' flag to acquire exclusive lock while performing migrations for Postgres#596zapling wants to merge 4 commits into
Conversation
zapling
changed the title
zapling
changed the title
|
This is exactly what I need right now. Would love to see this get merged @amacneil ! |
|
We have been using these changes in production for our client for a couple of months now, no issues so far 👌 |
|
As an aside, my proposed support for an Just another use case for |
Usage of This PR addresses this by opening a separate database connection that starts a transaction just to hold the lock, so when the transaction is comitted or rollbacked, the lock is automatically released. |
Ahh, that's clever. Maybe unnecessarily so? Why not use a session-level lock, instead?
In other words, |
Our use case is running this from within the application which runs inside a kubernetes pod, if the pod gets killed while the migration is running we get into a broken state where the lock would need to be manually deleted. |
Session-level locks get released when the session ends, so even if the pod is killed, if the migration is still running and the session hasn't been killed with a In other words, it sounds to me like the session-level lock held in the session applying the actual long-running migrations ensures actual mutual exclusivity so that two executions of dbmate won't apply migrations concurrently, where the implementation in this PR could still result in a long-running migration being started twice, if the first one was terminated and the transaction lock held in a separate session was forcibly released. |
We did see some issues with a session level lock when testing this initially, hence the usage of the transaction level lock. I do not recall the exact case, could have been some other issue on our end, and maybe a session level lock is actually enough. I might have time to re-test this with a session level lock this coming week. |
|
fwiw, I am using the following function to implement locks until this PR is merged. I am using dbmate as a library package pgutil
import (
"context"
"database/sql"
"fmt"
)
// AcquireAdvisoryLock acquires a session-level advisory lock on the given key.
// It blocks until the lock is acquired, then returns a function to release the lock.
func AcquireAdvisoryLock(ctx context.Context, db *sql.DB, key int64) (func(), error) {
// Get a dedicated connection from the pool
conn, err := db.Conn(ctx)
if err != nil {
return nil, fmt.Errorf("failed to get dedicated connection: %w", err)
}
// Acquire the advisory lock
if _, err := conn.ExecContext(ctx, `SELECT pg_advisory_lock($1)`, key); err != nil {
_ = conn.Close() // always clean up on failure
return nil, fmt.Errorf("failed to acquire advisory lock: %w", err)
}
fmt.Println("Advisory lock acquired")
// Unlock function
unlock := func() {
defer conn.Close() // releases the connection and any held locks
if _, err := conn.ExecContext(context.Background(), `SELECT pg_advisory_unlock($1)`, key); err != nil {
fmt.Printf("Failed to release advisory lock: %v\n", err)
} else {
fmt.Println("Advisory lock released successfully")
}
}
return unlock, nil
}Example // Attempt to acquire an advisory lock with key 42
unlock, err := AcquireAdvisoryLock(ctx, db, 42)
if err != nil {
fmt.Printf("Error acquiring advisory lock: %v\n", err)
return
}
defer unlock()
// Your critical section
fmt.Println("Migrating...")
if err := db.Migrate(); err != nil {
fmt.Println("Failed to run migrations")
}
fmt.Println("Done.") |
4 participants
First I'd like to thank you @amacneil for all the hard work, been using this project for years now and it's the best tool for the job.
We currently have a client who are performing migrations from within their code using the dbmate library to do so. They have multiple components that are run as their own deployment in kubernetes, meaning each component is peforming a migration. When we do new deployments of this application, each component starts up and migrations are attempted at the same time, resulting in race conditions.
We have resolved this by adding a new
DriverMigrationLockinterface, that the Postgres driver implements. It uses an postgres advisory lock within a database transaction to prevent any other dbmate instance from migrating while the lock is active.The use of the exclusive lock is currently opt-in by using the
UseMigrationLockfield on theDBstruct.I feel like this might be something more people would like to have, who might be in a similar position.
I have added a few tests for the Postgres driver, I can add more, but unsure what exactly what we should test? It might be nice to have one test for the
DBstruct, if so, could you point me in the right direction?