Merge pull request lightningnetwork#9723 from ziggie1984/add-global-lock-walletdb

Add the global lock for the wallet db back for postgres

Author: Roasbeef

docs/release-notes/release-notes-0.19.0.md

@@ -475,6 +475,8 @@ the on going rate we'll permit.
 * [When running with neutrino as a backend with the kv-db backend `postgres`
 selected use postgres for the neutrino.db store](https://github.com/lightningnetwork/lnd/pull/9674).
 
+* [Add the global lock back to the wallet db for the postgres backend](https://github.com/lightningnetwork/lnd/pull/9723).
+
 ## Code Health
 
 * A code refactor that [moves all the graph related DB code out of the 

go.mod

@@ -216,3 +216,5 @@ replace google.golang.org/protobuf => github.com/lightninglabs/protobuf-go-hex-d
 go 1.23.6
 
 retract v0.0.2
+
+replace github.com/lightningnetwork/lnd/kvdb => ./kvdb

go.sum

@@ -371,8 +371,6 @@ github.com/lightningnetwork/lnd/fn/v2 v2.0.8 h1:r2SLz7gZYQPVc3IZhU82M66guz3Zk2oY
 github.com/lightningnetwork/lnd/fn/v2 v2.0.8/go.mod h1:TOzwrhjB/Azw1V7aa8t21ufcQmdsQOQMDtxVOQWNl8s=
 github.com/lightningnetwork/lnd/healthcheck v1.2.6 h1:1sWhqr93GdkWy4+6U7JxBfcyZIE78MhIHTJZfPx7qqI=
 github.com/lightningnetwork/lnd/healthcheck v1.2.6/go.mod h1:Mu02um4CWY/zdTOvFje7WJgJcHyX2zq/FG3MhOAiGaQ=
-github.com/lightningnetwork/lnd/kvdb v1.4.15 h1:3eN6uGcubvGB5itPp1D0D4uEEkIMYht3w0LDnqLzAWI=
-github.com/lightningnetwork/lnd/kvdb v1.4.15/go.mod h1:HW+bvwkxNaopkz3oIgBV6NEnV4jCEZCACFUcNg4xSjM=
 github.com/lightningnetwork/lnd/queue v1.1.1 h1:99ovBlpM9B0FRCGYJo6RSFDlt8/vOkQQZznVb18iNMI=
 github.com/lightningnetwork/lnd/queue v1.1.1/go.mod h1:7A6nC1Qrm32FHuhx/mi1cieAiBZo5O6l8IBIoQxvkz4=
 github.com/lightningnetwork/lnd/sqldb v1.0.9 h1:7OHi+Hui823mB/U9NzCdlZTAGSVdDCbjp33+6d/Q+G0=

kvdb/postgres/config.go

@@ -9,4 +9,5 @@ type Config struct {
 	Dsn            string        `long:"dsn" description:"Database connection string."`
 	Timeout        time.Duration `long:"timeout" description:"Database connection timeout. Set to zero to disable."`
 	MaxConnections int           `long:"maxconnections" description:"The maximum number of open connections to the database. Set to zero for unlimited."`
+	WithGlobalLock bool          `long:"withgloballock" description:"Use a global lock to ensure a single writer."`
 }

kvdb/postgres/db.go

@@ -28,6 +28,7 @@ func newPostgresBackend(ctx context.Context, config *Config, prefix string) (
 		Schema:                "public",
 		TableNamePrefix:       prefix,
 		SQLiteCmdReplacements: sqliteCmdReplacements,
+		WithTxLevelLock:       config.WithGlobalLock,
 	}
 
 	return sqlbase.NewSqlBackend(ctx, cfg)

kvdb/sqlbase/db.go

@@ -55,6 +55,13 @@ type Config struct {
 	// commands. Note that the sqlite keywords to be replaced are
 	// case-sensitive.
 	SQLiteCmdReplacements SQLiteCmdReplacements
+
+	// WithTxLevelLock when set will ensure that there is a transaction
+	// level lock.
+	//
+	// NOTE: Temporary, should be removed when all parts of the LND code
+	// are more resilient against concurrent db access..
+	WithTxLevelLock bool
 }
 
 // db holds a reference to the sql db connection.
@@ -79,6 +86,10 @@ type db struct {
 	// top-level buckets that have keys that cannot be mapped to a distinct
 	// sql table.
 	table string
+
+	// lock is the global write lock that ensures single writer. This is
+	// only used if cfg.WithTxLevelLock is set.
+	lock sync.RWMutex
 }
 
 // Enforce db implements the walletdb.DB interface.

kvdb/sqlbase/readwrite_tx.go

@@ -5,6 +5,7 @@ package sqlbase
 import (
 	"context"
 	"database/sql"
+	"sync"
 
 	"github.com/btcsuite/btcwallet/walletdb"
 )
@@ -19,11 +20,28 @@ type readWriteTx struct {
 
 	// active is true if the transaction hasn't been committed yet.
 	active bool
+
+	// locker is a pointer to the global db lock.
+	locker sync.Locker
 }
 
 // newReadWriteTx creates an rw transaction using a connection from the
 // specified pool.
 func newReadWriteTx(db *db, readOnly bool) (*readWriteTx, error) {
+	locker := newNoopLocker()
+	if db.cfg.WithTxLevelLock {
+		// Obtain the global lock instance. An alternative here is to
+		// obtain a database lock from Postgres. Unfortunately there is
+		// no database-level lock in Postgres, meaning that each table
+		// would need to be locked individually. Perhaps an advisory
+		// lock could perform this function too.
+		locker = &db.lock
+		if readOnly {
+			locker = db.lock.RLocker()
+		}
+	}
+	locker.Lock()
+
 	// Start the transaction. Don't use the timeout context because it would
 	// be applied to the transaction as a whole. If possible, mark the
 	// transaction as read-only to make sure that potential programming
@@ -36,13 +54,15 @@ func newReadWriteTx(db *db, readOnly bool) (*readWriteTx, error) {
 		},
 	)
 	if err != nil {
+		locker.Unlock()
 		return nil, err
 	}
 
 	return &readWriteTx{
 		db:     db,
 		tx:     tx,
 		active: true,
+		locker: locker,
 	}, nil
 }
 
@@ -74,6 +94,7 @@ func (tx *readWriteTx) Rollback() error {
 
 	// Unlock the transaction regardless of the error result.
 	tx.active = false
+	tx.locker.Unlock()
 	return err
 }
 
@@ -141,6 +162,7 @@ func (tx *readWriteTx) Commit() error {
 
 	// Unlock the transaction regardless of the error result.
 	tx.active = false
+	tx.locker.Unlock()
 
 	return err
 }
@@ -182,3 +204,25 @@ func (tx *readWriteTx) Exec(query string, args ...interface{}) (sql.Result,
 
 	return tx.tx.ExecContext(ctx, query, args...)
 }
+
+// noopLocker is an implementation of a no-op sync.Locker.
+type noopLocker struct{}
+
+// newNoopLocker creates a new noopLocker.
+func newNoopLocker() sync.Locker {
+	return &noopLocker{}
+}
+
+// Lock is a noop.
+//
+// NOTE: this is part of the sync.Locker interface.
+func (n *noopLocker) Lock() {
+}
+
+// Unlock is a noop.
+//
+// NOTE: this is part of the sync.Locker interface.
+func (n *noopLocker) Unlock() {
+}
+
+var _ sync.Locker = (*noopLocker)(nil)

lncfg/db.go

@@ -443,12 +443,21 @@ func (db *DB) GetBackends(ctx context.Context, chanDBPath,
 		}
 		closeFuncs[NSTowerServerDB] = postgresTowerServerBackend.Close
 
+		// The wallet subsystem is still not robust enough to run it
+		// without a single writer in postgres therefore we create a
+		// new config with the global lock enabled.
+		//
+		// NOTE: This is a temporary measure and should be removed as
+		// soon as the wallet code is more robust.
+		postgresConfigWalletDB := GetPostgresConfigKVDB(db.Postgres)
+		postgresConfigWalletDB.WithGlobalLock = true
+
 		postgresWalletBackend, err := kvdb.Open(
 			kvdb.PostgresBackendName, ctx,
-			postgresConfig, NSWalletDB,
+			postgresConfigWalletDB, NSWalletDB,
 		)
 		if err != nil {
-			return nil, fmt.Errorf("error opening postgres macaroon "+
+			return nil, fmt.Errorf("error opening postgres wallet "+
 				"DB: %v", err)
 		}
 		closeFuncs[NSWalletDB] = postgresWalletBackend.Close