Skip to content

Commit

Permalink
kv,grpcutil: Add an explicit preflight check for grpc connection state
Browse files Browse the repository at this point in the history
This is similar to what grpc does internally for fail-fast rpcs, but
lets us control the error returned to work around grpc/grpc-go#1443

Fixes cockroachdb#19708

Release note (performance improvement): Reduced the occurrence of
ambiguous errors when a node is down.
  • Loading branch information
bdarnell committed Jan 10, 2018
1 parent 8e731c6 commit 6212f7e
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 0 deletions.
3 changes: 3 additions & 0 deletions pkg/kv/transport.go
Expand Up @@ -248,6 +248,9 @@ func (gt *grpcTransport) send(
if err != nil {
return nil, err
}
if err := grpcutil.ConnectionReady(conn); err != nil {
return nil, err
}
reply, err := roachpb.NewInternalClient(conn).Batch(ctx, &client.args)
if reply != nil {
for i := range reply.Responses {
Expand Down
31 changes: 31 additions & 0 deletions pkg/util/grpcutil/grpc_util.go
Expand Up @@ -16,6 +16,7 @@ package grpcutil

import (
"context"
"fmt"
"io"
"strings"

Expand All @@ -24,6 +25,7 @@ import (

"google.golang.org/grpc"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/connectivity"
"google.golang.org/grpc/status"
"google.golang.org/grpc/transport"
)
Expand Down Expand Up @@ -73,6 +75,9 @@ func IsClosedConnection(err error) bool {
// TODO(bdarnell): Replace this with a cleaner mechanism when/if
// https://github.com/grpc/grpc-go/issues/1443 is resolved.
func RequestDidNotStart(err error) bool {
if _, ok := err.(connectionNotReadyError); ok {
return true
}
s, ok := status.FromError(err)
if !ok {
// This is a non-gRPC error; assume nothing.
Expand All @@ -87,3 +92,29 @@ func RequestDidNotStart(err error) bool {
}
return false
}

// ConnectionReady returns nil if the given connection is ready to
// send a request, or an error (which will pass RequestDidNotStart) if
// not.
//
// This is a workaround for the fact that gRPC 1.7 fails to
// distinguish between ambiguous and unambiguous errors.
//
// This is designed for use with connections prepared by
// pkg/rpc.Connection.Connect (which performs an initial heartbeat and
// thereby ensures that we will never see a connection in the
// first-time Connecting state).
func ConnectionReady(conn *grpc.ClientConn) error {
if s := conn.GetState(); s == connectivity.TransientFailure {
return connectionNotReadyError{s}
}
return nil
}

type connectionNotReadyError struct {
state connectivity.State
}

func (e connectionNotReadyError) Error() string {
return fmt.Sprintf("connection not ready: %s", e.state)
}

0 comments on commit 6212f7e

Please sign in to comment.