Merge pull request #10394 from ziggie1984/add-back-globallock

lncfg: readd global lock for postgres and the channeldb_kv table

docs/postgres.md

@@ -42,6 +42,16 @@ db.postgres.timeout=0
 Connection timeout is disabled, to account for situations where the database
 might be slow for unexpected reasons.
 
+Moreover for particular kv tables we also add the option to access the
+tables via a global lock (single wirter). This is a temorpary measure until 
+these particular tables have a native sql schema. This helps to mitigate
+resource exhaustion in case LND experiencing high concurrent load:
+
+* `db.postgres.walletdb-with-global-lock=true` to run LND with a single writer
+  for the walletdb_kv table (default is true).
+* `db.postgres.channeldb-with-global-lock=false` to run the channeldb_kv table
+  with a single writer (default is false).
+
 ## Important note about replication
 
 In case a replication architecture is planned, streaming replication should be avoided, as the master does not verify the replica is indeed identical, but it will only forward the edits queue, and let the slave catch up autonomously; synchronous mode, albeit slower, is paramount for `lnd` data integrity across the copies, as it will finalize writes only after the slave confirmed successful replication.

docs/release-notes/release-notes-0.20.1.md

@@ -51,6 +51,15 @@
 
 ## Performance Improvements
 
+* [Added new Postgres configuration 
+  options](https://github.com/lightningnetwork/lnd/pull/10394) 
+ `db.postgres.channeldb-with-global-lock` and 
+ `db.postgres.walletdb-with-global-lock` to allow fine-grained control over
+  database concurrency. The channeldb global lock defaults to `false` to enable
+  concurrent access, while the wallet global lock defaults to `true` to maintain
+  safe single-writer behavior until the wallet subsystem is fully 
+  concurrent-safe.
+
 ## Deprecations
 
 # Technical and Architectural Updates

lncfg/db.go

@@ -115,7 +115,15 @@ func DefaultDB() *DB {
 		},
 		Postgres: &sqldb.PostgresConfig{
 			MaxConnections: defaultPostgresMaxConnections,
-			QueryConfig:    *sqldb.DefaultPostgresConfig(),
+			// Normally we don't use a global lock for channeldb
+			// access, but if a user encounters huge concurrency
+			// issues, they can enable this to use a global lock.
+			ChannelDBWithGlobalLock: false,
+			// Default to true to maintain safe single-writer
+			// behavior until the wallet subsystem is upgraded to
+			// a native sql schema.
+			WalletDBWithGlobalLock: true,
+			QueryConfig:            *sqldb.DefaultPostgresConfig(),
 		},
 		Sqlite: &sqldb.SqliteConfig{
 			MaxConnections: defaultSqliteMaxConnections,
@@ -400,9 +408,15 @@ func (db *DB) GetBackends(ctx context.Context, chanDBPath,
 		// users to native SQL.
 		postgresConfig := GetPostgresConfigKVDB(db.Postgres)
 
+		// Create a separate config for channeldb with the global lock
+		// setting if configured.
+		postgresConfigChannelDB := GetPostgresConfigKVDB(db.Postgres)
+		postgresConfigChannelDB.WithGlobalLock = db.Postgres.
+			ChannelDBWithGlobalLock
+
 		postgresBackend, err := kvdb.Open(
 			kvdb.PostgresBackendName, ctx,
-			postgresConfig, NSChannelDB,
+			postgresConfigChannelDB, NSChannelDB,
 		)
 		if err != nil {
 			return nil, fmt.Errorf("error opening postgres graph "+
@@ -450,14 +464,11 @@ func (db *DB) GetBackends(ctx context.Context, chanDBPath,
 		}
 		closeFuncs[NSTowerServerDB] = postgresTowerServerBackend.Close
 
-		// The wallet subsystem is still not robust enough to run it
+		// Create a separate config for wallet with the global lock
-		// without a single writer in postgres therefore we create a
+		// setting if configured.
-		// 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
+		postgresConfigWalletDB.WithGlobalLock = db.Postgres.
+			WalletDBWithGlobalLock
 
 		postgresWalletBackend, err := kvdb.Open(
 			kvdb.PostgresBackendName, ctx,

sample-lnd.conf

@@ -1625,6 +1625,17 @@
 ; Whether to skip executing schema migrations.
 ; db.postgres.skipmigrations=false
 
+; Use a global lock for channeldb access. This ensures only a single writer at
+; a time but reduces concurrency. This is a temporary workaround until the
+; revocation log is migrated to native SQL.
+; db.postgres.channeldb-with-global-lock=false
+
+
+; Use a global lock for wallet database access. This is a temporary workaround 
+; until the wallet subsystem is upgraded to a native sql schema.
+; db.postgres.walletdb-with-global-lock=true
+
+
 ; The maximum number of elements to use in a native-SQL batch query IN clause.
 ; db.postgres.query.max-batch-size=5000
 

scripts/check-sample-lnd-conf.sh

@@ -59,7 +59,7 @@ OPTIONS_NO_LND_DEFAULT_VALUE_CHECK="channel-max-fee-exposure adminmacaroonpath \
     backupfilepath maxchansize bitcoin.chaindir bitcoin.defaultchanconfs \
     bitcoin.defaultremotedelay bitcoin.dnsseed signrpc.signermacaroonpath \
     walletrpc.walletkitmacaroonpath chainrpc.notifiermacaroonpath \
-    routerrpc.routermacaroonpath" 
+    routerrpc.routermacaroonpath db.postgres.walletdb-with-global-lock" 
 
 
 # EXITCODE is returned at the end after all checks are performed and set to 1 

sqldb/config.go

@@ -44,11 +44,13 @@ func (p *SqliteConfig) Validate() error {
 //
 //nolint:ll
 type PostgresConfig struct {
-	Dsn            string        `long:"dsn" description:"Database connection string."`
+	Dsn                     string        `long:"dsn" description:"Database connection string."`
-	Timeout        time.Duration `long:"timeout" description:"Database connection timeout. Set to zero to disable."`
+	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."`
+	MaxConnections          int           `long:"maxconnections" description:"The maximum number of open connections to the database. Set to zero for unlimited."`
-	SkipMigrations bool          `long:"skipmigrations" description:"Skip applying migrations on startup."`
+	SkipMigrations          bool          `long:"skipmigrations" description:"Skip applying migrations on startup."`
-	QueryConfig    `group:"query" namespace:"query"`
+	ChannelDBWithGlobalLock bool          `long:"channeldb-with-global-lock" description:"Use a global lock for channeldb access. This ensures only a single writer at a time but reduces concurrency. This is a temporary workaround until the revocation log is migrated to a native sql schema."`
+	WalletDBWithGlobalLock  bool          `long:"walletdb-with-global-lock" description:"Use a global lock for wallet database access. This ensures only a single writer at a time but reduces concurrency. This is a temporary workaround until the wallet subsystem is upgraded to a native sql schema."`
+	QueryConfig             `group:"query" namespace:"query"`
 }
 
 // Validate checks that the PostgresConfig values are valid.