cockroachdb / cockroachdb/cockroach

kvcoord: retry requests that hit proxy auth errors

Open
#158,677 1 comment 0 reactions 0 assignees View on GitHub
A-kv-client branch-master C-bug P-3 T-kv
Dominant language
Go
Stars
32.5k
Forks
4.1k
PR merge metrics
PR metrics pending

Description

The `DistSender` has special treatment of auth gRPC error in certain cases. In the general case, these errors are retried idempotently because the request is guaranteed to not have started evaluating:

https://github.com/cockroachdb/cockroach/blob/7fe41c1ecad760fc25d907d2e734e60976b936d7/pkg/kv/kvclient/kvcoord/dist_sender.go#L2923-L2925

In the above snippet, `RequestDidNotStart` captures all auth errors, and some other ones as well.

However, if the transport is exhausted and the last error is an auth error, the `DistSender` returns the error to the client because this is a good indication that the auth issue is on the client side (so there is no point in more retries).

https://github.com/cockroachdb/cockroach/blob/7fe41c1ecad760fc25d907d2e734e60976b936d7/pkg/kv/kvclient/kvcoord/dist_sender.go#L3319-L3327

The implementation above does not actually check that _all_ unsuccessful attempts that exhausted the transport resulted in auth errors; just that the last one did. But the spirit of the behavior here seems to work well.

For proxied requests, however, the above behavior is not ideal. Consider a node (`n1`) that's in the process of shutting down, and suppose another node (`n2`) proxies a request via `n1` such that if this proxying fails, `n2`'s transport will be exhausted. When `n1` tries to send the request, it gets a `PermissionDenied` error, which in this case is due to a client issue at `n1`: it is shutting down. However, when this response makes it back to `n2`, it is no longer an indication of a client issue at `n2`: it's perfectly healthy. The expectation here is that `n2` should generate a send error and keep retrying, but due to the logic above, the `PermissionDenied` error is returned to the client.

One way to improve the behavior for proxy requests is to keep the auth errors they return so they can hit the idempotent retry path (via `RequestDidNotStart`), but exclude these proxy auth errors from the transport-exhausted logic that returns them to the client.

Jira issue: CRDB-57438

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.