diff --git a/pkg/storage/mvcc.go b/pkg/storage/mvcc.go index 5a5b243a258e..eb8de8eb3c27 100644 --- a/pkg/storage/mvcc.go +++ b/pkg/storage/mvcc.go @@ -5264,7 +5264,13 @@ func mvccResolveWriteIntent( var rolledBackVal *MVCCValue buf.newMeta = *meta newMeta := &buf.newMeta - if len(update.IgnoredSeqNums) > 0 { + // Update the MVCC history only if: + // 1. There are IgnoredSeqNums present. + // 2. The update is not going to abort the intent; otherwise, the entire + // history will be removed anyway. + // 3. The epochs of the intent and the update match; otherwise the epochs may + // have different seq nums (and ignored seq nums). + if len(update.IgnoredSeqNums) > 0 && (commit || inProgress) && epochsMatch { // NOTE: mvccMaybeRewriteIntentHistory mutates its meta argument. // TODO(nvanbenschoten): this is an awkward interface. We shouldn't // be mutating meta and we shouldn't be restoring the previous value @@ -5431,6 +5437,7 @@ func mvccResolveWriteIntent( // Update or remove the metadata key. var metaKeySize, metaValSize int64 + var logicalOp MVCCLogicalOpType if !commit { // Keep existing intent if we're updating it. We update the existing // metadata's timestamp instead of using the supplied intent meta to avoid @@ -5440,6 +5447,7 @@ func mvccResolveWriteIntent( outcome = lockOverwritten metaKeySize, metaValSize, err = buf.putLockMeta( writer, metaKey.Key, lock.Intent, newMeta, true /* alreadyExists */) + logicalOp = MVCCUpdateIntentOpType } else { outcome = lockClearedByDelete useSingleDelete := canSingleDelHelper.onCommitLock() @@ -5451,6 +5459,7 @@ func mvccResolveWriteIntent( ValueSizeKnown: true, ValueSize: uint32(origMetaValSize), }) + logicalOp = MVCCCommitIntentOpType } if err != nil { return lockNoop, err @@ -5463,10 +5472,6 @@ func mvccResolveWriteIntent( } // Log the logical MVCC operation. - logicalOp := MVCCCommitIntentOpType - if pushed { - logicalOp = MVCCUpdateIntentOpType - } writer.LogLogicalOp(logicalOp, MVCCLogicalOpDetails{ Txn: update.Txn, Key: update.Key, diff --git a/pkg/storage/testdata/mvcc_histories/ignored_seq_nums b/pkg/storage/testdata/mvcc_histories/ignored_seq_nums index bc92ce8839f1..bc01389c4010 100644 --- a/pkg/storage/testdata/mvcc_histories/ignored_seq_nums +++ b/pkg/storage/testdata/mvcc_histories/ignored_seq_nums @@ -675,9 +675,6 @@ clear_range k=k end=p # Try to update a pending intent with a lower epoch than the resolution. # The intent should be aborted because the new epoch may not write it again. -# The test exposes a bug where the intent is updated instead of aborted. -# Moreover, it logs MVCCCommitIntentOp instead of MVCCUpdateIntentOp. - run ok with t=G k=p txn_begin ts=50 @@ -708,9 +705,8 @@ resolve_intent: "p" -> resolved key = true get: "p" -> >> at end: txn: "G" meta={id=00000007 key="p" iso=Serializable pri=0.00000000 epo=1 ts=50.000000000,0 min=0,0 seq=0} lock=true stat=PENDING rts=50.000000000,0 wto=false gul=0,0 isn=1 -meta: "p"/0,0 -> txn={id=00000007 key="p" iso=Serializable pri=0.00000000 epo=0 ts=50.000000000,0 min=0,0 seq=10} ts=50.000000000,0 del=false klen=12 vlen=6 mergeTs= txnDidNotUpdateMeta=false -data: "p"/50.000000000,0 -> /BYTES/a -logical op: *enginepb.MVCCCommitIntentOp + +logical op: *enginepb.MVCCAbortIntentOp run ok clear_range k=p end=-p @@ -722,10 +718,6 @@ clear_range k=p end=-p # The intent is not pushed, so it is not updated because the intent history # should be updated only when the epochs match. -# The test exposes a bug where the intent is actually updated, as evident by the -# changed intent history. -# Moreover, it logs MVCCCommitIntentOp instead of MVCCUpdateIntentOp. - run ok with t=H k=q txn_begin ts=50 @@ -753,13 +745,12 @@ with t=H k=q resolve_intent status=PENDING check_intent ---- -resolve_intent: "q" -> resolved key = true -meta: "q" -> txn={id=00000008 key="q" iso=Serializable pri=0.00000000 epo=1 ts=50.000000000,0 min=0,0 seq=10} ts=50.000000000,0 del=false klen=12 vlen=6 mergeTs= txnDidNotUpdateMeta=false +resolve_intent: "q" -> resolved key = false +meta: "q" -> txn={id=00000008 key="q" iso=Serializable pri=0.00000000 epo=1 ts=50.000000000,0 min=0,0 seq=20} ts=50.000000000,0 del=false klen=12 vlen=6 ih={{10 /BYTES/a}} mergeTs= txnDidNotUpdateMeta=false >> at end: txn: "H" meta={id=00000008 key="q" iso=Serializable pri=0.00000000 epo=0 ts=50.000000000,0 min=0,0 seq=0} lock=true stat=PENDING rts=50.000000000,0 wto=false gul=0,0 isn=1 -meta: "q"/0,0 -> txn={id=00000008 key="q" iso=Serializable pri=0.00000000 epo=1 ts=50.000000000,0 min=0,0 seq=10} ts=50.000000000,0 del=false klen=12 vlen=6 mergeTs= txnDidNotUpdateMeta=false -data: "q"/50.000000000,0 -> /BYTES/a -logical op: *enginepb.MVCCCommitIntentOp +meta: "q"/0,0 -> txn={id=00000008 key="q" iso=Serializable pri=0.00000000 epo=1 ts=50.000000000,0 min=0,0 seq=20} ts=50.000000000,0 del=false klen=12 vlen=6 ih={{10 /BYTES/a}} mergeTs= txnDidNotUpdateMeta=false +data: "q"/50.000000000,0 -> /BYTES/b run ok clear_range k=q end=-q @@ -771,8 +762,6 @@ clear_range k=q end=-q # The intent is pushed, so its timestamp is updated to help the pusher make progress. # The intent history is not updated because the epochs don't match. -# The test exposes a bug where the intent history is actually updated. - run ok with t=I k=r txn_begin ts=50 @@ -802,9 +791,9 @@ with t=I k=r check_intent ---- resolve_intent: "r" -> resolved key = true -meta: "r" -> txn={id=00000009 key="r" iso=Serializable pri=0.00000000 epo=1 ts=60.000000000,0 min=0,0 seq=10} ts=60.000000000,0 del=false klen=12 vlen=20 mergeTs= txnDidNotUpdateMeta=false +meta: "r" -> txn={id=00000009 key="r" iso=Serializable pri=0.00000000 epo=1 ts=60.000000000,0 min=0,0 seq=20} ts=60.000000000,0 del=false klen=12 vlen=20 ih={{10 /BYTES/a}} mergeTs= txnDidNotUpdateMeta=false >> at end: txn: "I" meta={id=00000009 key="r" iso=Serializable pri=0.00000000 epo=0 ts=60.000000000,0 min=0,0 seq=0} lock=true stat=PENDING rts=50.000000000,0 wto=false gul=0,0 isn=1 -meta: "r"/0,0 -> txn={id=00000009 key="r" iso=Serializable pri=0.00000000 epo=1 ts=60.000000000,0 min=0,0 seq=10} ts=60.000000000,0 del=false klen=12 vlen=20 mergeTs= txnDidNotUpdateMeta=false -data: "r"/60.000000000,0 -> {localTs=50.000000000,0}/BYTES/a +meta: "r"/0,0 -> txn={id=00000009 key="r" iso=Serializable pri=0.00000000 epo=1 ts=60.000000000,0 min=0,0 seq=20} ts=60.000000000,0 del=false klen=12 vlen=20 ih={{10 /BYTES/a}} mergeTs= txnDidNotUpdateMeta=false +data: "r"/60.000000000,0 -> {localTs=50.000000000,0}/BYTES/b logical op: *enginepb.MVCCUpdateIntentOp diff --git a/pkg/storage/testdata/mvcc_histories/ignored_seq_nums_abort b/pkg/storage/testdata/mvcc_histories/ignored_seq_nums_abort index 7467a3a51455..0fd2d6368926 100644 --- a/pkg/storage/testdata/mvcc_histories/ignored_seq_nums_abort +++ b/pkg/storage/testdata/mvcc_histories/ignored_seq_nums_abort @@ -2,10 +2,6 @@ # The test exposes a bug where even though the transaction is aborted, its # intent is updated instead of aborted. -# The test exposes a bug where even though the transaction is aborted, its -# intent is updated instead of aborted. -# Moreover, it logs MVCCCommitIntentOp instead of MVCCUpdateIntentOp. - run ok log-ops with t=A k=k txn_begin ts=11 @@ -21,11 +17,10 @@ with t=A k=k put: lock acquisition = {k id=00000001 key="k" iso=Serializable pri=0.00000000 epo=0 ts=11.000000000,0 min=0,0 seq=10 Replicated Intent []} put: lock acquisition = {k id=00000001 key="k" iso=Serializable pri=0.00000000 epo=0 ts=11.000000000,0 min=0,0 seq=20 Replicated Intent []} resolve_intent: "k" -> resolved key = true -get: "k" -> /BYTES/a @11.000000000,0 +get: "k" -> >> at end: txn: "A" meta={id=00000001 key="k" iso=Serializable pri=0.00000000 epo=0 ts=11.000000000,0 min=0,0 seq=30} lock=true stat=PENDING rts=11.000000000,0 wto=false gul=0,0 isn=1 -meta: "k"/0,0 -> txn={id=00000001 key="k" iso=Serializable pri=0.00000000 epo=0 ts=11.000000000,0 min=0,0 seq=10} ts=11.000000000,0 del=false klen=12 vlen=6 mergeTs= txnDidNotUpdateMeta=false -data: "k"/11.000000000,0 -> /BYTES/a + logical op: *enginepb.MVCCWriteIntentOp logical op: *enginepb.MVCCUpdateIntentOp -logical op: *enginepb.MVCCCommitIntentOp +logical op: *enginepb.MVCCAbortIntentOp diff --git a/pkg/storage/testdata/mvcc_histories/ignored_seq_nums_commit b/pkg/storage/testdata/mvcc_histories/ignored_seq_nums_commit index 860f115b052d..d84ff0a7066d 100644 --- a/pkg/storage/testdata/mvcc_histories/ignored_seq_nums_commit +++ b/pkg/storage/testdata/mvcc_histories/ignored_seq_nums_commit @@ -142,9 +142,6 @@ get: "k" -> # Commit an intent with a lower epoch than the resolution. # The intent should be aborted because the new epoch may not write it again. -# The test exposes a bug where the intent is updated instead of aborted. -# Moreover, it logs MVCCCommitIntentOp instead of MVCCUpdateIntentOp. - run ok with t=B k=b txn_begin ts=12 @@ -179,9 +176,7 @@ resolve_intent: "b" -> resolved key = true get: "b" -> >> at end: txn: "B" meta={id=00000003 key="b" iso=Serializable pri=0.00000000 epo=1 ts=12.000000000,0 min=0,0 seq=0} lock=true stat=PENDING rts=12.000000000,0 wto=false gul=0,0 isn=1 -meta: "b"/0,0 -> txn={id=00000003 key="b" iso=Serializable pri=0.00000000 epo=0 ts=12.000000000,0 min=0,0 seq=10} ts=12.000000000,0 del=false klen=12 vlen=6 mergeTs= txnDidNotUpdateMeta=false -data: "b"/12.000000000,0 -> /BYTES/a data: "k"/11.000000000,0 -> /BYTES/c data: "k/10"/11.000000000,0 -> /BYTES/10 data: "k/30"/11.000000000,0 -> /BYTES/30 -logical op: *enginepb.MVCCCommitIntentOp +logical op: *enginepb.MVCCAbortIntentOp