Skip to content

dbconn: scope the table lock holder lookup to the session's own database - #284

Draft
Kiran01bm wants to merge 1 commit into
mainfrom
kiran01bm/cs-pd15-lock-holder-own-database
Draft

Kiran01bm wants to merge 1 commit into
mainfrom
kiran01bm/cs-pd15-lock-holder-own-database

Conversation

@Kiran01bm

Copy link
Copy Markdown
Collaborator

TableLockSession.Confirm and LookupTableLockHolder now read pg_locks scoped to the connection's own database, so a table lock held in another database of the same server no longer confirms as this database's lock.

Why

An advisory lock excludes only sessions of the database it was taken in, but pg_locks lists every database's grants. The holder lookup behind Confirm had no database predicate, so a working transaction in database A found the lock a session held in database B — same server, same key — and Confirm returned nil. The run then executed with a lock that kept nothing off the table in A; a second run correctly wired to A would take its own lock there without contention. Across clusters the lookup finds nothing and the run already refused, so this is specifically the same-server, other-database wiring.

pkg/migrate already stops that wiring at the front door (CopySwapConnectionMismatchError), but anything that reaches the lock through another door — schemachange.NewRunner(...).Run, or AcquireTableLock plus Confirm directly, all exported — was still confirmed against a lock in any database. The predicate closes the gap at its source and makes shadow-lock-unconfirmed mean what it says.

What

  • lookupTableLockHolder adds l.database = (SELECT oid FROM pg_database WHERE datname = current_database()). It serves Confirm, LookupTableLockHolder, and the contention holder report in AcquireTableLock, so all three now describe only this database's grant. sessionHoldsAdvisoryLock was already correct: it filters on pid = pg_backend_pid(), and one backend holds locks in one database.
  • TestTableLockConfirmIgnoresTheSameKeyHeldInAnotherDatabase: lock taken from database B, Confirm from a transaction on database A is ErrTableLockNotHeld and not a TableLockHeldError; LookupTableLockHolder on A finds nobody while on B it reports the session's PID. Fails on main at both assertions (Confirm returned nil; A "found" B's holder).
  • docs/invariants.md LK-1 Enforced: one clause for the database scope.

Before / after

Same wiring on both sides: the lock session is dialled into database b, the pool writes to database a, both on one server. The table is app.orders; the key is (0x70677370, hashtext('app.orders')).

Before, the working transaction on a asks pg_locks for that key with no database filter, sees the grant in b, and Confirm says the lock is held, so the write proceeds unprotected. After, the same query is scoped to a's OID, finds no grant, and Confirm returns ErrTableLockNotHeld; the caller fails closed before its first write. Nothing changes for the correctly wired case, where lock and work share one database.

Before
┌──────────────────────┐        ┌───────────────────────────────┐
│ lock session on  b   │        │ pg_locks (whole server)       │
│ pg_try_advisory_lock │──────▶ │ database=b  classid=pgsp      │
│ (pgsp, h(app.orders))│        │             objid=h(app.orders)│
└──────────────────────┘        └──────────────┬────────────────┘
                                               │ no database predicate:
┌──────────────────────┐                       │ the grant in b matches
│ working tx on  a     │  Confirm → nil ◀──────┘
│ about to write       │  first write runs; no lock excludes
│ app.orders in  a     │  a second run on a.app.orders
└──────────────────────┘

After
┌──────────────────────┐        ┌───────────────────────────────┐
│ lock session on  b   │        │ pg_locks (whole server)       │
│ pg_try_advisory_lock │──────▶ │ database=b  classid=pgsp      │
│ (pgsp, h(app.orders))│        │             objid=h(app.orders)│
└──────────────────────┘        └──────────────┬────────────────┘
                                               │ AND database = oid(a):
┌──────────────────────┐                       │ the grant in b is filtered out
│ working tx on  a     │  Confirm → ErrTableLockNotHeld
│ about to write       │  caller fails closed; nothing written
│ app.orders in  a     │
└──────────────────────┘

The front-door check in pkg/migrate is unchanged and still stops this wiring before the preflight; this PR is the safety net underneath it for every other entry point.

🤖 Generated with Amp (Claude Opus 4.6)

pg_locks is cluster-wide while an advisory lock is per database, so a
lock session dialled into another database of the same server confirmed
the lock it held there and the run went ahead with a lock that excluded
nothing on the working database's table.
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