cockroachdb / cockroachdb/cockroach
kvcoord: retry requests that hit proxy auth errors
- 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
Assessment
This issue has not been assessed yet.