cockroachdb / cockroachdb/cockroach
kvserver/concurrency: resolvableTxns cache bypasses the non-locking gate for pushed PENDING txns
- Dominant language
- Go
- Stars
- 32.5k
- Forks
- 4.1k
- PR merge metrics
- PR metrics pending
Description
**Describe the problem**
This is a latent hazard, not a live bug: the guarded property currently holds, but only by enforcement that lives two files away from the code that depends on it.
`canResolveKeyForHoldingTransaction` consults the guard's `resolvableTxns` cache ([lock_table.go#L1072](https://github.com/cockroachdb/cockroach/blob/a7e117882b4356096b3a8ea26046baa7e5d53fce/pkg/kv/kvserver/concurrency/lock_table.go#L1072)) *before* the `str == lock.None && batchPushedLockResolution()` gate at [lock_table.go#L1085](https://github.com/cockroachdb/cockroach/blob/a7e117882b4356096b3a8ea26046baa7e5d53fce/pkg/kv/kvserver/concurrency/lock_table.go#L1085). A cached pushed-PENDING entry would therefore let a *locking* scan treat a pending intent as non-conflicting — an isolation violation.
This is unreachable today only because `canVirtuallyResolve` ([concurrency_manager.go#L872](https://github.com/cockroachdb/cockroach/blob/a7e117882b4356096b3a8ea26046baa7e5d53fce/pkg/kv/kvserver/concurrency/concurrency_manager.go#L872)) restricts virtual intent resolution to read-only, non-locking batches, so VIR guards never scan at locking strengths. If VIR is ever extended to locking reads (e.g. SELECT FOR UPDATE), this silently breaks with no assertion to catch it.
**Suggested fix**
Add an assertion in `isResolvableTxn` ([lock_table.go#L642](https://github.com/cockroachdb/cockroach/blob/a7e117882b4356096b3a8ea26046baa7e5d53fce/pkg/kv/kvserver/concurrency/lock_table.go#L642)) or in the fast path itself, e.g. `assert(g.curStrength() == lock.None || up.Status.IsFinalized())`, so the cross-file assumption fails loudly instead of producing an isolation violation.
**Additional context**
Found during an agent-assisted correctness audit of `pkg/kv/kvserver/concurrency`. Code links are pinned to master @ a7e1178.
Jira issue: CRDB-65663
Contributor guide
Research direction
Start in pkg/kv/kvserver/concurrency/lock_table.go by reading isResolvableTxn and canResolveKeyForHoldingTransaction, then compare their assumptions with canVirtuallyResolve in concurrency_manager.go. Trace the resolvableTxns fast path and the non-locking gate; done means the cross-file invariant is enforced by an assertion that catches an unsafe locking-strength case.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- databases, distributed-systems
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100