cockroachdb / cockroachdb/cockroach
colmem: memory accounting drift in `GetBatchMemSize` / `ResetMaybeReallocate` for batches with downstream-appended columns
- Dominant language
- Go
- Stars
- 32.5k
- Forks
- 4.1k
- PR merge metrics
- PR metrics pending
Description
## Summary
`colmem.GetBatchMemSize` returns the footprint of **all** columns in a batch, and the `resetMaybeReallocate` / `AccountingHelper.ResetMaybeReallocate` release paths use it to release from the batch owner's allocator. That's only correct if every column was allocated through that allocator. Downstream enforcers (`vectorTypeEnforcer`, `BatchSchemaSubsetEnforcer`) append columns in place via `MaybeAppendColumn`, sometimes on a *different* account, and never release them — they rely on the owner's reset. `GetBatchMemSize` can't tell own columns from these foreign ones, so the release is wrong whenever owner and appender don't share an account.
## Consequences
- **Crash.** In the distinct-account case the owner over-releases. `ReleaseMemory` clamps, so this only silently under-counts — until a co-tenant of the same account does a raw, unclamped `BoundAccount.Shrink` (e.g. `singleDatumAggregateBase.reset`/`close`), which underflows and panics with `"no bytes in account to release"`. This was the ordered-aggregator crash; it was mitigated by separating the accounts, but the over-release remains and can recur wherever a raw-`Shrink` component shares an account with a reset owner. See #173459.
- AI-assisted audit was performed, and it appears that on 26.4 we shouldn't actually hit this anywhere.
- **Latent under-accounting.** Without such a co-tenant the over-release just under-counts buffered memory (`sort`, `crossJoiner`, `mergeJoiner`, `bufferedWindowOp`), weakening `distsql_workmem` enforcement.
## Why it's hard to fix
Releasing only own columns (`len(typs)`) is wrong in the **shared-account** case — appended columns are then never reclaimed; for `alwaysReallocate` owners (`SetAccountingHelper`) the leak is unbounded.
Carrying foreign vecs across a reallocation (release own, grow own, re-attach foreign) fixes direct-append chains, but **`projectingBatch` breaks it**: `simpleProjectOp` rebuilds its projected view from the base projection whenever the underlying batch object changes, so after realloc to a fresh batch the enforcer re-appends a fresh column (grow) while the carried-over vec is orphaned — an accumulating, unbounded leak. Correct carry-over needs the downstream view to survive realloc, via either:
- **(A)** grow own columns *in place* to preserve batch-object identity (needs a new `coldata.Batch` capacity setter — `Reset`'s rebuild drops foreign vecs — plus an audit of callers keying on `reallocated == true`); or
- **(B)** teach `simpleProjectOp` to keep its extended projection when the new batch still satisfies it (localized, but pushes the invariant into the projection layer).
Jira issue: CRDB-66935
Contributor guide
Research direction
Start by tracing colmem.GetBatchMemSize, resetMaybeReallocate, and AccountingHelper.ResetMaybeReallocate alongside MaybeAppendColumn in the downstream enforcers. Then follow how simpleProjectOp rebuilds its projected view after a batch changes, including the failure described in #173459. Done means distinct-account and shared-account paths release owned and appended columns exactly once without leaks, under-accounting, or account underflow.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- databases, performance
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100