cockroachdb / cockroachdb/cockroach

kvserver/concurrency: resolvableTxns cache bypasses the non-locking gate for pushed PENDING txns

Open
#172,427 0 comments 0 reactions 0 assignees View on GitHub
A-kv-transactions branch-master C-bug O-agent P-3 T-kv
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.