Merge cockroachdb#126417

126417: kvnemesis: skip validation for rolled-back locking reads in weak-isolation transactions r=arulajmani a=miraradeva

Currently, for weak-isolation-level transactions, kvnemesis does validation only for locking reads; such transactions ensure the locks are held until the transactions commit. However, this is not the case if a locking read is rolled back by a savepoint. In this case, the lock is not released eagerly, but it's also not guaranteed to be held; the lock may be released if the transaction is pushed. Once the read is no longer considered locking, weaker isolations levels do not require that its valid times intersect with the rest of the transaction's operations.

In this patch, we adjust the kvnemesis validation to skip such rolled-back reads.

Fixes: cockroachdb#125545

Release note: None

Co-authored-by: Mira Radeva <[email protected]>

pkg/kv/kvnemesis/validator.go

@@ -1115,8 +1115,17 @@ func (v *validator) checkAtomicCommitted(
 					panic(err)
 				}
 			}
+		case *observedRead:
+			// If v.observationFilter == observeLocking, this must be a locking read
+			// from a weak-isolation transaction. If this locking read is rolled back
+			// by a savepoint, there is no guarantee that the lock will be present
+			// after the savepoint rollback. Since the read is not part of a
+			// serializatble transaction and is no longer locking, it's valid at all
+			// times. Alternatively, we can remove the read from txnObservations.
+			if rollbackSp != nil && v.observationFilter == observeLocking {
+				o.ValidTimes = disjointTimeSpans{{Start: hlc.MinTimestamp, End: hlc.MaxTimestamp}}
+			}
 		case *observedSavepoint:
-
 			switch o.Type {
 			case create:
 				// Set rollbackSp to nil if this savepoint create matches the rolled
@@ -1188,7 +1197,11 @@ func (v *validator) checkAtomicCommitted(
 				}
 			}
 		case *observedRead:
-			o.ValidTimes = validReadTimes(batch, o.Key, o.Value.RawBytes, o.SkipLocked /* missingKeyValid */)
+			// For rolled back locking reads from weak-isolation transactions, the
+			// ValidTimes are already set above.
+			if len(o.ValidTimes) == 0 {
+				o.ValidTimes = validReadTimes(batch, o.Key, o.Value.RawBytes, o.SkipLocked /* missingKeyValid */)
+			}
 		case *observedScan:
 			// All kvs should be within scan boundary.
 			for _, kv := range o.KVs {