Skip to content

exec: catch panics coming from colBatchScan operator #37294

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 3, 2019
Merged

exec: catch panics coming from colBatchScan operator #37294

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 21 additions & 8 deletions pkg/sql/exec/error.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,11 @@ import (
"github.com/pkg/errors"
)

const (
panicLineSubstring = "runtime/panic.go"
execPackagePrefix = "github.com/cockroachdb/cockroach/pkg/sql/exec"
)
const panicLineSubstring = "runtime/panic.go"

// CatchVectorizedRuntimeError executes operation, catches a runtime error if
// it is coming from exec package, and returns it. If an error occurs that is
// not from exec package, it is not recovered from.
// it is coming from the vectorized engine, and returns it. If an error not
// related to the vectorized engine occurs, it is not recovered from.
func CatchVectorizedRuntimeError(operation func()) (retErr error) {
defer func() {
if err := recover(); err != nil {
Expand All @@ -50,8 +47,10 @@ func CatchVectorizedRuntimeError(operation func()) (retErr error) {
panic(fmt.Sprintf("panic line %q not found in the stack trace\n%s", panicLineSubstring, stackTrace))
}
if scanner.Scan() {
if strings.HasPrefix(strings.TrimSpace(scanner.Text()), execPackagePrefix) {
// We only want to catch runtime errors coming from the exec package.
panicEmittedFrom := strings.TrimSpace(scanner.Text())
if isPanicFromVectorizedEngine(panicEmittedFrom) {
// We only want to catch runtime errors coming from the vectorized
// engine.
switch t := err.(type) {
case *pgerror.Error:
retErr = t
Expand All @@ -74,6 +73,20 @@ func CatchVectorizedRuntimeError(operation func()) (retErr error) {
return retErr
}

const (
execPackagePrefix = "github.com/cockroachdb/cockroach/pkg/sql/exec"
colBatchScanPrefix = "github.com/cockroachdb/cockroach/pkg/sql/distsqlrun.(*colBatchScan)"
)

// isPanicFromVectorizedEngine checks whether the panic that was emitted from
// panicEmittedFrom line of code (which includes package name as well as the
// file name and the line number) came from the vectorized engine.
// panicEmittedFrom must be trimmed to not have any white spaces in the prefix.
func isPanicFromVectorizedEngine(panicEmittedFrom string) bool {
return strings.HasPrefix(panicEmittedFrom, execPackagePrefix) ||
strings.HasPrefix(panicEmittedFrom, colBatchScanPrefix)
}

// TestVectorizedErrorEmitter is an Operator that panics on every odd-numbered
// invocation of Next() and returns the next batch from the input on every
// even-numbered (i.e. it becomes a noop for those iterations). Used for tests
Expand Down