Skip to content

Commit

Permalink
engine: Handle MVCCPut case with no committed value and all writes ig…
Browse files Browse the repository at this point in the history 
…nored

Updates mvccPutInternal to account for the case where all past writes
by this transaction at a key have been rolled back, and there was no
past committed value. This is denoted by txn.Epoch == meta.txn.Epoch,
and existingVal == nil (even after the inconsistent mvccGetInternal).
The intent history should be cleared in this case.

Also updates comments in the function to account for this exception.

Release note: None.
  • Loading branch information
@itsbilal
itsbilal committed Feb 5, 2020
1 parent 6e02915 commit b0c0cdc
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 39 deletions.
70 changes: 39 additions & 31 deletions pkg/storage/engine/mvcc.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1528,17 +1528,27 @@ func mvccPutInternal(
// that though - we must record the older value in the IntentHistory.

// But where to find the older value? There are 4 cases:
// - last write inside txn, same epoch, seqnum of last write is not ignored: value at key.
// => read the value associated with the intent with consistent mvccGetInternal().
// (This is the common case.)
// - last write inside txn, same epoch, seqnum of last write is ignored: cannot use value at key.
// - last write inside txn, same epoch, seqnum of last write is not
// ignored: value at key.
// => read the value associated with the intent with consistent
// mvccGetInternal(). (This is the common case.)
// - last write inside txn, same epoch, seqnum of last write is ignored:
// cannot use value at key.
// => try reading from intent history.
// => if all intent history entries are rolled back, fall back to last case below.
// - last write outside txn or at different epoch: use inconsistent mvccGetInternal,
// which will find it outside.
// => if all intent history entries are rolled back, fall back to last
// case below.
// - last write outside txn or at different epoch: use inconsistent
// mvccGetInternal, which will find it outside.
//
// (Note that _some_ value is guaranteed to be found, as indicated by ok == true above.)
// (Note that _some_ value is guaranteed to be found, as indicated by
// ok == true above. The one notable exception is when there are no past
// committed values, and all past writes by this transaction have been
// rolled back, either due to transaction retries or transaction savepoint
// rollbacks.)
var existingVal *roachpb.Value
// Aliases existingVal when the current provisional value is readable
// and not rolled back (due to a savepoint rollback or txn restart).
var curProvVal *roachpb.Value
if txn.Epoch == meta.Txn.Epoch /* last write inside txn */ {
if !enginepb.TxnSeqIsIgnored(meta.Txn.Sequence, txn.IgnoredSeqNums) {
// Seqnum of last write is not ignored. Retrieve the value
Expand All @@ -1553,17 +1563,17 @@ func mvccPutInternal(
if err != nil {
return err
}
curProvVal = existingVal
} else {
// Seqnum of last write was ignored. Try retrieving the value from the history.
prevIntent, prevValueWritten := meta.GetPrevIntentSeq(txn.Sequence, txn.IgnoredSeqNums)
if prevValueWritten {
existingVal = &roachpb.Value{RawBytes: prevIntent.Value}
}
}

}
if existingVal == nil {
// "last write inside txn && seqnum of last write is not ignored"
// "last write inside txn && seqnum of all writes are ignored"
// OR
// "last write outside txn"
// => use inconsistent mvccGetInternal to retrieve the last committed value at key.
Expand All @@ -1590,16 +1600,6 @@ func mvccPutInternal(
}
}

// It's possible that the existing value is nil if the intent on the key
// has a lower epoch. We don't have to deal with this as a special case
// because in this case, the value isn't written to the intent history.
// Instead, the intent history is blown away completely.
var prevIntentValBytes []byte
if existingVal != nil {
prevIntentValBytes = existingVal.RawBytes
}
prevIntentSequence := meta.Txn.Sequence

// We are replacing our own write intent. If we are writing at
// the same timestamp (see comments in else block) we can
// overwrite the existing intent; otherwise we must manually
Expand Down Expand Up @@ -1637,20 +1637,28 @@ func mvccPutInternal(
}
// Since an intent with a smaller sequence number exists for the
// same transaction, we must add the previous value and sequence
// to the intent history.
// to the intent history, if that previous value does not belong to an
// ignored sequence number.
//
// If the epoch of the transaction doesn't match the epoch of the
// intent, blow away the intent history.
if txn.Epoch == meta.Txn.Epoch {
if existingVal == nil {
// This case shouldn't pop up, but it is worth asserting
// that it doesn't. We shouldn't write invalid intents
// to the history.
return errors.Errorf(
"previous intent of the transaction with the same epoch not found for %s (%+v)",
metaKey, txn)
// intent, or if the existing value is nil due to all past writes being
// ignored and there are no other committed values, blow away the intent
// history.
//
// Note that the only case where txn.Epoch == meta.Txn.Epoch &&
// existingVal == nil will be true is when all past writes by this
// transaction are ignored, and there are no past committed values at this
// key. In that case, we can also blow up the intent history.
if txn.Epoch == meta.Txn.Epoch && existingVal != nil {
// Only add the current provisional value to the intent
// history if the current sequence number is not ignored. There's no
// reason to add past committed values or a value already in the intent
// history back into it.
if curProvVal != nil {
prevIntentValBytes := curProvVal.RawBytes
prevIntentSequence := meta.Txn.Sequence
buf.newMeta.AddToIntentHistory(prevIntentSequence, prevIntentValBytes)
}
buf.newMeta.AddToIntentHistory(prevIntentSequence, prevIntentValBytes)
} else {
buf.newMeta.IntentHistory = nil
}
Expand Down
8 changes: 4 additions & 4 deletions pkg/storage/engine/testdata/mvcc_histories/ignored_seq_nums_cput
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ run ok
cput t=A k=k cond=first v=b
----
>> at end:
meta: "k"/0,0 -> txn={id=00000000 key=/Min pri=0.00000000 epo=0 ts=0.000000011,0 min=0,0 seq=20} ts=0.000000011,0 del=false klen=12 vlen=6 ih={{10 /BYTES/first}}
meta: "k"/0,0 -> txn={id=00000000 key=/Min pri=0.00000000 epo=0 ts=0.000000011,0 min=0,0 seq=20} ts=0.000000011,0 del=false klen=12 vlen=6
data: "k"/0.000000011,0 -> /BYTES/b
data: "k"/0.000000001,0 -> /BYTES/first

Expand Down Expand Up @@ -86,7 +86,7 @@ run ok
cput t=B k=k cond=a v=c
----
>> at end:
meta: "k"/0,0 -> txn={id=00000000 key=/Min pri=0.00000000 epo=0 ts=0.000000011,0 min=0,0 seq=30} ts=0.000000011,0 del=false klen=12 vlen=6 ih={{10 /BYTES/a}{20 /BYTES/a}}
meta: "k"/0,0 -> txn={id=00000000 key=/Min pri=0.00000000 epo=0 ts=0.000000011,0 min=0,0 seq=30} ts=0.000000011,0 del=false klen=12 vlen=6 ih={{10 /BYTES/a}}
data: "k"/0.000000011,0 -> /BYTES/c
data: "k"/0.000000001,0 -> /BYTES/first

Expand Down Expand Up @@ -143,7 +143,7 @@ run ok
cput t=C k=k cond=a v=c
----
>> at end:
meta: "k"/0,0 -> txn={id=00000000 key=/Min pri=0.00000000 epo=0 ts=0.000000011,0 min=0,0 seq=40} ts=0.000000011,0 del=false klen=12 vlen=6 ih={{10 /BYTES/a}{20 /BYTES/b}{30 /BYTES/a}}
meta: "k"/0,0 -> txn={id=00000000 key=/Min pri=0.00000000 epo=0 ts=0.000000011,0 min=0,0 seq=40} ts=0.000000011,0 del=false klen=12 vlen=6 ih={{10 /BYTES/a}{20 /BYTES/b}}
data: "k"/0.000000011,0 -> /BYTES/c
data: "k"/0.000000001,0 -> /BYTES/first

Expand Down Expand Up @@ -199,6 +199,6 @@ run ok
cput t=D k=k cond=first v=c
----
>> at end:
meta: "k"/0,0 -> txn={id=00000000 key=/Min pri=0.00000000 epo=0 ts=0.000000011,0 min=0,0 seq=30} ts=0.000000011,0 del=false klen=12 vlen=6 ih={{10 /BYTES/a}{20 /BYTES/first}}
meta: "k"/0,0 -> txn={id=00000000 key=/Min pri=0.00000000 epo=0 ts=0.000000011,0 min=0,0 seq=30} ts=0.000000011,0 del=false klen=12 vlen=6 ih={{10 /BYTES/a}}
data: "k"/0.000000011,0 -> /BYTES/c
data: "k"/0.000000001,0 -> /BYTES/first
46 changes: 42 additions & 4 deletions pkg/storage/engine/testdata/mvcc_histories/put_after_rollback
View file
Original file line number Diff line number Diff line change
@@ -1,14 +1,52 @@
run error
run ok
with t=A k=k2
txn_begin ts=1
txn_step seq=10
put v=a
txn_ignore_seqs seqs=(5-15)
txn_step seq=20
put v=b
get
txn_ignore_seqs seqs=(5-25)
get
----
get: "k2" -> /BYTES/b @0.000000001,0
get: "k2" -> <no data>
>> at end:
txn: "A" meta={id=00000000 key="k2" pri=0.00000000 epo=0 ts=0.000000001,0 min=0,0 seq=20} rw=true stat=PENDING rts=0.000000001,0 wto=false max=0,0 isn=1
meta: "k2"/0,0 -> txn={id=00000000 key="k2" pri=0.00000000 epo=0 ts=0.000000001,0 min=0,0 seq=10} ts=0.000000001,0 del=false klen=12 vlen=6
data: "k2"/0.000000001,0 -> /BYTES/a
error: (*withstack.withStack:) previous intent of the transaction with the same epoch not found for "k2"/0,0 ("A" meta={id=00000000 key="k2" pri=0.00000000 epo=0 ts=0.000000001,0 min=0,0 seq=20} rw=true stat=PENDING rts=0.000000001,0 wto=false max=0,0 isn=1)
meta: "k2"/0,0 -> txn={id=00000000 key="k2" pri=0.00000000 epo=0 ts=0.000000001,0 min=0,0 seq=20} ts=0.000000001,0 del=false klen=12 vlen=6
data: "k2"/0.000000001,0 -> /BYTES/b

run ok
with t=A k=k3
txn_step seq=30
put v=a
txn_ignore_seqs seqs=(5-35)
txn_step seq=40
del
----
>> at end:
txn: "A" meta={id=00000000 key="k2" pri=0.00000000 epo=0 ts=0.000000001,0 min=0,0 seq=40} rw=true stat=PENDING rts=0.000000001,0 wto=false max=0,0 isn=1
meta: "k2"/0,0 -> txn={id=00000000 key="k2" pri=0.00000000 epo=0 ts=0.000000001,0 min=0,0 seq=20} ts=0.000000001,0 del=false klen=12 vlen=6
data: "k2"/0.000000001,0 -> /BYTES/b
meta: "k3"/0,0 -> txn={id=00000000 key="k2" pri=0.00000000 epo=0 ts=0.000000001,0 min=0,0 seq=40} ts=0.000000001,0 del=true klen=12 vlen=0
data: "k3"/0.000000001,0 -> /<empty>

run ok
with t=A k=k4
txn_step seq=50
put v=a
txn_step seq=51
cput v=b cond=a
txn_ignore_seqs seqs=(5-55)
txn_step seq=60
cput v=c
----
>> at end:
txn: "A" meta={id=00000000 key="k2" pri=0.00000000 epo=0 ts=0.000000001,0 min=0,0 seq=60} rw=true stat=PENDING rts=0.000000001,0 wto=false max=0,0 isn=1
meta: "k2"/0,0 -> txn={id=00000000 key="k2" pri=0.00000000 epo=0 ts=0.000000001,0 min=0,0 seq=20} ts=0.000000001,0 del=false klen=12 vlen=6
data: "k2"/0.000000001,0 -> /BYTES/b
meta: "k3"/0,0 -> txn={id=00000000 key="k2" pri=0.00000000 epo=0 ts=0.000000001,0 min=0,0 seq=40} ts=0.000000001,0 del=true klen=12 vlen=0
data: "k3"/0.000000001,0 -> /<empty>
meta: "k4"/0,0 -> txn={id=00000000 key="k2" pri=0.00000000 epo=0 ts=0.000000001,0 min=0,0 seq=60} ts=0.000000001,0 del=false klen=12 vlen=6
data: "k4"/0.000000001,0 -> /BYTES/c

0 comments on commit b0c0cdc

Please sign in to comment.