From 208f59b92b7818aba9f3d587f0ee63eafd67ee54 Mon Sep 17 00:00:00 2001
From: Andrei Matei <andrei@cockroachlabs.com>
Date: Fri, 28 Sep 2018 12:54:42 -0400
Subject: [PATCH] Revert "client: don't allocate some commit batches"

This reverts commit 06c1adf483fbbfcd82b28a6346b62da5b0e5ca0d.

Revert the last commit from #30485 - the one that pre-allocation and
possible reuse of EndTransaction batches. It's causing races.
I'll figure it out and re-send the original patch.
Touches #30769

Release note: None
---
 pkg/internal/client/txn.go | 36 ++----------------------------------
 1 file changed, 2 insertions(+), 34 deletions(-)

diff --git a/pkg/internal/client/txn.go b/pkg/internal/client/txn.go
index 06d0f9900dd4..e1a3f1860023 100644
--- a/pkg/internal/client/txn.go
+++ b/pkg/internal/client/txn.go
@@ -38,18 +38,6 @@ type Txn struct {
 	// typ indicates the type of transaction.
 	typ TxnType
 
-	// alloc holds pre-allocated fields that can be used to avoid heap allocations
-	// for batches with lifetimes tied to a Txn.
-	// TODO(andrei): A lot more things should be pre-allocated or otherwise pooled
-	// - in particular the roachpb.Transaction proto and the TxnCoordSender which
-	// are quite large. Pooling them would also force us to clean up their
-	// lifetimes, which is a good idea on its own.
-	alloc struct {
-		requests [1]roachpb.RequestUnion
-		etUnion  roachpb.RequestUnion_EndTransaction
-		et       roachpb.EndTransactionRequest
-	}
-
 	// gatewayNodeID, if != 0, is the ID of the node on whose behalf this
 	// transaction is running. Normally this is the current node, but in the case
 	// of Txns created on remote nodes by DistSQL this will be the gateway.
@@ -521,7 +509,8 @@ func (txn *Txn) Run(ctx context.Context, b *Batch) error {
 }
 
 func (txn *Txn) commit(ctx context.Context) error {
-	ba := txn.getCommitReq(txn.deadline(), txn.systemConfigTrigger)
+	var ba roachpb.BatchRequest
+	ba.Add(endTxnReq(true /* commit */, txn.deadline(), txn.systemConfigTrigger))
 	_, pErr := txn.Send(ctx, ba)
 	if pErr == nil {
 		for _, t := range txn.commitTriggers {
@@ -545,27 +534,6 @@ func (txn *Txn) CleanupOnError(ctx context.Context, err error) {
 	}
 }
 
-func (txn *Txn) getCommitReq(deadline *hlc.Timestamp, hasTrigger bool) roachpb.BatchRequest {
-	ba := roachpb.BatchRequest{
-		Requests: txn.alloc.requests[:1],
-	}
-	etReq := &txn.alloc.et
-	etUnion := &txn.alloc.etUnion
-	ba.Requests[0].Value = etUnion
-	etUnion.EndTransaction = etReq
-	etReq.Reset()
-	etReq.Commit = true
-	etReq.Deadline = deadline
-	if hasTrigger {
-		etReq.InternalCommitTrigger = &roachpb.InternalCommitTrigger{
-			ModifiedSpanTrigger: &roachpb.ModifiedSpanTrigger{
-				SystemConfigSpan: true,
-			},
-		}
-	}
-	return ba
-}
-
 // Commit is the same as CommitOrCleanup but will not attempt to clean
 // up on failure. This can be used when the caller is prepared to do proper
 // cleanup.