kv: don't consult ReadTimestamp in Transaction.LastActive

Informs #101938.

Without the synthetic timestamp bit, we don't know for sure whether the
transaction's ReadTimestamp is a ClockTimestamp or not. To avoid
comparing a future-time MVCC timestamp against a clock timestamp for
purposes of detecting transaction liveness, we stop consulting the
ReadTimestamp. This was always an unproven optimization anyway, so it's
safe to remove.

Release note: None

pkg/kv/kvclient/kvcoord/txn_interceptor_heartbeater_test.go

@@ -18,6 +18,7 @@ import (
 
 	"github.com/cockroachdb/cockroach/pkg/base"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvpb"
+	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/concurrency/isolation"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/concurrency/lock"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/txnwait"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
@@ -268,7 +269,7 @@ func TestTxnHeartbeaterLoopStartsBeforeExpiry(t *testing.T) {
 		},
 		{
 			// First locking request happens at expiration. Heartbeat immediately.
-			lockingRequestDelay: 5 * time.Second,
+			lockingRequestDelay: 5*time.Second + 1*time.Nanosecond,
 			consideredExpired:   true,
 			loopStarts:          StartImmediately,
 		},
@@ -281,11 +282,10 @@ func TestTxnHeartbeaterLoopStartsBeforeExpiry(t *testing.T) {
 	} {
 		t.Run(fmt.Sprintf("delay=%s", test.lockingRequestDelay), func(t *testing.T) {
 			ctx := context.Background()
-			txn := makeTxnProto()
-
 			manualTime := timeutil.NewManualTime(timeutil.Unix(0, 123))
 			clock := hlc.NewClockForTesting(manualTime)
-			txn.MinTimestamp, txn.WriteTimestamp = clock.Now(), clock.Now()
+			txn := roachpb.MakeTransaction("test", []byte("key"), isolation.Serializable, 0, clock.Now(),
+				0 /* maxOffsetNs */, 0 /* coordinatorNodeID */, 0, false /* omitInRangefeeds */)
 
 			// We attempt to simulate a transaction that heartbeats every 1s, however
 			// it is important to note that a transaction is considered expired when it

pkg/kv/kvserver/intentresolver/intent_resolver_test.go

@@ -76,17 +76,17 @@ func TestCleanupTxnIntentsOnGCAsync(t *testing.T) {
 	txn0 := newTransaction("txn0", key, 1, clock)
 	// Txn1 is in the pending state but is expired.
 	txn1 := newTransaction("txn1", key, 1, clock)
-	txn1.ReadTimestamp.WallTime -= int64(100 * time.Second)
-	txn1.LastHeartbeat = txn1.ReadTimestamp
+	txn1.MinTimestamp.WallTime -= int64(100 * time.Second)
+	txn1.LastHeartbeat = txn1.MinTimestamp
 	// Txn2 is in the staging state and is not old enough to have expired so the
 	// code ought to send nothing.
 	txn2 := newTransaction("txn2", key, 1, clock)
 	txn2.Status = roachpb.STAGING
 	// Txn3 is in the staging state but is expired.
 	txn3 := newTransaction("txn3", key, 1, clock)
 	txn3.Status = roachpb.STAGING
-	txn3.ReadTimestamp.WallTime -= int64(100 * time.Second)
-	txn3.LastHeartbeat = txn3.ReadTimestamp
+	txn3.MinTimestamp.WallTime -= int64(100 * time.Second)
+	txn3.LastHeartbeat = txn3.MinTimestamp
 	// Txn4 is in the committed state.
 	txn4 := newTransaction("txn4", key, 1, clock)
 	txn4.Status = roachpb.COMMITTED

pkg/roachpb/data.go

@@ -997,13 +997,10 @@ func MakeTransaction(
 }
 
 // LastActive returns the last timestamp at which client activity definitely
-// occurred, i.e. the maximum of ReadTimestamp and LastHeartbeat.
+// occurred, i.e. the maximum of MinTimestamp and LastHeartbeat.
 func (t Transaction) LastActive() hlc.Timestamp {
-	ts := t.LastHeartbeat
-	// TODO(nvanbenschoten): remove this when we remove synthetic timestamps.
-	if !t.ReadTimestamp.Synthetic {
-		ts.Forward(t.ReadTimestamp)
-	}
+	ts := t.MinTimestamp
+	ts.Forward(t.LastHeartbeat)
 	return ts
 }