From 19abbbdf67f5df509f585d158d4a618d21b7bbc0 Mon Sep 17 00:00:00 2001 From: Nathan VanBenschoten Date: Tue, 24 Apr 2018 13:38:53 -0400 Subject: [PATCH] storage: bump LastHeartbeat timestamp when writing txn record Fixes #23945. See #20448. This change addresses a case where delayed BeginTxn requests can result in txn records looking inactive immediately upon being written. We now bump the txn record's LastHeartbeat timestamp when writing the record. Release note: None --- .../batcheval/cmd_begin_transaction.go | 23 +++++++++++++------ 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/pkg/storage/batcheval/cmd_begin_transaction.go b/pkg/storage/batcheval/cmd_begin_transaction.go index b9371b95917f..d43ee84d5e10 100644 --- a/pkg/storage/batcheval/cmd_begin_transaction.go +++ b/pkg/storage/batcheval/cmd_begin_transaction.go @@ -70,7 +70,8 @@ func BeginTransaction( clonedTxn := h.Txn.Clone() reply.Txn = &clonedTxn - // Verify transaction does not already exist. + // Check whether the transaction record already exists. If it already + // exists, check its current status and react accordingly. tmpTxn := roachpb.Transaction{} ok, err := engine.MVCCGetProto(ctx, batch, key, hlc.Timestamp{}, true, nil, &tmpTxn) if err != nil { @@ -109,18 +110,26 @@ func BeginTransaction( } } - threshold := cArgs.EvalCtx.GetTxnSpanGCThreshold() - - // Disallow creation of a transaction record if it's at a timestamp before - // the TxnSpanGCThreshold, as in that case our transaction may already have - // been aborted by a concurrent actor which encountered one of our intents - // (which may have been written before this entry). + // Disallow creation or modification of a transaction record if it's at a + // timestamp before the TxnSpanGCThreshold, as in that case our transaction + // may already have been aborted by a concurrent actor which encountered one + // of our intents (which may have been written before this entry). // // See #9265. + threshold := cArgs.EvalCtx.GetTxnSpanGCThreshold() if reply.Txn.LastActive().Less(threshold) { return result.Result{}, roachpb.NewTransactionAbortedError() } + // Transaction heartbeats don't begin until after the transaction record + // has been laid down and the response has returned to the transaction + // coordinator. This poses a problem for BeginTxn requests that get + // delayed, because it's possible that the transaction records they + // write will look inactive immediately after being written. To avoid + // this situation resulting in transactions being aborted unnecessarily, + // we bump the record's heartbeat timestamp right before laying it down. + reply.Txn.LastHeartbeat.Forward(cArgs.EvalCtx.Clock().Now()) + // Write the txn record. reply.Txn.Writing = true return result.Result{}, engine.MVCCPutProto(ctx, batch, cArgs.Stats, key, hlc.Timestamp{}, nil, reply.Txn)