Skip to content

Commit

Permalink
Merge #30773
Browse files Browse the repository at this point in the history
30773: Revert "client: don't allocate some commit batches" r=andreimatei a=andreimatei

This reverts commit 06c1adf.

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

Co-authored-by: Andrei Matei <[email protected]>
  • Loading branch information
craig[bot] and andreimatei committed Sep 28, 2018
2 parents 403cd3e + 208f59b commit 4ce1f0e
Showing 1 changed file with 2 additions and 34 deletions.
36 changes: 2 additions & 34 deletions pkg/internal/client/txn.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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 {
Expand All @@ -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.
Expand Down

0 comments on commit 4ce1f0e

Please sign in to comment.