cockroachdb / cockroachdb/cockroach

kv: audit and test the efficiency of error handling

Open
#143,445 2 comments 0 reactions 0 assignees View on GitHub
A-kv branch-master C-bug O-postmortem P-3 T-kv
Dominant language
Go
Stars
32.5k
Forks
4.1k
PR merge metrics
PR metrics pending

Description

We have seen multiple examples of expensive error handling paths. Some recent examples and observations include:
- Unnecessarily wrapping errors (e.g. https://github.com/cockroachdb/cockroach/issues/143442).
https://github.com/cockroachdb/cockroach/blob/e048c1b1f3821a7ac57c6c8ca2077084150849fa/pkg/kv/kvserver/replica_send.go#L430-L432
- Printing the contents of the entire batch, which shows up prominently in CPU profiles ([slack](https://cockroachlabs.slack.com/archives/G01G8LK77DK/p1733243463798689)). E.g.
https://github.com/cockroachdb/cockroach/blob/c83c57d354741ac36740894f7387c014eb6c09fe/pkg/kv/kvclient/kvcoord/transport.go#L242-L244
- Calling `Error.GetDetail()` multiple times in the stack due to poor code organization. Each time it's called results in decoding the error protobuf. E.g. in `sendToReplica`, it's called at least 3 times: [here](https://github.com/cockroachdb/cockroach/blob/94b0c2e691b3a3c60b77b7556e6536bafe8aa091/pkg/kv/kvclient/kvcoord/dist_sender.go#L2781) and then again [here](https://github.com/cockroachdb/cockroach/blob/94b0c2e691b3a3c60b77b7556e6536bafe8aa091/pkg/kv/kvclient/kvcoord/dist_sender.go#L2794) and [here](https://github.com/cockroachdb/cockroach/blob/94b0c2e691b3a3c60b77b7556e6536bafe8aa091/pkg/kv/kvclient/kvcoord/dist_sender.go#L2901).

We should also check what some of these expensive error-handling paths do to memory allocation and resulting GC.

It seems like we're lacking scale and performance testing for these and other error paths. One idea is to set up a workloads that result in large percentage of errors and ensure those are cheap to handle. For example, to prevent the issue in https://github.com/cockroachdb/cockroach/issues/143442 we can set up a workload with a very small per-statement timeout which will force most requests to hit the context cancelation error.

Jira issue: CRDB-48840

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.