Skip to content

Commit

Permalink
base: add DialTimeout
Browse files Browse the repository at this point in the history
This patch adds a `DialTimeout` constant, set to `2 * NetworkTimeout` to
account for the additional roundtrips in TCP + TLS handshakes.

Release note: None
  • Loading branch information
erikgrinaker committed Dec 1, 2022
1 parent 0481b90 commit 00f22fe
Show file tree
Hide file tree
Showing 7 changed files with 20 additions and 17 deletions.
5 changes: 5 additions & 0 deletions pkg/base/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,11 @@ func DefaultHistogramWindowInterval() time.Duration {
}

var (
// DialTimeout is the timeout used when dialing nodes. For gRPC, this is 3
// roundtrips (TCP + TLS handshake), so we set it to twice the network
// timeout.
DialTimeout = 2 * NetworkTimeout

// defaultRaftElectionTimeoutTicks specifies the number of Raft Tick
// invocations that must pass between elections.
defaultRaftElectionTimeoutTicks = envutil.EnvOrDefaultInt(
Expand Down
3 changes: 2 additions & 1 deletion pkg/rpc/addjoin.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"math"
"time"

"github.com/cockroachdb/cockroach/pkg/base"
"google.golang.org/grpc"
"google.golang.org/grpc/backoff"
"google.golang.org/grpc/credentials"
Expand All @@ -38,7 +39,7 @@ func GetAddJoinDialOptions(certPool *x509.CertPool) []grpc.DialOption {
backoffConfig.MaxDelay = time.Second
dialOpts = append(dialOpts, grpc.WithConnectParams(grpc.ConnectParams{
Backoff: backoffConfig,
MinConnectTimeout: minConnectionTimeout}))
MinConnectTimeout: base.DialTimeout}))
dialOpts = append(dialOpts, grpc.WithKeepaliveParams(clientKeepalive))
dialOpts = append(dialOpts,
grpc.WithInitialWindowSize(initialWindowSize),
Expand Down
16 changes: 6 additions & 10 deletions pkg/rpc/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,6 @@ var (
"COCKROACH_RANGEFEED_RPC_INITIAL_WINDOW_SIZE", RangefeedClass, 2*defaultWindowSize /* 128K */)
)

// GRPC Dialer connection timeout.
var minConnectionTimeout = 5 * time.Second

// errDialRejected is returned from client interceptors when the server's
// stopper is quiescing. The error is constructed to return true in
// `grpcutil.IsConnectionRejected` which prevents infinite retry loops during
Expand Down Expand Up @@ -1751,18 +1748,17 @@ func (rpcCtx *Context) grpcDialRaw(
// our setup with onlyOnceDialer below. So note that our choice here is
// inconsequential assuming all works as designed.
backoff := time.Second
if backoff > minConnectionTimeout {
// This is for testing where we set a small minConnectionTimeout.
// gRPC will internally round up the min connection timeout to the max
// backoff. This can be unintuitive and so we opt out of it by lowering the
// max backoff.
backoff = minConnectionTimeout
if backoff > base.DialTimeout {
// This is for testing where we set a small DialTimeout. gRPC will
// internally round up the min connection timeout to the max backoff. This
// can be unintuitive and so we opt out of it by lowering the max backoff.
backoff = base.DialTimeout
}
backoffConfig.BaseDelay = backoff
backoffConfig.MaxDelay = backoff
dialOpts = append(dialOpts, grpc.WithConnectParams(grpc.ConnectParams{
Backoff: backoffConfig,
MinConnectTimeout: minConnectionTimeout}))
MinConnectTimeout: base.DialTimeout}))
dialOpts = append(dialOpts, grpc.WithKeepaliveParams(clientKeepalive))
dialOpts = append(dialOpts, grpc.WithInitialConnWindowSize(initialConnWindowSize))
if class == RangefeedClass {
Expand Down
7 changes: 4 additions & 3 deletions pkg/rpc/down_node_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"testing"
"time"

"github.com/cockroachdb/cockroach/pkg/base"
"github.com/cockroachdb/cockroach/pkg/testutils"
"github.com/cockroachdb/cockroach/pkg/util"
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
Expand All @@ -33,9 +34,9 @@ func TestConnectingToDownNode(t *testing.T) {

{
defer func(prev time.Duration) {
minConnectionTimeout = prev
}(minConnectionTimeout)
minConnectionTimeout = time.Millisecond
base.DialTimeout = prev
}(base.DialTimeout)
base.DialTimeout = time.Millisecond
}

testutils.RunTrueAndFalse(t, "refused", func(t *testing.T, refused bool) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/server/pagination.go
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ func (r *rpcNodePaginator) queryNode(ctx context.Context, nodeID roachpb.NodeID,
r.mu.currentIdx++
r.mu.turnCond.Broadcast()
}
if err := contextutil.RunWithTimeout(ctx, "dial node", base.NetworkTimeout, func(ctx context.Context) error {
if err := contextutil.RunWithTimeout(ctx, "dial node", base.DialTimeout, func(ctx context.Context) error {
var err error
client, err = r.dialFn(ctx, nodeID)
return err
Expand Down
2 changes: 1 addition & 1 deletion pkg/server/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -2681,7 +2681,7 @@ func (s *statusServer) iterateNodes(

nodeQuery := func(ctx context.Context, nodeID roachpb.NodeID) {
var client interface{}
err := contextutil.RunWithTimeout(ctx, "dial node", base.NetworkTimeout, func(ctx context.Context) error {
err := contextutil.RunWithTimeout(ctx, "dial node", base.DialTimeout, func(ctx context.Context) error {
var err error
client, err = dialFn(ctx, nodeID)
return err
Expand Down
2 changes: 1 addition & 1 deletion pkg/server/tenant_status.go
Original file line number Diff line number Diff line change
Expand Up @@ -747,7 +747,7 @@ func (t *tenantStatusServer) iteratePods(

instanceQuery := func(ctx context.Context, instance sqlinstance.InstanceInfo) {
var client interface{}
err := contextutil.RunWithTimeout(ctx, "dial instance", base.NetworkTimeout, func(ctx context.Context) error {
err := contextutil.RunWithTimeout(ctx, "dial instance", base.DialTimeout, func(ctx context.Context) error {
var err error
client, err = dialFn(ctx, instance.InstanceID, instance.InstanceAddr)
return err
Expand Down

0 comments on commit 00f22fe

Please sign in to comment.