Merged PRs
dolt
- 11097: go/go.mod: Bump fslock to pick up recent minor cleanup.
go-mysql-server
- 3561: fix(rowexec): dispose CachedResults child when closing hashLookupGeneratingIter (#3560)
Summary
hashLookupGeneratingIter.Closereturnednilwithout closing the child iter chain or disposing the wrapped*plan.CachedResultsnode. Every executed hash join over aCachedResultssubtree could therefore leak one entry intoAnalyzer.CachedResultsManager.cachedResultsCaches, freed only by process restart.
This PR makesClose:- Dispose the
CachedResultschild if present (releases the global Manager entry). - Propagate
Closedown the rest of the child iter chain.
Fix
func (h *hashLookupGeneratingIter) Close(c *sql.Context) error { - return nil + if cr, ok := h.n.Child.(*plan.CachedResults); ok { + cr.Dispose(c) + } + return h.childIter.Close(c) }Why
Full RCA, reproducer, and pprof evidence are in #3560. Headline numbers from the 3-minute, ~30 req/sec workload reported there (Dolt 2.0.6 sql-server vendoring this commit):Metric Vanilla Δ Patched Δ Improvement RSS +5069 MB +44 MB ~115× HeapAlloc (live) +4.3 GB +82 MB ~52× HeapObjects (live) +90 million +1.5 million ~60× Mallocs - Frees(net live)+204 million +4 million ~50× Allocation rate is unchanged (~1.1B mallocs in 3 min both runs); the patched build services the same workload but Freeskeeps up withMallocs.Tests
go test ./sql/rowexec/...passes locally with the patch applied.
I did not include a unit-level regression test in this PR. In a minimalengine.Query→RowIterToRowstest,TrackedRowIter.Close → done → disposeNodealready walks the plan tree and disposes theCachedResultsviasql.Dispose(ctx, node), so the leak doesn't reproduce that way; faithfully exercising the production path (Dolt sql-server, prepared statements via the MySQL wire protocol) needs significantly more harness scaffolding. Happy to add a test if maintainers can point me at the right entry point.
Closes #3560 - Dispose the
- 3558: Unwrap wrapped strings before casting to string during
RegexpReplace.Eval
Fixes #11095
REGEXP_REPLACEqueries onLONGTEXTtype columns were panicking because we were casting values to strings without unwrapping them. Note that this doesn't happen with tables created in Dolt 2.0 due to the recent changes to adaptive encoding, but we still need to support tables before then.
Also movedmockStringWrapperto a sharedtestutil package so it can be used in various tests.
Closed Issues
- 3560: CachedResults / HashLookup chain leaks rows-cache for every hash join until process restart