Skip to content

Commit bd9e2a6

Browse files
committed
kv: gRPC Unavailable errors are ambiguous
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 #17491
1 parent 361c35e commit bd9e2a6

File tree

1 file changed

+4
-15
lines changed

1 file changed

+4
-15
lines changed

pkg/kv/dist_sender.go

+4-15
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,6 @@ import (
1919
"sync/atomic"
2020
"unsafe"
2121

22-
"google.golang.org/grpc"
23-
"google.golang.org/grpc/codes"
24-
2522
"golang.org/x/net/context"
2623

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

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

0 commit comments

Comments
 (0)