Instead of simply deleting the automaticRetryClient, I think that it might be better to:

1. "disable" automatic retries by making each of these methods a pass-through method to the underlying client (not adding in the defaults.RetryPolicy call options)

   func (a *automaticRetryClient) Search(ctx context.Context, in *proto.SearchRequest, opts ...grpc.CallOption) (*proto.SearchResponse, error) {
   	return a.base.Search(ctx, in, opts...)
   }

   You can see how I do this for some gitserver methods here: https://github.com/sourcegraph/sourcegraph/blob/95a44f053d003699dc6dcbca999671c3e9638307/internal/gitserver/retry.go#L22-L39

2. Document for each method why we aren't using the default retry policy. In this case, it's not because the methods aren't idempotent, but instead because there is "higher level" retry policy in the application.

   You can see a similar comment that Ieft for gitserver's CreateCommitFromPatchBinary method to explain why I wasn't using the automatic retry support with it: https://github.com/sourcegraph/sourcegraph/blob/95a44f053d003699dc6dcbca999671c3e9638307/internal/gitserver/retry.go#L35-L39

   func (r *automaticRetryClient) CreateCommitFromPatchBinary(ctx context.Context, opts ...grpc.CallOption) (proto.GitserverService_CreateCommitFromPatchBinaryClient, error) {
   	// CreateCommitFromPatchBinary isn't idempotent. It also is a client-streaming method, which is currently unsupported by our automatic retry logic.
   	// The caller is responsible for implementing their own retry semantics for this method.
   	return r.base.CreateCommitFromPatchBinary(ctx, opts...)
   }

I believe this approach is more discoverable since it uses the same standardized pattern that other clients use. It's also a natural place that someone would look for documentation about retries.

Thanks for the detailed review! For this PR, I would like to completely remove the retry behavior when connecting to Zoekt, as opposed to doing it only for some methods. This matches the old behavior most closely, and will let us test this as a potential source of the instability/ latency spikes we've seen on dot com. As a follow-up, we can look into re-introducing retries for some methods, for example List.

Since we're totally eliminating retries, I think it makes the most sense to not keep wrapping things in a "retry client". However, I'll definitely add a comment here highlighting that Zoekt is taking a non-default approach!