Skip to content

Commit

Permalink
kv: gRPC Unavailable errors are ambiguous
Browse files Browse the repository at this point in the history
This error code is used for fail-fast errors (which can be retried
unambiguously), but it is also used in other cases (such as a server
draining) in which we cannot assume that the previous attempt was not
completed. (It's unclear whether this assumption was once true and
changed or if it's always been incorrect. The specific source of
ambiguous Unavailable errors we're seeing is grpc/grpc-go#1147)

This is expected to increase prevalence of AmbiguousResultErrors; this
will be fixed in a follow-up change.

Fixes cockroachdb#17491
  • Loading branch information
bdarnell committed Aug 17, 2017
1 parent 361c35e commit bd9e2a6
Showing 1 changed file with 4 additions and 15 deletions.
19 changes: 4 additions & 15 deletions pkg/kv/dist_sender.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,6 @@ import (
"sync/atomic"
"unsafe"

"google.golang.org/grpc"
"google.golang.org/grpc/codes"

"golang.org/x/net/context"

"github.com/cockroachdb/cockroach/pkg/base"
Expand Down Expand Up @@ -1162,10 +1159,9 @@ func (ds *DistSender) sendToReplicas(

case call := <-done:
if err := call.Err; err != nil {
// All connection errors except for an unavailable node (this
// is GRPC's fail-fast error), may mean that the request
// succeeded on the remote server, but we were unable to
// receive the reply. Set the ambiguous commit flag.
// All connection errors may mean that the request succeeded
// on the remote server, but we were unable to receive the
// reply. Set the ambiguous commit flag.
//
// We retry ambiguous commit batches to avoid returning the
// unrecoverable AmbiguousResultError. This is safe because
Expand All @@ -1176,14 +1172,7 @@ func (ds *DistSender) sendToReplicas(
// leader). If the original attempt merely timed out or was
// lost, then the batch will succeed and we can be assured the
// commit was applied just once.
//
// The Unavailable code is used by GRPC to indicate that a
// request fails fast and is not sent, so we can be sure there
// is no ambiguity on these errors. Note that these are common
// if a node is down.
// See https://github.com/grpc/grpc-go/blob/52f6504dc290bd928a8139ba94e3ab32ed9a6273/call.go#L182
// See https://github.com/grpc/grpc-go/blob/52f6504dc290bd928a8139ba94e3ab32ed9a6273/stream.go#L158
if haveCommit && grpc.Code(err) != codes.Unavailable {
if haveCommit {
ambiguousError = err
}
log.ErrEventf(ctx, "RPC error: %s", err)
Expand Down

0 comments on commit bd9e2a6

Please sign in to comment.