From c3878bd3d2c49c3f2b54bcd9e209bf8ec37eb292 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Mon, 27 Jul 2026 11:46:17 -0700 Subject: [PATCH 1/4] kvdb: run read-only Postgres transactions at REPEATABLE READ In this commit, we relax the isolation level used for read-only transactions on the shared SQL kvdb backend when it's backed by Postgres. Read-write transactions are untouched and remain SERIALIZABLE. lnd currently opens every transaction at SERIALIZABLE. Under Postgres' serializable snapshot isolation, even a read-only transaction takes SIRead predicate locks and fully participates in the serialization conflict graph, so it can both suffer a 40001 abort and cause one for a concurrent writer by acting as a pivot. Since lnd is extremely read heavy, that adds up to a lot of needless abort pressure. REPEATABLE READ in Postgres is snapshot isolation. A read-only transaction still reads from a single consistent snapshot for its whole lifetime, taken when its first statement runs rather than at BEGIN. This is a genuine, if modest, weakening: that snapshot is no longer guaranteed to correspond to a serial ordering of the writers running alongside it, so the read-only transaction anomaly of Fekete and O'Neil becomes possible again. We're fine with that, because these read paths only ever consume a point-in-time view of the database, much like bbolt's View transactions do, and a read feeding a later write in a separate transaction was never protected across that boundary at any isolation level. What we get in return is that the transaction takes no part in the SSI conflict graph at all: no SIRead predicate locks, no exposure to SSI serialization failures, and no chance of aborting a concurrent writer as a pivot. The sqlbase package is shared with the SQLite backend, so we gate this on the driver name. SQLite is always effectively serializable and its driver ignores the requested isolation level entirely, so there's nothing to gain there. --- kvdb/sqlbase/db.go | 12 ++ kvdb/sqlbase/readwrite_tx.go | 39 ++++- kvdb/sqlbase/readwrite_tx_postgres_test.go | 187 +++++++++++++++++++++ kvdb/sqlbase/readwrite_tx_test.go | 82 +++++++++ 4 files changed, 319 insertions(+), 1 deletion(-) create mode 100644 kvdb/sqlbase/readwrite_tx_postgres_test.go create mode 100644 kvdb/sqlbase/readwrite_tx_test.go diff --git a/kvdb/sqlbase/db.go b/kvdb/sqlbase/db.go index 8ff7f979aff..5e0adced636 100644 --- a/kvdb/sqlbase/db.go +++ b/kvdb/sqlbase/db.go @@ -25,6 +25,11 @@ const ( // transaction if it fails with an error that permits transaction // repetition. DefaultNumTxRetries = 50 + + // postgresDriverName is the name of the registered SQL driver that is + // used whenever the shared SQL backend is backed by Postgres. It is + // used to select behavior that only applies to Postgres. + postgresDriverName = "pgx" ) // Config holds a set of configuration options of a sql database connection. @@ -95,6 +100,13 @@ type db struct { // Enforce db implements the walletdb.DB interface. var _ walletdb.DB = (*db)(nil) +// isPostgres returns true if the backend is backed by a Postgres database. The +// shared SQL backend is also used for SQLite, so any Postgres specific behavior +// needs to be gated on this. +func (d *db) isPostgres() bool { + return d.cfg.DriverName == postgresDriverName +} + var ( // dbConns is a global set of database connections. dbConns *dbConnSet diff --git a/kvdb/sqlbase/readwrite_tx.go b/kvdb/sqlbase/readwrite_tx.go index ec761931adc..beb783fb544 100644 --- a/kvdb/sqlbase/readwrite_tx.go +++ b/kvdb/sqlbase/readwrite_tx.go @@ -25,6 +25,43 @@ type readWriteTx struct { locker sync.Locker } +// txIsolationLevel returns the isolation level that a transaction against the +// given database should be opened with. +// +// Read-write transactions always run at SERIALIZABLE, since that is the level +// the kvdb abstraction has always promised for writers. Read-only transactions +// on Postgres are instead opened at REPEATABLE READ, which in Postgres is +// snapshot isolation: the transaction reads from a single consistent snapshot +// for its whole lifetime, taken when its first statement runs rather than at +// BEGIN. +// +// That is a real, if modest, weakening. Snapshot isolation is not +// serializability, so the reader is no longer guaranteed to observe a state +// that corresponds to some serial ordering of the writers running alongside it. +// The read-only transaction anomaly described by Fekete and O'Neil is once +// again permitted. We accept that here because our read paths only ever consume +// a point-in-time view of the database, much like bbolt's View transactions do, +// and never depended on being ordered against writers in other transactions. A +// read that feeds a later write in a separate transaction was never protected +// across that boundary at any isolation level. +// +// In exchange, a read-only REPEATABLE READ transaction takes no part in +// Postgres' serializable snapshot isolation conflict graph. It acquires no +// SIRead predicate locks, is not itself subject to SSI serialization failures, +// and can no longer cause a concurrent writer to be aborted as a pivot. Since +// lnd is extremely read heavy, this removes a large amount of needless abort +// pressure from the system. +// +// SQLite is always effectively serializable because it only ever admits a +// single writer, so there is nothing to gain there and we leave it alone. +func txIsolationLevel(db *db, readOnly bool) sql.IsolationLevel { + if readOnly && db.isPostgres() { + return sql.LevelRepeatableRead + } + + return sql.LevelSerializable +} + // newReadWriteTx creates an rw transaction using a connection from the // specified pool. func newReadWriteTx(db *db, readOnly bool) (*readWriteTx, error) { @@ -50,7 +87,7 @@ func newReadWriteTx(db *db, readOnly bool) (*readWriteTx, error) { context.Background(), &sql.TxOptions{ ReadOnly: readOnly, - Isolation: sql.LevelSerializable, + Isolation: txIsolationLevel(db, readOnly), }, ) if err != nil { diff --git a/kvdb/sqlbase/readwrite_tx_postgres_test.go b/kvdb/sqlbase/readwrite_tx_postgres_test.go new file mode 100644 index 00000000000..65f0ee387f9 --- /dev/null +++ b/kvdb/sqlbase/readwrite_tx_postgres_test.go @@ -0,0 +1,187 @@ +//go:build kvdb_postgres + +package sqlbase + +import ( + "context" + "fmt" + "io" + "path/filepath" + "testing" + "time" + + "github.com/btcsuite/btcwallet/walletdb" + embeddedpostgres "github.com/fergusstrange/embedded-postgres" + "github.com/stretchr/testify/require" +) + +const ( + // testPgPort is the port that the embedded Postgres instance used by + // this package listens on. A dedicated port is used here so that this + // test binary can run alongside the one of the kvdb/postgres package, + // which brings up its own instance. + testPgPort = 9877 + + // testPgDsnTemplate is the connection string template for the embedded + // Postgres instance above. + testPgDsnTemplate = "postgres://postgres:postgres@localhost:%d/" + + "postgres?sslmode=disable" + + // testPgMaxConnections is the maximum number of connections that the + // embedded Postgres instance accepts. + testPgMaxConnections = 20 +) + +// newPostgresTestBackend spins up an embedded Postgres instance and returns a +// SQL backend that is connected to it. +func newPostgresTestBackend(t *testing.T) *db { + t.Helper() + + Init(testPgMaxConnections) + + // Keep all of the state of the embedded instance contained in a + // temporary directory that is removed once the test completes. + runtimePath := t.TempDir() + + pg := embeddedpostgres.NewDatabase( + embeddedpostgres.DefaultConfig(). + Port(testPgPort). + RuntimePath(runtimePath). + DataPath(filepath.Join(runtimePath, "data")). + Logger(io.Discard). + StartParameters(map[string]string{ + "max_connections": fmt.Sprintf( + "%d", testPgMaxConnections, + ), + }), + ) + require.NoError(t, pg.Start()) + t.Cleanup(func() { + require.NoError(t, pg.Stop()) + }) + + backend, err := NewSqlBackend(context.Background(), &Config{ + DriverName: postgresDriverName, + Dsn: fmt.Sprintf(testPgDsnTemplate, testPgPort), + Timeout: time.Minute, + Schema: "public", + TableNamePrefix: "test", + SQLiteCmdReplacements: SQLiteCmdReplacements{ + "BLOB": "BYTEA", + "INTEGER PRIMARY KEY": "BIGSERIAL PRIMARY KEY", + }, + }) + require.NoError(t, err) + t.Cleanup(func() { + require.NoError(t, backend.Close()) + }) + + return backend +} + +// TestPostgresTxIsolationLevel asserts that the isolation level that Postgres +// itself reports for a transaction matches what we expect: read-only +// transactions run at repeatable read while read-write transactions remain +// serializable. We also assert the read-only flag that Postgres reports, so +// that dropping it from the tx options would be caught here as well. +func TestPostgresTxIsolationLevel(t *testing.T) { + backend := newPostgresTestBackend(t) + + tests := []struct { + name string + readOnly bool + expected string + expectedFlag string + }{ + { + name: "read-only", + readOnly: true, + expected: "repeatable read", + expectedFlag: "on", + }, + { + name: "read-write", + readOnly: false, + expected: "serializable", + expectedFlag: "off", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + tx, err := newReadWriteTx(backend, test.readOnly) + require.NoError(t, err) + defer func() { + require.NoError(t, tx.Rollback()) + }() + + var level string + row := tx.tx.QueryRow("SHOW transaction_isolation") + require.NoError(t, row.Scan(&level)) + require.Equal(t, test.expected, level) + + var readOnly string + row = tx.tx.QueryRow("SHOW transaction_read_only") + require.NoError(t, row.Scan(&readOnly)) + require.Equal(t, test.expectedFlag, readOnly) + }) + } +} + +// TestPostgresReadTxSnapshotStability asserts the property that the relaxed +// isolation level is chosen for, rather than just the knob itself: a read-only +// transaction keeps reading from the snapshot it started with, even after a +// concurrent writer on a different connection has committed over the same key. +func TestPostgresReadTxSnapshotStability(t *testing.T) { + backend := newPostgresTestBackend(t) + + var ( + bucketKey = []byte("snapshot") + key = []byte("key") + before = []byte("before") + after = []byte("after") + ) + + // Seed the key with its original value. + err := backend.Update(func(tx walletdb.ReadWriteTx) error { + bucket, err := tx.CreateTopLevelBucket(bucketKey) + if err != nil { + return err + } + + return bucket.Put(key, before) + }, func() {}) + require.NoError(t, err) + + readTx, err := newReadWriteTx(backend, true) + require.NoError(t, err) + defer func() { + require.NoError(t, readTx.Rollback()) + }() + + // Take the first read. Note that this is what pins the snapshot, since + // Postgres only acquires it once the first statement of a repeatable + // read transaction runs, not at BEGIN. + readBucket := readTx.ReadBucket(bucketKey) + require.NotNil(t, readBucket) + require.Equal(t, before, readBucket.Get(key)) + + // Now overwrite the key and commit, using a separate connection from + // the pool. + err = backend.Update(func(tx walletdb.ReadWriteTx) error { + return tx.ReadWriteBucket(bucketKey).Put(key, after) + }, func() {}) + require.NoError(t, err) + + // The write is visible to anyone starting fresh. + err = backend.View(func(tx walletdb.ReadTx) error { + require.Equal(t, after, tx.ReadBucket(bucketKey).Get(key)) + + return nil + }, func() {}) + require.NoError(t, err) + + // The long lived read transaction, however, must still observe the + // value that its snapshot was taken at. + require.Equal(t, before, readTx.ReadBucket(bucketKey).Get(key)) +} diff --git a/kvdb/sqlbase/readwrite_tx_test.go b/kvdb/sqlbase/readwrite_tx_test.go new file mode 100644 index 00000000000..579577532e8 --- /dev/null +++ b/kvdb/sqlbase/readwrite_tx_test.go @@ -0,0 +1,82 @@ +//go:build kvdb_postgres || (kvdb_sqlite && !(windows && (arm || 386)) && !(linux && (ppc64 || mips || mipsle || mips64))) + +package sqlbase + +import ( + "database/sql" + "testing" + + "github.com/stretchr/testify/require" +) + +// TestTxIsolationLevel tests that the isolation level of a transaction is only +// relaxed for read-only transactions on Postgres. Every other combination must +// remain fully serializable. +func TestTxIsolationLevel(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + driverName string + readOnly bool + expected sql.IsolationLevel + }{ + { + name: "postgres read-only", + driverName: "pgx", + readOnly: true, + expected: sql.LevelRepeatableRead, + }, + { + name: "postgres read-write", + driverName: "pgx", + readOnly: false, + expected: sql.LevelSerializable, + }, + { + name: "sqlite read-only", + driverName: "sqlite", + readOnly: true, + expected: sql.LevelSerializable, + }, + { + name: "sqlite read-write", + driverName: "sqlite", + readOnly: false, + expected: sql.LevelSerializable, + }, + + // Anything we don't positively recognize as Postgres must fall + // back to the strictest level. In particular "postgres" is not + // the driver name we register, so it must not opt in here. + { + name: "unset driver read-only", + driverName: "", + readOnly: true, + expected: sql.LevelSerializable, + }, + { + name: "unknown driver read-only", + driverName: "postgres", + readOnly: true, + expected: sql.LevelSerializable, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + db := &db{ + cfg: &Config{ + DriverName: test.driverName, + }, + } + + require.Equal( + t, test.expected, + txIsolationLevel(db, test.readOnly), + ) + }) + } +} From e02cd706878b5676c6a3a2d494917dda6e4dcdb7 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Mon, 27 Jul 2026 11:46:23 -0700 Subject: [PATCH 2/4] sqldb: run read-only Postgres transactions at REPEATABLE READ In this commit, we apply the same isolation level change to the native SQL backend that the previous commit applied to the kvdb SQL backend: read-only transactions on Postgres now run at REPEATABLE READ, while read-write transactions stay SERIALIZABLE. A read-only transaction still reads from a single consistent snapshot for its whole lifetime, taken at its first statement. It gives up the guarantee that the snapshot corresponds to a serial ordering of the concurrent writers, which these read paths never relied on, and in return stops participating in Postgres' serializable snapshot isolation conflict graph: no SIRead predicate locks, no exposure to SSI serialization failures, and no aborting a concurrent writer as a pivot. BaseDB is also used for SQLite, so it needs to know which backend it's talking to. We add a BackendType to BaseDB mirroring the one that sqldb/v2 already carries, and set it at the two construction sites. --- sqldb/interfaces.go | 64 ++++++++++++++++++- sqldb/interfaces_test.go | 135 +++++++++++++++++++++++++++++++++++++++ sqldb/postgres.go | 5 +- sqldb/sqlite.go | 5 +- 4 files changed, 203 insertions(+), 6 deletions(-) create mode 100644 sqldb/interfaces_test.go diff --git a/sqldb/interfaces.go b/sqldb/interfaces.go index 12ce63a1bb9..123d69d2f95 100644 --- a/sqldb/interfaces.go +++ b/sqldb/interfaces.go @@ -32,6 +32,21 @@ const ( DefaultMaxRetryDelay = time.Second ) +// BackendType is an enum that represents the type of database backend we're +// using. +type BackendType uint8 + +const ( + // BackendTypeUnknown indicates we're using an unknown backend. + BackendTypeUnknown BackendType = iota + + // BackendTypeSqlite indicates we're using a SQLite backend. + BackendTypeSqlite + + // BackendTypePostgres indicates we're using a Postgres backend. + BackendTypePostgres +) + // TxOptions represents a set of options one can use to control what type of // database transaction is created. Transaction can be either read or write. type TxOptions interface { @@ -409,16 +424,61 @@ type BaseDB struct { *sql.DB *sqlc.Queries + + // BackendType defines the type of database backend the database is. + BackendType BackendType +} + +// Backend returns the type of the database backend used. +func (s *BaseDB) Backend() BackendType { + return s.BackendType } // BeginTx wraps the normal sql specific BeginTx method with the TxOptions // interface. This interface is then mapped to the concrete sql tx options // struct. func (s *BaseDB) BeginTx(ctx context.Context, opts TxOptions) (*sql.Tx, error) { + readOnly := opts.ReadOnly() + sqlOptions := sql.TxOptions{ - Isolation: sql.LevelSerializable, - ReadOnly: opts.ReadOnly(), + Isolation: txIsolationLevel(s.BackendType, readOnly), + ReadOnly: readOnly, } return s.DB.BeginTx(ctx, &sqlOptions) } + +// txIsolationLevel returns the isolation level that a transaction against the +// given backend should be opened with. +// +// Read-write transactions always run at SERIALIZABLE. Read-only transactions on +// Postgres are instead opened at REPEATABLE READ, which in Postgres is snapshot +// isolation: the transaction reads from a single consistent snapshot for its +// whole lifetime, taken when its first statement runs rather than at BEGIN. +// +// That is a real, if modest, weakening. Snapshot isolation is not +// serializability, so the reader is no longer guaranteed to observe a state +// that corresponds to some serial ordering of the writers running alongside it. +// The read-only transaction anomaly described by Fekete and O'Neil is once +// again permitted. We accept that here because our read paths only ever consume +// a point-in-time view of the database and never depended on being ordered +// against writers in other transactions. A read that feeds a later write in a +// separate transaction was never protected across that boundary at any +// isolation level. +// +// In exchange, a read-only REPEATABLE READ transaction takes no part in +// Postgres' serializable snapshot isolation conflict graph. It acquires no +// SIRead predicate locks, is not itself subject to SSI serialization failures, +// and can no longer cause a concurrent writer to be aborted as a pivot. Since +// lnd is extremely read heavy, this removes a large amount of needless abort +// pressure from the system. +// +// SQLite is always effectively serializable because it only ever admits a +// single writer, so there is nothing to gain there and we leave it alone. +func txIsolationLevel(backend BackendType, readOnly bool) sql.IsolationLevel { + if readOnly && backend == BackendTypePostgres { + return sql.LevelRepeatableRead + } + + return sql.LevelSerializable +} diff --git a/sqldb/interfaces_test.go b/sqldb/interfaces_test.go new file mode 100644 index 00000000000..820059b3836 --- /dev/null +++ b/sqldb/interfaces_test.go @@ -0,0 +1,135 @@ +package sqldb + +import ( + "database/sql" + "testing" + + "github.com/stretchr/testify/require" +) + +// TestTxIsolationLevel tests that the isolation level of a transaction is only +// relaxed for read-only transactions on Postgres. Every other combination must +// remain fully serializable. +func TestTxIsolationLevel(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + backend BackendType + readOnly bool + expected sql.IsolationLevel + }{ + { + name: "postgres read-only", + backend: BackendTypePostgres, + readOnly: true, + expected: sql.LevelRepeatableRead, + }, + { + name: "postgres read-write", + backend: BackendTypePostgres, + readOnly: false, + expected: sql.LevelSerializable, + }, + { + name: "sqlite read-only", + backend: BackendTypeSqlite, + readOnly: true, + expected: sql.LevelSerializable, + }, + { + name: "sqlite read-write", + backend: BackendTypeSqlite, + readOnly: false, + expected: sql.LevelSerializable, + }, + { + name: "unknown read-only", + backend: BackendTypeUnknown, + readOnly: true, + expected: sql.LevelSerializable, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + require.Equal( + t, test.expected, + txIsolationLevel(test.backend, test.readOnly), + ) + }) + } +} + +// TestBeginTxIsolationLevel asserts that the isolation level that the database +// itself reports for a transaction opened through BeginTx matches what we +// expect. On Postgres, read-only transactions run at repeatable read while +// read-write transactions remain serializable. We also assert the read-only +// flag that Postgres reports, so that dropping it from the tx options would be +// caught here as well. +func TestBeginTxIsolationLevel(t *testing.T) { + t.Parallel() + ctx := t.Context() + + db := NewTestDB(t).GetBaseDB() + + tests := []struct { + name string + opts TxOptions + expected string + expectedFlag string + }{ + { + name: "read-only", + opts: ReadTxOpt(), + expected: "repeatable read", + expectedFlag: "on", + }, + { + name: "read-write", + opts: WriteTxOpt(), + expected: "serializable", + expectedFlag: "off", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + tx, err := db.BeginTx(ctx, test.opts) + + // SQLite has no notion of a transaction isolation + // level and its driver ignores the one we request + // outright. All we can assert there is that opening + // the transaction still succeeds, which is what would + // break if the driver ever started rejecting the + // levels we ask for. + require.NoError(t, err) + defer func() { + require.NoError(t, tx.Rollback()) + }() + + if isSQLite { + require.Equal(t, BackendTypeSqlite, db.Backend()) + return + } + + require.Equal(t, BackendTypePostgres, db.Backend()) + + var level string + row := tx.QueryRowContext( + ctx, "SHOW transaction_isolation", + ) + require.NoError(t, row.Scan(&level)) + require.Equal(t, test.expected, level) + + var readOnly string + row = tx.QueryRowContext( + ctx, "SHOW transaction_read_only", + ) + require.NoError(t, row.Scan(&readOnly)) + require.Equal(t, test.expectedFlag, readOnly) + }) + } +} diff --git a/sqldb/postgres.go b/sqldb/postgres.go index 70dba82a1a0..8e8c5eca31f 100644 --- a/sqldb/postgres.go +++ b/sqldb/postgres.go @@ -136,8 +136,9 @@ func NewPostgresStore(cfg *PostgresConfig) (*PostgresStore, error) { return &PostgresStore{ cfg: cfg, BaseDB: &BaseDB{ - DB: db, - Queries: queries, + DB: db, + Queries: queries, + BackendType: BackendTypePostgres, }, }, nil } diff --git a/sqldb/sqlite.go b/sqldb/sqlite.go index 1ed26810d02..c32797046e2 100644 --- a/sqldb/sqlite.go +++ b/sqldb/sqlite.go @@ -144,8 +144,9 @@ func NewSqliteStore(cfg *SqliteConfig, dbPath string) (*SqliteStore, error) { s := &SqliteStore{ cfg: cfg, BaseDB: &BaseDB{ - DB: db, - Queries: queries, + DB: db, + Queries: queries, + BackendType: BackendTypeSqlite, }, } From bb25116383eafada3a673f86254ced0902350cb8 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Mon, 27 Jul 2026 11:46:27 -0700 Subject: [PATCH 3/4] sqldb/v2: run read-only Postgres transactions at REPEATABLE READ In this commit, we mirror the isolation level change into sqldb/v2. BaseDB already carries a BackendType here, so we can gate directly on it: read-only transactions on Postgres are opened at REPEATABLE READ, everything else stays SERIALIZABLE. --- sqldb/v2/interfaces.go | 41 +++++++++++++++++- sqldb/v2/interfaces_db_test.go | 78 ++++++++++++++++++++++++++++++++++ sqldb/v2/interfaces_test.go | 56 ++++++++++++++++++++++++ 3 files changed, 173 insertions(+), 2 deletions(-) create mode 100644 sqldb/v2/interfaces_db_test.go diff --git a/sqldb/v2/interfaces.go b/sqldb/v2/interfaces.go index 0bf6da88194..8fac8daabfa 100644 --- a/sqldb/v2/interfaces.go +++ b/sqldb/v2/interfaces.go @@ -452,14 +452,51 @@ type BaseDB struct { // interface. This interface is then mapped to the concrete sql tx options // struct. func (s *BaseDB) BeginTx(ctx context.Context, opts TxOptions) (*sql.Tx, error) { + readOnly := opts.ReadOnly() + sqlOptions := sql.TxOptions{ - Isolation: sql.LevelSerializable, - ReadOnly: opts.ReadOnly(), + Isolation: txIsolationLevel(s.BackendType, readOnly), + ReadOnly: readOnly, } return s.DB.BeginTx(ctx, &sqlOptions) } +// txIsolationLevel returns the isolation level that a transaction against the +// given backend should be opened with. +// +// Read-write transactions always run at SERIALIZABLE. Read-only transactions on +// Postgres are instead opened at REPEATABLE READ, which in Postgres is snapshot +// isolation: the transaction reads from a single consistent snapshot for its +// whole lifetime, taken when its first statement runs rather than at BEGIN. +// +// That is a real, if modest, weakening. Snapshot isolation is not +// serializability, so the reader is no longer guaranteed to observe a state +// that corresponds to some serial ordering of the writers running alongside it. +// The read-only transaction anomaly described by Fekete and O'Neil is once +// again permitted. We accept that here because our read paths only ever consume +// a point-in-time view of the database and never depended on being ordered +// against writers in other transactions. A read that feeds a later write in a +// separate transaction was never protected across that boundary at any +// isolation level. +// +// In exchange, a read-only REPEATABLE READ transaction takes no part in +// Postgres' serializable snapshot isolation conflict graph. It acquires no +// SIRead predicate locks, is not itself subject to SSI serialization failures, +// and can no longer cause a concurrent writer to be aborted as a pivot. Since +// lnd is extremely read heavy, this removes a large amount of needless abort +// pressure from the system. +// +// SQLite is always effectively serializable because it only ever admits a +// single writer, so there is nothing to gain there and we leave it alone. +func txIsolationLevel(backend BackendType, readOnly bool) sql.IsolationLevel { + if readOnly && backend == BackendTypePostgres { + return sql.LevelRepeatableRead + } + + return sql.LevelSerializable +} + // Backend returns the type of the database backend used. func (s *BaseDB) Backend() BackendType { return s.BackendType diff --git a/sqldb/v2/interfaces_db_test.go b/sqldb/v2/interfaces_db_test.go new file mode 100644 index 00000000000..d04a95a3696 --- /dev/null +++ b/sqldb/v2/interfaces_db_test.go @@ -0,0 +1,78 @@ +//go:build !js && !(windows && (arm || 386)) && !(linux && (ppc64 || mips || mipsle || mips64)) && !(netbsd || openbsd) + +package sqldb + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +// TestBeginTxIsolationLevel asserts that the isolation level that the database +// itself reports for a transaction opened through BeginTx matches what we +// expect. On Postgres, read-only transactions run at repeatable read while +// read-write transactions remain serializable. We also assert the read-only +// flag that Postgres reports, so that dropping it from the tx options would be +// caught here as well. +func TestBeginTxIsolationLevel(t *testing.T) { + t.Parallel() + ctx := t.Context() + + db := NewTestDB(t, nil).GetBaseDB() + + tests := []struct { + name string + opts TxOptions + expected string + expectedFlag string + }{ + { + name: "read-only", + opts: ReadTxOpt(), + expected: "repeatable read", + expectedFlag: "on", + }, + { + name: "read-write", + opts: WriteTxOpt(), + expected: "serializable", + expectedFlag: "off", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + tx, err := db.BeginTx(ctx, test.opts) + + // SQLite has no notion of a transaction isolation + // level and its driver ignores the one we request + // outright. All we can assert there is that opening + // the transaction still succeeds, which is what would + // break if the driver ever started rejecting the + // levels we ask for. + require.NoError(t, err) + defer func() { + require.NoError(t, tx.Rollback()) + }() + + if db.Backend() != BackendTypePostgres { + require.Equal(t, BackendTypeSqlite, db.Backend()) + return + } + + var level string + row := tx.QueryRowContext( + ctx, "SHOW transaction_isolation", + ) + require.NoError(t, row.Scan(&level)) + require.Equal(t, test.expected, level) + + var readOnly string + row = tx.QueryRowContext( + ctx, "SHOW transaction_read_only", + ) + require.NoError(t, row.Scan(&readOnly)) + require.Equal(t, test.expectedFlag, readOnly) + }) + } +} diff --git a/sqldb/v2/interfaces_test.go b/sqldb/v2/interfaces_test.go index 024b367465c..2ca7b7fcb20 100644 --- a/sqldb/v2/interfaces_test.go +++ b/sqldb/v2/interfaces_test.go @@ -46,3 +46,59 @@ func TestTransactionExecutorBackend(t *testing.T) { require.Equal(t, BackendTypePostgres, executor.Backend()) } + +// TestTxIsolationLevel tests that the isolation level of a transaction is only +// relaxed for read-only transactions on Postgres. Every other combination must +// remain fully serializable. +func TestTxIsolationLevel(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + backend BackendType + readOnly bool + expected sql.IsolationLevel + }{ + { + name: "postgres read-only", + backend: BackendTypePostgres, + readOnly: true, + expected: sql.LevelRepeatableRead, + }, + { + name: "postgres read-write", + backend: BackendTypePostgres, + readOnly: false, + expected: sql.LevelSerializable, + }, + { + name: "sqlite read-only", + backend: BackendTypeSqlite, + readOnly: true, + expected: sql.LevelSerializable, + }, + { + name: "sqlite read-write", + backend: BackendTypeSqlite, + readOnly: false, + expected: sql.LevelSerializable, + }, + { + name: "unknown read-only", + backend: BackendTypeUnknown, + readOnly: true, + expected: sql.LevelSerializable, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + require.Equal( + t, test.expected, + txIsolationLevel(test.backend, test.readOnly), + ) + }) + } +} From 82d7f16e25ed8e6903820d9e04505469f5dc707e Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Mon, 27 Jul 2026 11:46:27 -0700 Subject: [PATCH 4/4] docs: document the read-only Postgres isolation change In this commit, we add the release notes entry for the isolation level change, along with an operator facing note in docs/postgres.md. The latter spells out that reads now run under snapshot isolation, and warns that lnd holds some read transactions open for a long time. The graph cache load and each pathfinding GraphSession keep a snapshot pinned for their whole duration, which is worth knowing before setting idle_in_transaction_session_timeout or statement_timeout. --- docs/postgres.md | 22 ++++++++++++++++++++++ docs/release-notes/release-notes-0.22.0.md | 17 +++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/docs/postgres.md b/docs/postgres.md index 89b16ebcf26..71cd01a4c00 100644 --- a/docs/postgres.md +++ b/docs/postgres.md @@ -52,6 +52,28 @@ resource exhaustion in case LND experiencing high concurrent load: * `db.postgres.channeldb-with-global-lock=false` to run the channeldb_kv table with a single writer (default is false). +## Transaction isolation + +`lnd` opens read-write transactions at the `SERIALIZABLE` isolation level. +Read-only transactions are opened at `REPEATABLE READ`, which in Postgres is +snapshot isolation: the transaction reads from a single consistent snapshot, +taken when its first statement runs. Reads therefore acquire no `SIRead` +predicate locks and take no part in Postgres' serializable snapshot isolation +conflict graph, which substantially cuts the number of `40001` serialization +failures that `lnd` and its concurrent writers have to retry through. + +Operators should be aware that some of `lnd`'s read transactions are long +lived. Loading the graph cache at startup and each `GraphSession` used for +pathfinding hold a read transaction open for their full duration, and each +holds its snapshot for that whole time. Two consequences follow. First, +Postgres cannot vacuum row versions that are still visible to an open snapshot, +so a very slow or stuck read transaction delays cleanup and can bloat tables. +Second, such a session sits in the `idle in transaction` state whenever `lnd` +is computing between queries, so if `idle_in_transaction_session_timeout` is +configured it must be generous enough to cover a full pathfinding pass or +Postgres will terminate the transaction mid-flight. The same caution applies to +`statement_timeout` for the individual queries these transactions run. + ## 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. diff --git a/docs/release-notes/release-notes-0.22.0.md b/docs/release-notes/release-notes-0.22.0.md index 750a20069cd..b9bf5e57878 100644 --- a/docs/release-notes/release-notes-0.22.0.md +++ b/docs/release-notes/release-notes-0.22.0.md @@ -96,6 +96,23 @@ ## Performance Improvements +* [Read-only Postgres transactions now run at `REPEATABLE READ` instead of + `SERIALIZABLE`](https://github.com/lightningnetwork/lnd/pull/10997). In + Postgres that is snapshot isolation: a read-only transaction still reads from + a single consistent snapshot for its whole lifetime, taken when its first + statement runs. That snapshot is no longer guaranteed to correspond to a + serial ordering of the writers running alongside it, which is acceptable + because `lnd`'s read paths only consume a point-in-time view and never + depended on being ordered against writers in other transactions. In exchange, + such a transaction takes no part in Postgres' serializable snapshot isolation + conflict graph: it acquires no `SIRead` predicate locks, is not itself subject + to SSI serialization failures, and can no longer cause a concurrent writer to + be aborted as a pivot. Since `lnd` is very read heavy, this removes a large + amount of needless abort pressure. Read-write transactions are unaffected and + remain `SERIALIZABLE`, and the SQLite backend is untouched. See + [docs/postgres.md](../postgres.md) for the operator-facing note on long-lived + read transactions. + ## Deprecations # Technical and Architectural Updates