cockroachdb / cockroachdb/cockroach
sql: add panic catching to samplerProcessor
- 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
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