DistSender: don't wrap in transactions unnecessarily

ResolveIntent requests are often sent in batches even when they span
ranges, and currently the DistSender will try to wrap those in
transactions, even though these requests do not benefit from being
in a transaction.

The immediate motivation for this change is that transactions are
subject to infinite retries (unlike non-transactional requests),
so the transaction-wrapped version would timeout at shutdown.

kv/dist_sender.go

@@ -518,7 +518,8 @@ func (ds *DistSender) sendChunk(ctx context.Context, ba roachpb.BatchRequest) (*
 			// re-run as part of a transaction for consistency. The
 			// case where we don't need to re-run is if the read
 			// consistency is not required.
-			if needAnother && ba.Txn == nil && ba.ReadConsistency != roachpb.INCONSISTENT {
+			if needAnother && ba.Txn == nil && ba.IsPossibleTransaction() &&
+				ba.ReadConsistency != roachpb.INCONSISTENT {
 				return nil, roachpb.NewError(&roachpb.OpRequiresTxnError{})
 			}
 

roachpb/api.go

@@ -63,6 +63,7 @@ const (
 	isAdmin    = 1 << iota // admin cmds don't go through raft, but run on leader
 	isRead                 // read-only cmds don't go through raft, but may run on leader
 	isWrite                // write cmds go through raft and must be proposed on leader
+	isTxn                  // txn commands may be part of a transaction
 	isTxnWrite             // txn write cmds start heartbeat and are marked for intent resolution
 	isRange                // range commands may span multiple keys
 	isReverse              // reverse commands traverse ranges in descending direction
@@ -506,22 +507,22 @@ func NewReverseScan(key, endKey Key, maxResults int64) Request {
 	}
 }
 
-func (*GetRequest) flags() int                { return isRead }
-func (*PutRequest) flags() int                { return isWrite | isTxnWrite }
-func (*ConditionalPutRequest) flags() int     { return isRead | isWrite | isTxnWrite }
-func (*IncrementRequest) flags() int          { return isRead | isWrite | isTxnWrite }
-func (*DeleteRequest) flags() int             { return isWrite | isTxnWrite }
-func (*DeleteRangeRequest) flags() int        { return isWrite | isTxnWrite | isRange }
-func (*ScanRequest) flags() int               { return isRead | isRange }
-func (*ReverseScanRequest) flags() int        { return isRead | isRange | isReverse }
-func (*BeginTransactionRequest) flags() int   { return isWrite }
-func (*EndTransactionRequest) flags() int     { return isWrite | isAlone }
+func (*GetRequest) flags() int                { return isRead | isTxn }
+func (*PutRequest) flags() int                { return isWrite | isTxn | isTxnWrite }
+func (*ConditionalPutRequest) flags() int     { return isRead | isWrite | isTxn | isTxnWrite }
+func (*IncrementRequest) flags() int          { return isRead | isWrite | isTxn | isTxnWrite }
+func (*DeleteRequest) flags() int             { return isWrite | isTxn | isTxnWrite }
+func (*DeleteRangeRequest) flags() int        { return isWrite | isTxn | isTxnWrite | isRange }
+func (*ScanRequest) flags() int               { return isRead | isRange | isTxn }
+func (*ReverseScanRequest) flags() int        { return isRead | isRange | isReverse | isTxn }
+func (*BeginTransactionRequest) flags() int   { return isWrite | isTxn }
+func (*EndTransactionRequest) flags() int     { return isWrite | isTxn | isAlone }
 func (*AdminSplitRequest) flags() int         { return isAdmin | isAlone }
 func (*AdminMergeRequest) flags() int         { return isAdmin | isAlone }
-func (*HeartbeatTxnRequest) flags() int       { return isWrite }
+func (*HeartbeatTxnRequest) flags() int       { return isWrite | isTxn }
 func (*GCRequest) flags() int                 { return isWrite | isRange }
 func (*PushTxnRequest) flags() int            { return isWrite }
-func (*RangeLookupRequest) flags() int        { return isRead }
+func (*RangeLookupRequest) flags() int        { return isRead | isTxn }
 func (*ResolveIntentRequest) flags() int      { return isWrite }
 func (*ResolveIntentRangeRequest) flags() int { return isWrite | isRange }
 func (*NoopRequest) flags() int               { return isRead } // slightly special

roachpb/batch.go

@@ -45,6 +45,12 @@ func (ba *BatchRequest) IsReverse() bool {
 	return (ba.flags() & isReverse) != 0
 }
 
+// IsPossibleTransaction returns true iff the BatchRequest contains
+// requests that can be part of a transaction.
+func (ba *BatchRequest) IsPossibleTransaction() bool {
+	return (ba.flags() & isTxn) != 0
+}
+
 // IsTransactionWrite returns true iff the BatchRequest contains a txn write.
 func (ba *BatchRequest) IsTransactionWrite() bool {
 	return (ba.flags() & isTxnWrite) != 0