cockroachdb / cockroachdb/cockroach

sql/opt: cross-session data race on cached PrefixSorter eval.Context

Open
#174,887 1 comment 0 reactions 0 assignees View on GitHub
A-sql-optimizer C-bug O-agent T-sql-queries
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

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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.