cockroachdb / cockroachdb/cockroach

sql: add panic catching to samplerProcessor

Open
#112,072 2 comments 0 reactions 0 assignees View on GitHub
C-bug O-support P-3 T-sql-queries
Dominant language
Go
Stars
32.5k
Forks
4.1k
PR merge metrics
PR metrics pending

Description

There's no reason for https://github.com/cockroachdb/cockroach/issues/84901 type bugs to be a panic. This crashes the node, instead it should just be an assertion/internal error. Another example:

```
github.com/cockroachdb/cockroach/pkg/sql/rowenc.(*EncDatum).IsNull(0xc0159b4390)
github.com/cockroachdb/cockroach/pkg/sql/rowenc/encoded_datum.go:220 +0x1c6
github.com/cockroachdb/cockroach/pkg/sql/rowexec.(*sketchInfo).addRow(0xc01539ec00, {0x63ea438, 0xc026815440}, {0xc0159b4240, 0xb, 0xc0157a1880}, {0xc015983320, 0x13, 0x24}, 0xc00af8f9f0, ...)
github.com/cockroachdb/cockroach/pkg/sql/rowexec/sampler.go:505 +0x91
github.com/cockroachdb/cockroach/pkg/sql/rowexec.(*samplerProcessor).mainLoop(0xc007b46c80, {0x63ea438, 0xc026815440})
github.com/cockroachdb/cockroach/pkg/sql/rowexec/sampler.go:320 +0x1d72
github.com/cockroachdb/cockroach/pkg/sql/rowexec.(*samplerProcessor).Run(0xc007b46c80, {0x63ea390, 0xc024586c80})
github.com/cockroachdb/cockroach/pkg/sql/rowexec/sampler.go:229 +0x9e
github.com/cockroachdb/cockroach/pkg/sql/flowinfra.(*FlowBase).StartInternal.func1(0xc000eae790)
github.com/cockroachdb/cockroach/pkg/sql/flowinfra/flow.go:421 +0x48
created by github.com/cockroachdb/cockroach/pkg/sql/flowinfra.(*FlowBase).StartInternal
github.com/cockroachdb/cockroach/pkg/sql/flowinfra/flow.go:420 +0x35a
```

We want to fail the stats collection, not crash the node.

Jira issue: CRDB-32223

Contributor guide

Open the contributing guide

Research direction

Start in pkg/sql/rowexec/sampler.go, focusing on samplerProcessor.Run, mainLoop, and sketchInfo.addRow; the stack trace also points to pkg/sql/rowenc/encoded_datum.go. Trace how the sampler handles failures and identify the existing error-reporting path. Done means stats collection fails with an assertion or internal error instead of crashing the node.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
databases
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.