Skip to content

Commit

Permalink
opt: fix memo cycle test in dbg builds
Browse files Browse the repository at this point in the history
In #106944 the `dbg` compilation mode became the default for all
non-release builds. This caused stack overflows in the memo cycle
detection tests, which excessive invoke recursive functions, likely
because stack frames are larger in `dbg` mode than in `opt` mode or
because certain optimizations aren't performed in `dbg` mode that reduce
stack depth due to recursion, like inlining.

This commit fixes the test by lowering the cycle detection limit, making
it so that a cycle is detected before a stack overflow occurs.

Epic: None

Release note: None
  • Loading branch information
mgartner committed Jul 19, 2023
1 parent 30acaf9 commit cc8ea4f
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 20 deletions.
40 changes: 23 additions & 17 deletions pkg/sql/opt/xform/optimizer.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/opt/ordering"
"github.com/cockroachdb/cockroach/pkg/sql/opt/props/physical"
"github.com/cockroachdb/cockroach/pkg/sql/sem/eval"
"github.com/cockroachdb/cockroach/pkg/util/buildutil"
"github.com/cockroachdb/cockroach/pkg/util/cancelchecker"
"github.com/cockroachdb/cockroach/pkg/util/errorutil"
"github.com/cockroachdb/cockroach/pkg/util/intsets"
Expand Down Expand Up @@ -111,14 +112,20 @@ type Optimizer struct {
scratchSort *memo.SortExpr
}

// maxGroupPasses is the maximum allowed number of optimization passes for any
// single memo group. The groupState.passes field is incremented every time
// optimizeGroup is called on the group. If a groupState's passes exceeds this
// limit, there is likely a cycle in the memo where a path exists from a group
// member's children back to the group member's group. To avoid stack overflows
// that these memo cycles cause, the optimizer throws an internal error when
// this limit is reached.
const maxGroupPasses = 100_000
const (
// maxGroupPasses is the maximum allowed number of optimization passes for
// any single memo group. The groupState.passes field is incremented every
// time optimizeGroup is called on the group. If a groupState's passes
// exceeds this limit, there is likely a cycle in the memo where a path
// exists from a group member's children back to the group member's group.
// To avoid stack overflows that these memo cycles cause, the optimizer
// throws an internal error when this limit is reached.
maxGroupPasses = 100_000

// maxGroupPassesTest is similar to maxGroupPasses but only applies in test
// builds.
maxGroupPassesTest = 10_000
)

// Init initializes the Optimizer with a new, blank memo structure inside. This
// must be called before the optimizer can be used (or reused).
Expand Down Expand Up @@ -493,20 +500,19 @@ func (o *Optimizer) optimizeGroup(grp memo.RelExpr, required *physical.Required)
}

state.passes++
if state.passes > maxGroupPasses {
var maxPasses int
if buildutil.CrdbTestBuild {
maxPasses = maxGroupPassesTest
} else {
maxPasses = maxGroupPasses
}
if state.passes > maxPasses {
// If optimizeGroup has been called on a group more than maxGroupPasses
// times, there is likely a cycle in the memo. To avoid a stack
// overflow, throw an internal error. The formatted memo is included as
// an error detail to aid in debugging the cycle.
mf := makeMemoFormatter(o, FmtCycle, false /* redactableValues */)
panic(errors.WithDetail(
errors.AssertionFailedf(
"memo group optimization passes surpassed limit of %v; "+
"there may be a cycle in the memo",
maxGroupPasses,
),
mf.format(),
))
panic(errors.WithDetail(errors.AssertionFailedf("memo cycle detected"), mf.format()))
}

// Iterate until the group has been fully optimized.
Expand Down
6 changes: 3 additions & 3 deletions pkg/sql/opt/xform/testdata/rules/cycle
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ expropt disable=MemoCycleTestRelRuleFilter skip-race
[ (Eq (Var "b") (Const 1 "int")) ]
)
----
error: memo group optimization passes surpassed limit of 100000; there may be a cycle in the memo
error: memo cycle detected
details:
memo (not optimized, ~3KB, required=[], cycle=[G1->G1])
├── G1: (memo-cycle-test-rel G2 G3) (memo-cycle-test-rel G1 G3)
Expand Down Expand Up @@ -60,7 +60,7 @@ expropt disable=MemoCycleTestRelRuleFilter skip-race
[ ]
)
----
error: memo group optimization passes surpassed limit of 100000; there may be a cycle in the memo
error: memo cycle detected
details:
memo (not optimized, ~16KB, required=[], cycle=[G1->G3->G5->G5])
├── G1: (left-join G2 G3 G4)
Expand Down Expand Up @@ -88,7 +88,7 @@ expropt disable=MemoCycleTestRelRule skip-race
[ (Eq (Var "v") (Const 1 "int")) ]
)
----
error: memo group optimization passes surpassed limit of 100000; there may be a cycle in the memo
error: memo cycle detected
details:
memo (not optimized, ~6KB, required=[], cycle=[G1->G4->G6->G9->G10->G12->G13->G1])
├── G1: (memo-cycle-test-rel G2 G3) (select G2 G4)
Expand Down

0 comments on commit cc8ea4f

Please sign in to comment.