Skip to content

Commit

Permalink
rpc: disable use of http proxies by default
Browse files Browse the repository at this point in the history
GRPC uses the HTTPS_PROXY environment variable by default[1]. This is
surprising, and likely undesirable for CRDB because it turns the proxy
into an availability risk and a throughput bottleneck. We disable the
use of proxies by default when retrieving grpc dial options.

This diff came up in the context of cockroachdb#55289, and was a recent regression
in 20.2 after having introduced the join rpc in cockroachdb#52526 (it's the rpc
responsible for adding new nodes to the cluster cockroachdb#52526). Our existing
RPC connections use the `WithContextDialer` option already, which has the
side effect of disabling proxy use. The join RPC didn't, so an existing
cluster upgrade to 20.2 wouldn't use the proxy until a node was added.

Fixes cockroachdb#55289. This will need to get backported to release-20.2.

[1]: https://github.com/grpc/grpc-go/blob/c0736608/Documentation/proxy.md

Release note (bug fix): Previously we used the HTTPS_PROXY variable for
the "join RPC" when adding a node to the cluster (the RPC prevents new
clusters from starting or adding nodes to an existing cluster). The
proxy needed to configured to transparently pass HTTP/2+GRPC inter-node
traffic. This was an unintentional addition, and this patch ignores the
proxies for all intra-node traffic. They were already ignored in
releases prior to 20.2.
  • Loading branch information
irfansharif committed Oct 13, 2020
1 parent 8c603a1 commit 2a90fe6
Showing 1 changed file with 8 additions and 0 deletions.
8 changes: 8 additions & 0 deletions pkg/rpc/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -733,6 +733,14 @@ func (ctx *Context) grpcDialOptions(
dialOpts = append(dialOpts, grpc.WithDefaultCallOptions(grpc.UseCompressor((snappyCompressor{}).Name())))
}

// GRPC uses the HTTPS_PROXY environment variable by default[1]. This is
// surprising, and likely undesirable for CRDB because it turns the proxy
// into an availability risk and a throughput bottleneck. We disable the use
// of proxies by default.
//
// [1]: https://github.com/grpc/grpc-go/blob/c0736608/Documentation/proxy.md
dialOpts = append(dialOpts, grpc.WithNoProxy())

var unaryInterceptors []grpc.UnaryClientInterceptor

if tracer := ctx.AmbientCtx.Tracer; tracer != nil {
Expand Down

0 comments on commit 2a90fe6

Please sign in to comment.