cockroachdb / cockroachdb/cockroach
sql: transaction advisory locks are not released by ROLLBACK TO SAVEPOINT
- Dominant language
- Go
- Stars
- 32.5k
- Forks
- 4.1k
- PR merge metrics
- PR metrics pending
Description
**Describe the problem**
A transaction-level advisory lock acquired after a `SAVEPOINT` is not released
when that savepoint is rolled back. It continues to block other sessions until
the transaction ends.
Worse, `pg_locks` reports the opposite: the advisory row disappears on
`ROLLBACK TO SAVEPOINT`, so the lock is invisible while it is still enforcing.
A session blocked on it has no way to find the holder through
`pg_locks` or `crdb_internal.cluster_held_advisory_locks`.
PostgreSQL releases the lock in this situation, and its `pg_locks` agrees with
the actual lock state.
**To Reproduce**
Start a single-node cluster (`cockroach demo --insecure`) and open two SQL
shells against it.
Session A — acquire under a savepoint, then roll the savepoint back:
```sql
BEGIN;
SAVEPOINT s;
SELECT pg_advisory_xact_lock(5550001);
ROLLBACK TO SAVEPOINT s;
SELECT count(*) FROM pg_locks WHERE locktype = 'advisory' AND objid = 5550001;
-- count
-- -------
-- 0 <- CockroachDB reports the lock is gone
```
Session B, while A is still open — try to take the same key:
```sql
SET lock_timeout = '3s'; -- only to bound the wait; without it, B hangs
BEGIN;
SELECT pg_advisory_xact_lock(5550001);
-- ERROR: canceling statement due to lock timeout
```
Now end A's transaction (`ROLLBACK;` or `COMMIT;`) and retry in B — it
succeeds immediately, confirming the lock was held by A the whole time.
Control: a key that was never locked is acquired by B instantly, so the probe
is not simply always timing out.
**Expected behavior**
`ROLLBACK TO SAVEPOINT` should release advisory locks acquired after the
savepoint, matching PostgreSQL.
Running the identical script against PostgreSQL 17:
| | `pg_locks` after rollback | session B acquiring the key |
|---|---|---|
| PostgreSQL 17 | 0 rows | succeeds in 0.2 ms |
| CockroachDB | 0 rows | times out after 3 s |
| PG control (held, no rollback) | — | times out after 3 s |
At minimum the two should be self-consistent. Today `pg_locks` claims a release
that did not happen, which is the most confusing possible combination: an
invisible lock that still blocks.
**Additional data / screenshots**
Root cause: advisory locks are acquired as **replicated** KV locks, by design.
[`pkg/sql/advisorylock/manager.go:221-234`](https://github.com/cockroachdb/cockroach/blob/8812064a015d2faf99d3fc7e15880f94042954b0/pkg/sql/advisorylock/manager.go#L221-L234):
```go
// Lock the key in the appropriate mode, we are going to be locking
// a non-existing key. Additionally, for simplicity this lock will
// be replicated, since otherwise we need some ability to detect if
// the lock was lost.
b.AddRawRequest(&kvpb.GetRequest{
RequestHeader: kvpb.RequestHeader{Key: encodedKey},
KeyLockingStrength: lockMode,
LockNonExisting: true, // Key will not exist.
KeyLockingDurability: lock.Replicated,
})
```
Savepoint rollback is sequence-number based. It drops *unreplicated* locks and
marks write intents as ignored, but replicated locks are durable lock records
and are only cleaned up at transaction end by lock resolution —
[`lock_table.go:4148`](https://github.com/cockroachdb/cockroach/blob/8812064a015d2faf99d3fc7e15880f94042954b0/pkg/kv/kvserver/concurrency/lock_table.go#L4148)
explicitly declines to track sequence numbers for them ("savepoint rollbacks
are rare enough").
Meanwhile the SQL layer truncates its own in-memory tracking on rollback —
[`conn_executor_savepoints.go:230`](https://github.com/cockroachdb/cockroach/blob/8812064a015d2faf99d3fc7e15880f94042954b0/pkg/sql/conn_executor_savepoints.go#L230)
calls
[`Manager.OnSQLRollbackToSavepoint`](https://github.com/cockroachdb/cockroach/blob/8812064a015d2faf99d3fc7e15880f94042954b0/pkg/sql/advisorylock/manager.go#L327),
which pops the acquisition stack. That stack is what feeds `pg_locks` (its only
consumer is
[`GetHeldLocks`](https://github.com/cockroachdb/cockroach/blob/8812064a015d2faf99d3fc7e15880f94042954b0/pkg/sql/advisorylock/manager.go#L293)),
so the reporting rolls back while KV does not.
**Environment:**
- CockroachDB version: master, `v26.4.0-alpha` (verified at public master `8812064`)
- Server OS: Linux
- Client app: `psql` / `pgx`
- Compared against: PostgreSQL 17 (official `postgres:17` Docker image)
**Additional context**
Impact: an application using advisory locks for mutual exclusion inside a
transaction that also uses savepoints — including any ORM or framework that
wraps nested blocks in savepoints, and PL/pgSQL exception handlers, which open
a savepoint per block — will hold locks it believes it released. Those locks
serialize unrelated work for the rest of the transaction and are undiagnosable
from `pg_locks`.
Two possible directions:
1. Make advisory locks savepoint-aware (matching PostgreSQL). This likely means
giving up replicated locks, or teaching savepoint rollback to release the
specific replicated locks acquired since the savepoint.
2. If holding to transaction end is accepted as intended, then `pg_locks`
should keep reporting the lock, and the divergence from PostgreSQL should be
documented.
Option 2 is much cheaper but leaves a real PostgreSQL incompatibility.
Jira issue: CRDB-66795
Epic CRDB-65516
Contributor guide
Research direction
Start with pkg/sql/advisorylock/manager.go, conn_executor_savepoints.go, and kv/kvserver/concurrency/lock_table.go, then reproduce the two-session SQL sequence in the issue. Trace how savepoint rollback updates advisory-lock tracking versus replicated KV locks. Done means the rollback behavior and pg_locks reporting are self-consistent and match the chosen compatibility direction, with coverage for the reproduced case.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go, sql
- Domain
- databases
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 42/100