sqldb: reuse one migrate driver across migrations to avoid pool exhaustion

docs/release-notes/release-notes-0.22.0.md

@@ -138,6 +138,12 @@
 
 ## Database
 
+* [Fixed a bug](https://github.com/lightningnetwork/lnd/pull/11071) where a fresh
+  Postgres database could hang during startup migrations when a small
+  `db.postgres.maxconnections` was set. Each migration reserved a pool connection
+  that was never released, exhausting a small pool mid-startup. Migrations now
+  reuse a single connection.
+
 ## Code Health
 
 ## Tooling and Documentation
@@ -159,3 +165,4 @@
 * Boris Nagaev
 * Erick Cestari
 * Jared Tobin
+* Vandit Singh

sqldb/migrations_test.go

@@ -6,6 +6,7 @@ import (
 	"path/filepath"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/golang-migrate/migrate/v4"
 	"github.com/golang-migrate/migrate/v4/database"
@@ -723,3 +724,52 @@ func TestMigrationConfigConsistency(t *testing.T) {
 	}
 
 }
+
+// TestPostgresMigrationSmallConnPool is a regression test for
+// https://github.com/lightningnetwork/lnd/issues/11007. Applying all migrations
+// to a fresh Postgres database with a connection pool smaller than the number
+// of migrations must not deadlock. Previously each migration created its own
+// migrate driver, and every driver reserved a dedicated pool connection that
+// was never released, so a small pool was exhausted mid-startup and the
+// migration-tracker transaction blocked forever with no timeout or error.
+func TestPostgresMigrationSmallConnPool(t *testing.T) {
+	t.Parallel()
+
+	ctxb := t.Context()
+
+	fixture := NewTestPgFixture(t, DefaultPostgresFixtureLifetime)
+	t.Cleanup(func() {
+		fixture.TearDown(t)
+	})
+
+	dbName := randomDBName(t)
+	_, err := fixture.db.ExecContext(ctxb, "CREATE DATABASE "+dbName)
+	require.NoError(t, err)
+
+	// Cap the client pool well below the number of migrations so the old
+	// per-migration driver leak would exhaust it before the run completes.
+	cfg := fixture.GetConfig(dbName)
+	cfg.MaxConnections = 2
+
+	store, err := NewPostgresStore(cfg)
+	require.NoError(t, err)
+	t.Cleanup(func() {
+		require.NoError(t, store.DB.Close())
+	})
+
+	// ApplyAllMigrations blocks forever when the pool is exhausted, so run
+	// it under a deadline instead of hanging the whole test binary.
+	done := make(chan error, 1)
+	go func() {
+		done <- store.ApplyAllMigrations(ctxb, GetMigrations())
+	}()
+
+	select {
+	case err := <-done:
+		require.NoError(t, err)
+
+	case <-time.After(time.Minute):
+		t.Fatal("migrations deadlocked with a small connection pool " +
+			"(issue #11007)")
+	}
+}

sqldb/postgres.go

@@ -9,6 +9,7 @@ import (
 	"strings"
 	"time"
 
+	"github.com/golang-migrate/migrate/v4/database"
 	pgx_migrate "github.com/golang-migrate/migrate/v4/database/pgx/v5"
 	_ "github.com/golang-migrate/migrate/v4/source/file" // Read migrations from files. // nolint:ll
 	_ "github.com/jackc/pgx/v5"
@@ -158,15 +159,93 @@ func (s *PostgresStore) ApplyAllMigrations(ctx context.Context,
 		return nil
 	}
 
-	return ApplyMigrations(ctx, s.BaseDB, s, migrations)
+	dbName, err := getDatabaseNameFromDSN(s.cfg.Dsn)
+	if err != nil {
+		return err
+	}
+
+	// Reuse a single migrate driver for every migration step. Creating
+	// one per step (as the per-call methods below do) reserves a pool
+	// connection each, so a small pool is exhausted and the
+	// migration-tracker transaction deadlocks with no timeout or error.
+	// See https://github.com/lightningnetwork/lnd/issues/11007.
+	//
+	// The driver runs on its own database handle, not the store's pool, and
+	// is closed once migrations finish. Its Close closes both the reserved
+	// connection and this handle, so nothing stays reserved on the store's
+	// pool afterwards, not even when there is nothing to migrate.
+	migrateDB, err := sql.Open("pgx", s.cfg.Dsn)
+	if err != nil {
+		return err
+	}
+
+	driver, err := pgx_migrate.WithInstance(
+		migrateDB, &pgx_migrate.Config{},
+	)
+	if err != nil {
+		if cerr := migrateDB.Close(); cerr != nil {
+			log.Errorf("Error closing migration db: %v", cerr)
+		}
+
+		return errPostgresMigration(err)
+	}
+	defer func() {
+		if cerr := driver.Close(); cerr != nil {
+			log.Errorf("Error closing migration driver: %v", cerr)
+		}
+	}()
+
+	executor := &postgresMigrationExecutor{
+		driver: driver,
+		dbName: dbName,
+	}
+
+	return ApplyMigrations(ctx, s.BaseDB, executor, migrations)
 }
 
 func errPostgresMigration(err error) error {
 	return fmt.Errorf("error creating postgres migration: %w", err)
 }
 
+// postgresMigrationExecutor implements MigrationExecutor with a single reusable
+// migrate driver, so all migration steps share one pool connection instead of
+// each reserving (and leaking) its own.
+type postgresMigrationExecutor struct {
+	driver database.Driver
+	dbName string
+}
+
+var _ MigrationExecutor = (*postgresMigrationExecutor)(nil)
+
+// ExecuteMigrations runs the migrations up to the given target using the shared
+// driver.
+func (p *postgresMigrationExecutor) ExecuteMigrations(
+	target MigrationTarget) error {
+
+	postgresFS := newReplacerFS(sqlSchemas, postgresSchemaReplacements)
+	return applyMigrations(
+		postgresFS, p.driver, "sqlc/migrations", p.dbName, target,
+	)
+}
+
+// GetSchemaVersion returns the current schema version using the shared driver.
+func (p *postgresMigrationExecutor) GetSchemaVersion() (int, bool, error) {
+	return p.driver.Version()
+}
+
+// SetSchemaVersion sets the schema version using the shared driver.
+func (p *postgresMigrationExecutor) SetSchemaVersion(version int,
+	dirty bool) error {
+
+	return p.driver.SetVersion(version, dirty)
+}
+
 // ExecuteMigrations runs migrations for the Postgres database, depending on the
 // target given, either all migrations or up to a given version.
+//
+// NOTE: This builds a fresh driver that holds a pool connection until the store
+// is closed. Do not call it in a loop across many schema versions, or the pool
+// is exhausted; ApplyAllMigrations reuses a single driver for that. See #11007.
 func (s *PostgresStore) ExecuteMigrations(target MigrationTarget) error {
 	dbName, err := getDatabaseNameFromDSN(s.cfg.Dsn)
 	if err != nil {