Skip to content

kvdb+sqldb: run read-only Postgres transactions at REPEATABLE READ - #10997

Open
Roasbeef wants to merge 4 commits into
lightningnetwork:masterfrom
Roasbeef:postgres-ro-repeatable-read
Open

kvdb+sqldb: run read-only Postgres transactions at REPEATABLE READ#10997
Roasbeef wants to merge 4 commits into
lightningnetwork:masterfrom
Roasbeef:postgres-ro-repeatable-read

Conversation

@Roasbeef

Copy link
Copy Markdown
Member

In this PR, we relax the isolation level for read-only Postgres transactions from SERIALIZABLE to REPEATABLE READ, across the kvdb SQL backend and both native SQL backends (sqldb and sqldb/v2). 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 itself and cause a concurrent writer to be aborted as a pivot. Since lnd is extremely read heavy, that adds up to a lot of needless abort pressure, which is the background noise behind reports like #10995 and #8531.

A READ ONLY REPEATABLE READ transaction observes a single consistent snapshot taken when the transaction starts, which is exactly the semantics bbolt's View transactions provide, so the read paths see precisely what they saw before. What changes is that such a transaction takes no predicate locks at all, can never fail with a serialization error, and no longer shows up in anyone else's conflict graph. Writers see fewer aborts as a direct consequence.

Each of the three transaction open sites funnels through a small txIsolationLevel helper that carries the reasoning in a comment. The shared sqlbase package also backs SQLite, so the change is gated: kvdb keys off the driver name, sqldb/v2 off the BackendType its BaseDB already carries, and sqldb v1 gains the same BackendType (which also resolves the isSQLite TODOs that were sitting in its tests). We verified the modernc SQLite driver ignores the requested isolation level entirely (its BeginTx never reads the field, and database/sql passes it through unvalidated for drivers implementing ConnBeginTx), so the gate is there to document intent rather than to dodge an error, and SQLite behavior is unchanged either way.

New tests assert SHOW transaction_isolation reports repeatable read for read-only transactions and serializable for read-write ones at all three layers, against a real Postgres via the existing embedded and docker fixtures, plus pure unit tests of the helper matrix.

This complements #10996: that PR stops a serialization failure from being reported to the peer, while this one removes the largest source of those failures in the first place. A follow up will harden the handful of write paths that an audit flagged as needing same-row conflicts before write transactions can also move to REPEATABLE READ.

Roasbeef added 4 commits July 27, 2026 13:27
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.
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.
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.
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.
@Roasbeef
Roasbeef force-pushed the postgres-ro-repeatable-read branch from 84c5e05 to 82d7f16 Compare July 27, 2026 20:48
Roasbeef added a commit to Roasbeef/lnd that referenced this pull request Jul 27, 2026
In this commit, we add a release notes entry for the four write paths
that were hardened so that read-write Postgres transactions can move to
repeatable read, which is the other half of the change that already put
read-only transactions there in lightningnetwork#10997.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant