cockroachdb / cockroachdb/cockroach
sql/opt: cross-session data race on cached PrefixSorter eval.Context
- Dominant language
- Go
- Stars
- 32.5k
- Forks
- 4.1k
- PR merge metrics
- PR metrics pending
Description
**Describe the problem**
The `PrefixSorter` cached on the optimizer's `TableMeta` holds an `EvalCtx *eval.Context`
([`pkg/sql/opt/partition/locality.go`](https://github.com/cockroachdb/cockroach/blob/master/pkg/sql/opt/partition/locality.go)),
and `TableMeta.copyFrom`
([`pkg/sql/opt/table_meta.go:327`](https://github.com/cockroachdb/cockroach/blob/master/pkg/sql/opt/table_meta.go#L327))
copies `indexPartitionLocalities` (the slice of `PrefixSorter`s) **by reference**. When a memo is
reused across sessions via the node-level query cache, the shared `PrefixSorter.EvalCtx` is a stale
pointer into whichever session originally built the memo. A later session that reuses the plan
dereferences that eval context during span consolidation (`searchPrefixes` → `DEnum.Compare` →
`UnwrapDatum` → `eval.(*Context).HasPlaceholders`) while the original session concurrently resets its
own eval context (`resetEvalCtx` sets `evalCtx.Placeholders = nil`, etc.). That is an unsynchronized
read/write of the same `*eval.Context` from two sessions — a data race.
**Reproduction**
Under `-race`, the concurrent partition-prefix workload from #174886 (many `READ COMMITTED` sessions
running a `REGIONAL BY ROW` scan that reaches `searchPrefixes`, with the region enum version churning)
reports a `DATA RACE`:
```
WARNING: DATA RACE
Read at 0x... by goroutine A:
eval.(*Context).HasPlaceholders
eval.UnwrapDatum
tree.(*DEnum).Compare
constraint.searchPrefixes ...
Previous write at 0x... by goroutine B:
sql.(*connExecutor).resetEvalCtx (evalCtx.Placeholders = nil, during prepare)
sql.(*connExecutor).resetPlanner
sql.(*connExecutor).prepare ...
```
Run the Variant-2 test from #174886 under `--race`.
**Expected behavior**
Reusing a cached plan across sessions must not dereference another session's mutable `*eval.Context`.
**Fix direction**
Give the `PrefixSorter` used during planning the executing session's eval context rather than the
cached one (e.g. set `ps.EvalCtx`/`ps.Ctx` from the current `optPlanningCtx` when the cached sorter
is fetched), or avoid sharing the `PrefixSorter` (and its embedded context) by reference across
memos. A one-line experiment pointing the fetched `PrefixSorter` at the live eval context silenced
this race under `-race`.
**Additional context**
Discovered alongside #174886 (enum comparison error from in-place type hydration); this is a distinct
defect — a different root (reference-shared eval context, not the version write) with a different fix
— hence a separate issue. It does not itself produce the enum-version assertion.
**Environment**
- CockroachDB version: reproduced on master; long-standing (the `PrefixSorter`/`copyFrom`
reference-sharing is not recent)
- Surfaces under `-race` / stress; requires cross-session query-cache reuse of a `REGIONAL BY ROW`
plan that reaches `searchPrefixes`
Jira issue: CRDB-68073
Contributor guide
Research direction
Start with pkg/sql/opt/partition/locality.go and pkg/sql/opt/table_meta.go:327, then trace PrefixSorter use through searchPrefixes and the cached-memo path. Run the Variant-2 concurrent partition-prefix test from #174886 under --race; done means cached plans no longer dereference another session's mutable eval.Context and the race is absent.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go, sql
- Domain
- databases
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100