cockroachdb / cockroachdb/cockroach

sql/stats: ensureAllTables blocks refresher startup at scale, affecting node drain and initial refresh burst

Open
#171,192 0 comments 0 reactions 0 assignees View on GitHub
A-many-descriptors C-bug C-investigation E-quick-win O-agent P-3 T-sql-queries
Dominant language
Go
Stars
32.5k
Forks
4.1k
PR merge metrics
PR metrics pending

Description

**Summary**

On node startup, `Refresher.ensureAllTables` runs a full descriptor scan
(`txn.Descriptors().GetAll`) in a single transaction to seed `mutationCounts`.
The refresher's main select loop is parked in `case <-initialTableCollection`
for the entire scan duration, so it cannot react to mutations, timer ticks,
setting overrides, or shutdown signals until the scan completes.

At 100K tables this completes in under 7 seconds. Linear extrapolation puts
1M tables at ~70 seconds; scaling may be worse than linear due to memory
pressure on the descriptor materialization (not measured here).

This blocks the refresher *only* — SQL queries, KV serving, raft, and the
descriptor lookup path used by user transactions are unaffected (the
`cachedCatalogReader` is per-`Collection`, per-transaction; see
[pkg/sql/catalog/descs/factory.go#L175](https://github.com/cockroachdb/cockroach/blob/master/pkg/sql/catalog/descs/factory.go#L175)).

**Measurements (100K-table shared cluster, 2026-05-29)**

- Goroutine dump caught `Refresher.getApplicableTables` in flight at T+13s
after `roachprod start`, gone by T+20s. Bound on scan duration: (1s, 7s).
- Goroutine state was `[runnable]`, actively in
`catpb.ShardedDescriptor.Unmarshal` — bottleneck is protobuf decode CPU,
not the KV read.
- Heap snapshot post-scan shows the live descriptor cache footprint at
~0.4 GB (top allocators: `tabledesc.makeImmutable`,
`tabledesc.BuildImmutableTable`, `tabledesc.newColumnCache`).
- 111,387 descriptors total at the time of measurement.

**Impact**

Three concrete consequences while the refresher loop is parked in
`ensureAllTables`:

1. **Drain latency.** `<-r.drainAutoStats` and `<-ctx.Done()` are select
cases in the same loop. Graceful drain requested during the scan window
is not acked by the auto-stats subsystem until the scan finishes. For
a 9-node rolling upgrade with sequential drain, a 70s-per-node scan
adds ~10 minutes to the upgrade window if refresher drain is on the
critical path.

2. **Mutation notification loss under heavy writes.** `r.mutations` is a
buffered channel of capacity 32768
([pkg/sql/stats/automatic_stats.go#L213](https://github.com/cockroachdb/cockroach/blob/master/pkg/sql/stats/automatic_stats.go#L213)).
Writes call `NotifyMutation`, which is a non-blocking channel send;
if the buffer fills, the mutation is dropped (rate-limited log via
`bufferedChanFullLogLimiter`). At sustained >468 mutations/sec to
this node during a 70s scan, the buffer overflows. Impact is delayed
stats refresh decisions, not data correctness.

3. **First refresh tick is the maximum burst.** When the scan completes,
`ensureAllTables` has just seeded `mutationCounts` with every
applicable table. The next `timer.C` tick processes that map
sequentially in one goroutine. At 1M tables this is the worst-case
fanout pass — separately tracked as failure mode #2 in CRDB-63468.

**Code references**

- [pkg/sql/stats/automatic_stats.go#L627-L770](https://github.com/cockroachdb/cockroach/blob/master/pkg/sql/stats/automatic_stats.go#L627-L770) — the parked select loop
- [pkg/sql/stats/automatic_stats.go#L871-L917](https://github.com/cockroachdb/cockroach/blob/master/pkg/sql/stats/automatic_stats.go#L871-L917) — `getApplicableTables`, single-txn full-catalog scan
- [pkg/sql/catalog/internal/catkv/catalog_reader_cached.go#L198](https://github.com/cockroachdb/cockroach/blob/master/pkg/sql/catalog/internal/catkv/catalog_reader_cached.go#L198) — `ScanAll`

**Possible mitigations** (not endorsing any specific one — investigation issue)

- Run `ensureAllTables` outside the main select loop, off a separate goroutine,
so the loop can drain shutdown/mutation/setting signals during the scan.
- Stream the scan: process descriptors as they arrive from `GetAll` rather
than materializing the full set in one allocation.
- Batch the scan (LIMIT/OFFSET on `system.descriptor`) and intersperse with
select-loop iterations.
- Avoid the seeding pass entirely for tables that haven't been mutated since
last node restart — rely on `NotifyMutation` to populate `mutationCounts`
organically, accepting longer staleness for read-only tables.

**Related**

- Parent observability gap: #171184 (blocks confirming exact scan duration)
- Partner concern (steady-state fanout): tracked in CRDB-63468 Phase 4

Epic CRDB-58778
Jira issue: CRDB-64362

Contributor guide

Open the contributing guide

Research direction

Start in pkg/sql/stats/automatic_stats.go, reading the main select loop around lines 627-770 and getApplicableTables around lines 871-917. Then inspect ScanAll in pkg/sql/catalog/internal/catkv/catalog_reader_cached.go and the referenced mutation channel behavior. Done requires selecting and validating an approach that prevents the full scan from blocking shutdown, mutation handling, and refresh signals at scale.

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
Quiet
Clarity
Needs clarification
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.