From cd713efebb7fd76eb331cf8adaac14d3f1b46a67 Mon Sep 17 00:00:00 2001 From: Nathan VanBenschoten Date: Wed, 10 Apr 2019 13:52:42 -0400 Subject: [PATCH] kv: ignore result of txn heartbeat after EndTransaction The changes in #33396 made it so that a `HeartbeatTxnRequest` that finds a missing transaction record will attempt to create the record. This means that it will discover if a transaction is not "committable" and return a TransactionAbortedError. Unfortunately, after a transaction commits and GCs its transaction record, it will also be considered not "committable". There will always be cases where a heartbeat request races with an EndTransaction request and incorrectly considers the transaction aborted (which is touched upon in a TODO a few lines down). However, in many of these cases, the coordinator already knows that the transaction is committed, so it doesn't need to attempt to roll back the transaction and clean up its intents. This commit checks for these cases and avoids sending useless rollbacks. The intention is to backport this to 19.1. Release note: None --- pkg/kv/txn_interceptor_heartbeater.go | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/pkg/kv/txn_interceptor_heartbeater.go b/pkg/kv/txn_interceptor_heartbeater.go index 8dc20341cfcc..a1cbefc56b1a 100644 --- a/pkg/kv/txn_interceptor_heartbeater.go +++ b/pkg/kv/txn_interceptor_heartbeater.go @@ -375,25 +375,28 @@ func (h *txnHeartbeater) heartbeat(ctx context.Context) bool { // Clone the txn in order to put it in the heartbeat request. txn := h.mu.txn.Clone() - if txn.Key == nil { log.Fatalf(ctx, "attempting to heartbeat txn without anchor key: %v", txn) } - ba := roachpb.BatchRequest{} ba.Txn = txn - - hb := &roachpb.HeartbeatTxnRequest{ + ba.Add(&roachpb.HeartbeatTxnRequest{ RequestHeader: roachpb.RequestHeader{ Key: txn.Key, }, Now: h.clock.Now(), - } - ba.Add(hb) + }) + // Send the heartbeat request directly through the gatekeeper interceptor. + // See comment on h.gatekeeper for a discussion of why. log.VEvent(ctx, 2, "heartbeat") br, pErr := h.gatekeeper.SendLocked(ctx, ba) + // If the txn is no longer pending, ignore the result of the heartbeat. + if h.mu.txn.Status != roachpb.PENDING { + return false + } + var respTxn *roachpb.Transaction if pErr != nil { log.VEventf(ctx, 2, "heartbeat failed: %s", pErr)