db: run read-only Postgres transactions at REPEATABLE READ

db/CLAUDE.md

@@ -216,3 +216,11 @@ when adding one.
 ## Deep Docs
 
 - [ARCHITECTURE.md](../ARCHITECTURE.md) — System-wide package map.
+- [docs/postgres_isolation.md](../docs/postgres_isolation.md) — Isolation
+  policy: read-only Postgres transactions run at `REPEATABLE READ` with
+  `READ ONLY` (no `SIRead` predicate locks, never a 40001), writers stay
+  `SERIALIZABLE`. Also holds the write-path snapshot-isolation audit and the
+  inventory of the six partial unique indexes that any new `ON CONFLICT`
+  target has to be checked against, along with the caveat that a conflict
+  target can also miss a plain unique constraint declared inline in a
+  `CREATE TABLE`.

db/interfaces.go

@@ -367,13 +367,57 @@ type BaseDB struct {
 	*sqlc.Queries
 }
 
+// txIsolationLevel returns the isolation level a transaction against the given
+// backend should be opened with.
+//
+// Read-write transactions always run at SERIALIZABLE, which is the level the
+// storage layer has always promised 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 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
+// the daemon 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 sqlc.BackendType,
+	readOnly bool) sql.IsolationLevel {
+
+	if readOnly && backend == sqlc.BackendTypePostgres {
+		return sql.LevelRepeatableRead
+	}
+
+	return sql.LevelSerializable
+}
+
 // 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()
+
+	// The read-only flag is not just advisory here: Postgres only skips
+	// predicate lock acquisition for a transaction that is actually
+	// declared READ ONLY, so the flag is what makes the relaxed isolation
+	// level worth anything.
 	sqlOptions := sql.TxOptions{
-		ReadOnly:  opts.ReadOnly(),
-		Isolation: sql.LevelSerializable,
+		ReadOnly:  readOnly,
+		Isolation: txIsolationLevel(s.Backend(), readOnly),
 	}
 
 	return s.DB.BeginTx(ctx, &sqlOptions)

db/interfaces_postgres_test.go

@@ -0,0 +1,88 @@
+//go:build test_postgres
+
+package db
+
+import (
+	"database/sql"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+// txSessionState reads back the isolation level and read-only access mode that
+// Postgres actually applied to the given transaction. Asserting on the server's
+// own view is the only way to prove that the requested sql.TxOptions survived
+// the trip through the pgx stdlib driver and into the BEGIN statement.
+func txSessionState(t *testing.T, tx *sql.Tx) (string, string) {
+	t.Helper()
+
+	var isolation string
+	err := tx.QueryRow("SHOW transaction_isolation").Scan(&isolation)
+	require.NoError(t, err)
+
+	var readOnly string
+	err = tx.QueryRow("SHOW transaction_read_only").Scan(&readOnly)
+	require.NoError(t, err)
+
+	return isolation, readOnly
+}
+
+// TestPostgresBeginTxIsolation asserts that read-only transactions against
+// Postgres are opened as a read-only REPEATABLE READ tx, while writers stay
+// SERIALIZABLE. The read-only flag matters as much as the level, because
+// Postgres only skips SIRead predicate lock acquisition for a transaction that
+// is genuinely declared read only.
+//
+// The subtests deliberately share one store. Each store costs a docker
+// container, and the fixture derives its port from a docker port binding that
+// is not always populated under load.
+func TestPostgresBeginTxIsolation(t *testing.T) {
+	t.Parallel()
+
+	ctx := t.Context()
+	store := NewTestPostgresDB(t)
+
+	t.Run("read-only", func(t *testing.T) {
+		tx, err := store.BeginTx(ctx, ReadTxOption())
+		require.NoError(t, err)
+		defer func() {
+			require.NoError(t, tx.Rollback())
+		}()
+
+		isolation, readOnly := txSessionState(t, tx)
+		require.Equal(t, "repeatable read", isolation)
+		require.Equal(t, "on", readOnly)
+	})
+
+	t.Run("read-write", func(t *testing.T) {
+		tx, err := store.BeginTx(ctx, WriteTxOption())
+		require.NoError(t, err)
+		defer func() {
+			require.NoError(t, tx.Rollback())
+		}()
+
+		isolation, readOnly := txSessionState(t, tx)
+		require.Equal(t, "serializable", isolation)
+		require.Equal(t, "off", readOnly)
+	})
+
+	// The READ ONLY access mode is enforced by the server rather than being
+	// advisory. This is the property that justifies the audit assumption
+	// that every ReadTxOption call site is truly read-only: if one of them
+	// ever starts writing, it fails loudly instead of silently relying on
+	// snapshot isolation.
+	t.Run("read-only rejects writes", func(t *testing.T) {
+		tx, err := store.BeginTx(ctx, ReadTxOption())
+		require.NoError(t, err)
+		defer func() {
+			require.NoError(t, tx.Rollback())
+		}()
+
+		_, err = tx.ExecContext(
+			ctx, "INSERT INTO chain_info (id, chain_name, "+
+				"genesis_hash) VALUES (2, 'nope', '\\x01')",
+		)
+		require.Error(t, err)
+		require.ErrorContains(t, err, "read-only transaction")
+	})
+}

db/interfaces_test.go

@@ -56,3 +56,86 @@ func TestTransactionExecutorUsesContextTx(t *testing.T) {
 	require.Error(t, err)
 	require.True(t, errors.Is(err, sql.ErrNoRows))
 }
+
+// TestTxIsolationLevel asserts that the isolation level is only relaxed for
+// read-only transactions on Postgres. Every other combination has to stay
+// fully serializable, in particular SQLite, which the same BaseDB backs.
+func TestTxIsolationLevel(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name     string
+		backend  sqlc.BackendType
+		readOnly bool
+		expected sql.IsolationLevel
+	}{
+		{
+			name:     "postgres read-only",
+			backend:  sqlc.BackendTypePostgres,
+			readOnly: true,
+			expected: sql.LevelRepeatableRead,
+		},
+		{
+			name:     "postgres read-write",
+			backend:  sqlc.BackendTypePostgres,
+			readOnly: false,
+			expected: sql.LevelSerializable,
+		},
+		{
+			name:     "sqlite read-only",
+			backend:  sqlc.BackendTypeSqlite,
+			readOnly: true,
+			expected: sql.LevelSerializable,
+		},
+		{
+			name:     "sqlite read-write",
+			backend:  sqlc.BackendTypeSqlite,
+			readOnly: false,
+			expected: sql.LevelSerializable,
+		},
+		{
+			name:     "unknown read-only",
+			backend:  sqlc.BackendTypeUnknown,
+			readOnly: true,
+			expected: sql.LevelSerializable,
+		},
+		{
+			name:     "unknown read-write",
+			backend:  sqlc.BackendTypeUnknown,
+			readOnly: false,
+			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),
+			)
+		})
+	}
+}
+
+// TestBeginTxReadOnlyUsable asserts that a read-only transaction against the
+// active test backend can still be opened and read from. On Postgres this
+// exercises the relaxed read-only REPEATABLE READ path, and on SQLite it
+// guards against a regression from the backend gate.
+func TestBeginTxReadOnlyUsable(t *testing.T) {
+	t.Parallel()
+
+	ctx := t.Context()
+	store := NewTestDB(t)
+
+	tx, err := store.BeginTx(ctx, ReadTxOption())
+	require.NoError(t, err)
+	defer func() {
+		require.NoError(t, tx.Rollback())
+	}()
+
+	var one int
+	require.NoError(t, tx.QueryRowContext(ctx, "SELECT 1").Scan(&one))
+	require.Equal(t, 1, one)
+}

db/sqlerrors.go

@@ -111,14 +111,95 @@ func classifyPostgresError(code string, dbErr error) error {
 	}
 }
 
+// PgErrorDetail extracts the Detail field of an underlying Postgres error, if
+// there is one. The Error method of pgconn.PgError renders only the severity,
+// the message and the SQLSTATE code, so the Detail is otherwise dropped on the
+// floor before it ever reaches a log line.
+//
+// That detail is the only thing that tells two very different 40001 aborts
+// apart. A true serializable snapshot isolation abort carries a "Reason code"
+// naming the transaction's role in the conflict graph, whereas an ordinary
+// write-write conflict on the same row carries none. Once read-only
+// transactions stop taking predicate locks, this is the signal that says
+// whether a given write path still depends on SSI or would be equally happy at
+// REPEATABLE READ. For a 23505 the detail names the constraint and the
+// conflicting key values, which is what identifies a lost creation race.
+func PgErrorDetail(err error) string {
+	var pgErrV4 *pgconnv4.PgError
+	if errors.As(err, &pgErrV4) {
+		return pgErrV4.Detail
+	}
+
+	var pgErrV5 *pgconnv5.PgError
+	if errors.As(err, &pgErrV5) {
+		return pgErrV5.Detail
+	}
+
+	return ""
+}
+
+// PgErrorConstraint extracts the name of the constraint that a Postgres error
+// was raised against, if there is one. The schema carries six partial unique
+// indexes, and without the constraint name a 23505 raised by any of them looks
+// exactly like a 23505 raised by the table's primary key.
+func PgErrorConstraint(err error) string {
+	var pgErrV4 *pgconnv4.PgError
+	if errors.As(err, &pgErrV4) {
+		return pgErrV4.ConstraintName
+	}
+
+	var pgErrV5 *pgconnv5.PgError
+	if errors.As(err, &pgErrV5) {
+		return pgErrV5.ConstraintName
+	}
+
+	return ""
+}
+
+// withPgDetail renders a database error together with the Postgres constraint
+// name and detail when they are available, and falls back to the plain error
+// otherwise.
+func withPgDetail(err error) string {
+	constraint := PgErrorConstraint(err)
+	detail := PgErrorDetail(err)
+
+	switch {
+	case constraint != "" && detail != "":
+		return fmt.Sprintf("%v (constraint: %s, detail: %s)", err,
+			constraint, detail)
+
+	case constraint != "":
+		return fmt.Sprintf("%v (constraint: %s)", err, constraint)
+
+	case detail != "":
+		return fmt.Sprintf("%v (detail: %s)", err, detail)
+
+	default:
+		return err.Error()
+	}
+}
+
 // ErrSQLUniqueConstraintViolation is an error type which represents a database
 // agnostic SQL unique constraint violation.
 type ErrSQLUniqueConstraintViolation struct {
 	DBError error
 }
 
 func (e ErrSQLUniqueConstraintViolation) Error() string {
-	return fmt.Sprintf("sql unique constraint violation: %v", e.DBError)
+	return fmt.Sprintf("sql unique constraint violation: %v",
+		withPgDetail(e.DBError))
+}
+
+// Unwrap returns the wrapped error.
+//
+// Without this, the mapped error is a dead end for errors.As, and the
+// PgErrorConstraint and PgErrorDetail extractors return empty for every caller
+// that holds the mapped error rather than the raw driver one. That is the
+// normal case, since ExecTx returns the mapped error, and identifying which of
+// the partial unique indexes actually fired is the whole point of surfacing
+// the constraint name.
+func (e ErrSQLUniqueConstraintViolation) Unwrap() error {
+	return e.DBError
 }
 
 // ErrSerializationError is an error type which represents a database agnostic
@@ -135,7 +216,7 @@ func (e ErrSerializationError) Unwrap() error {
 
 // Error returns the error message.
 func (e ErrSerializationError) Error() string {
-	return e.DBError.Error()
+	return withPgDetail(e.DBError)
 }
 
 // ErrDeadlockError is an error type which represents a database agnostic error
@@ -151,7 +232,22 @@ func (e ErrDeadlockError) Unwrap() error {
 
 // Error returns the error message.
 func (e ErrDeadlockError) Error() string {
-	return e.DBError.Error()
+	return withPgDetail(e.DBError)
+}
+
+// IsUniqueConstraintViolation returns true if the given error is a unique
+// constraint violation.
+//
+// This is deliberately not part of IsSerializationOrDeadlockError. A unique
+// violation is not safe to retry blindly, because a retry of a plain insert
+// that lost a creation race just loses it again. Callers that can lose such a
+// race need to either rephrase the insert as a no-op upsert or translate the
+// violation into a domain level "already exists", which is why the classifier
+// is exposed separately.
+func IsUniqueConstraintViolation(err error) bool {
+	var uniqueErr *ErrSQLUniqueConstraintViolation
+
+	return errors.As(err, &uniqueErr)
 }
 
 // IsSerializationError returns true if the given error is a serialization