From 82dff93c5af28c0f80461e7bda514d75b97f8450 Mon Sep 17 00:00:00 2001
From: Ben Darnell <ben@cockroachlabs.com>
Date: Tue, 3 Nov 2015 00:08:37 -0500
Subject: [PATCH] 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 |  3 ++-
 roachpb/api.go    | 25 +++++++++++++------------
 roachpb/batch.go  |  6 ++++++
 3 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/kv/dist_sender.go b/kv/dist_sender.go
index 5aa0a9c952e8..bbdcdcd72fc2 100644
--- a/kv/dist_sender.go
+++ b/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{})
 			}
 
diff --git a/roachpb/api.go b/roachpb/api.go
index 33efaf37953e..6b6860aaa66a 100644
--- a/roachpb/api.go
+++ b/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
diff --git a/roachpb/batch.go b/roachpb/batch.go
index 7fac336c3dd8..1f85b678187b 100644
--- a/roachpb/batch.go
+++ b/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