Skip to content

Commit

Permalink
opt: don't hold on to evalCtx from detached Memo
Browse files Browse the repository at this point in the history
This change adds more "cleanup" code when detaching a Memo (a detached
memo is stored in the query cache and is reused later in a "read-only"
fashion). In particular, we clear the EvalContext in
logicalPropsBuilder which can lead to inadvertently holding on to a
lot of memory.

Fixes cockroachdb#57059.

Release note: None
  • Loading branch information
RaduBerinde committed Nov 30, 2020
1 parent f2fe7a4 commit b5b1eb4
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 14 deletions.
34 changes: 23 additions & 11 deletions pkg/sql/opt/memo/memo.go
Original file line number Diff line number Diff line change
Expand Up @@ -384,17 +384,29 @@ func (m *Memo) NextWithID() opt.WithID {
return m.curWithID
}

// ClearColStats clears all column statistics from every relational expression
// in the memo. This is used to free up the potentially large amount of memory
// used by histograms.
func (m *Memo) ClearColStats(parent opt.Expr) {
for i, n := 0, parent.ChildCount(); i < n; i++ {
child := parent.Child(i)
m.ClearColStats(child)
}
// Detach is used when we detach a memo that is to be reused later (either for
// execbuilding or with AssignPlaceholders). New expressions should no longer be
// constructed in this memo.
func (m *Memo) Detach() {
m.interner = interner{}
// It is important to not hold on to the EvalCtx in the logicalPropsBuilder
// (#57059).
m.logPropsBuilder = logicalPropsBuilder{}

// Clear all column statistics from every relational expression in the memo.
// This is used to free up the potentially large amount of memory used by
// histograms.
var clearColStats func(parent opt.Expr)
clearColStats = func(parent opt.Expr) {
for i, n := 0, parent.ChildCount(); i < n; i++ {
child := parent.Child(i)
clearColStats(child)
}

switch t := parent.(type) {
case RelExpr:
t.Relational().Stats.ColStats = props.ColStatsMap{}
switch t := parent.(type) {
case RelExpr:
t.Relational().Stats.ColStats = props.ColStatsMap{}
}
}
clearColStats(m.RootExpr())
}
6 changes: 3 additions & 3 deletions pkg/sql/opt/norm/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,11 +114,11 @@ func (f *Factory) Init(evalCtx *tree.EvalContext, catalog cat.Catalog) {
// placeholders are assigned. If there are no placeholders, there is no need
// for column statistics, since the memo is already fully optimized.
func (f *Factory) DetachMemo() *memo.Memo {
f.mem.ClearColStats(f.mem.RootExpr())
detach := f.mem
m := f.mem
f.mem = nil
m.Detach()
f.Init(f.evalCtx, nil /* catalog */)
return detach
return m
}

// DisableOptimizations disables all transformation rules. The unaltered input
Expand Down

0 comments on commit b5b1eb4

Please sign in to comment.