Skip to content

Commit

Permalink
kv: Don't treat context cancellation as a SendError
Browse files Browse the repository at this point in the history
SendErrors cause range descriptors to be evicted from the range cache,
but happen innocuously all the time as requests like HeartbeatTxns are
cancelled because they're no longer needed.

This was part of the cause of range 1 getting thousands of qps on 30
node TPC-C testing, as initially called out in the comments on cockroachdb#26608.

Release note: None
  • Loading branch information
a-robinson committed Jun 16, 2018
1 parent b3f1d17 commit c6c7ab1
Showing 1 changed file with 15 additions and 10 deletions.
25 changes: 15 additions & 10 deletions pkg/kv/dist_sender.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/util/metric"
"github.com/cockroachdb/cockroach/pkg/util/retry"
"github.com/cockroachdb/cockroach/pkg/util/tracing"
"github.com/pkg/errors"
)

const (
Expand Down Expand Up @@ -1294,6 +1295,20 @@ func (ds *DistSender) sendToReplicas(
log.VErrEventf(ctx, 1, "application error: %s", br.Error)
}

// Has the caller given up?
if ctx.Err() != nil {
errMsg := fmt.Sprintf("context done during DistSender.Send: %s", ctx.Err())
log.Eventf(ctx, errMsg)
if ambiguousError != nil {
return nil, roachpb.NewAmbiguousResultError(errMsg)
}
// Don't consider this a SendError, because SendErrors indicate that we
// were unable to reach a replica that could serve the request, and they
// cause range cache evictions. Context cancellations just mean the
// sender changed its mind or the request timed out.
return nil, errors.New(errMsg)
}

if transport.IsExhausted() {
if ambiguousError != nil {
return nil, roachpb.NewAmbiguousResultError(fmt.Sprintf("error=%s [exhausted]", ambiguousError))
Expand All @@ -1308,16 +1323,6 @@ func (ds *DistSender) sendToReplicas(
)
}

// Has the caller given up?
if ctx.Err() != nil {
errMsg := fmt.Sprintf("context done during DistSender.Send: %s", ctx.Err())
log.Eventf(ctx, errMsg)
if ambiguousError != nil {
return nil, roachpb.NewAmbiguousResultError(errMsg)
}
return nil, roachpb.NewSendError(errMsg)
}

ds.metrics.NextReplicaErrCount.Inc(1)
log.VEventf(ctx, 2, "error: %v %v; trying next peer %s", br, err, transport.NextReplica())
br, err = transport.SendNext(ctx)
Expand Down

0 comments on commit c6c7ab1

Please sign in to comment.