From 870885c3570c691f46da1578236b5f6d35baa4be Mon Sep 17 00:00:00 2001
From: ekexium <eke@fastmail.com>
Date: Tue, 4 Mar 2025 20:13:47 +0800
Subject: [PATCH] (tidb-7.1) Validate ts only for stale read (#1595)

ref pingcap/tidb#59402

Signed-off-by: ekexium <eke@fastmail.com>
---
 oracle/oracles/local.go   | 20 +-------------------
 oracle/oracles/mock.go    | 18 ------------------
 oracle/oracles/pd.go      | 15 +++++++++++++++
 oracle/oracles/pd_test.go |  5 ++---
 4 files changed, 18 insertions(+), 40 deletions(-)

diff --git a/oracle/oracles/local.go b/oracle/oracles/local.go
index e916286ac3..8da336f5f2 100644
--- a/oracle/oracles/local.go
+++ b/oracle/oracles/local.go
@@ -36,11 +36,9 @@ package oracles
 
 import (
 	"context"
-	"math"
 	"sync"
 	"time"
 
-	"github.com/pingcap/errors"
 	"github.com/tikv/client-go/v2/oracle"
 )
 
@@ -138,22 +136,6 @@ func (l *localOracle) GetExternalTimestamp(ctx context.Context) (uint64, error)
 }
 
 func (l *localOracle) ValidateReadTS(ctx context.Context, readTS uint64, isStaleRead bool, opt *oracle.Option) error {
-	if readTS == math.MaxUint64 {
-		if isStaleRead {
-			return oracle.ErrLatestStaleRead{}
-		}
-		return nil
-	}
-
-	currentTS, err := l.GetTimestamp(ctx, opt)
-	if err != nil {
-		return errors.Errorf("fail to validate read timestamp: %v", err)
-	}
-	if currentTS < readTS {
-		return oracle.ErrFutureTSRead{
-			ReadTS:    readTS,
-			CurrentTS: currentTS,
-		}
-	}
+	// local oracle is not supposed to be used
 	return nil
 }
diff --git a/oracle/oracles/mock.go b/oracle/oracles/mock.go
index da8874d5c8..662766cec3 100644
--- a/oracle/oracles/mock.go
+++ b/oracle/oracles/mock.go
@@ -36,7 +36,6 @@ package oracles
 
 import (
 	"context"
-	"math"
 	"sync"
 	"time"
 
@@ -128,23 +127,6 @@ func (o *MockOracle) SetLowResolutionTimestampUpdateInterval(time.Duration) erro
 }
 
 func (o *MockOracle) ValidateReadTS(ctx context.Context, readTS uint64, isStaleRead bool, opt *oracle.Option) error {
-	if readTS == math.MaxUint64 {
-		if isStaleRead {
-			return oracle.ErrLatestStaleRead{}
-		}
-		return nil
-	}
-
-	currentTS, err := o.GetTimestamp(ctx, opt)
-	if err != nil {
-		return errors.Errorf("fail to validate read timestamp: %v", err)
-	}
-	if currentTS < readTS {
-		return oracle.ErrFutureTSRead{
-			ReadTS:    readTS,
-			CurrentTS: currentTS,
-		}
-	}
 	return nil
 }
 
diff --git a/oracle/oracles/pd.go b/oracle/oracles/pd.go
index 805d1b5c7a..a3f906bab1 100644
--- a/oracle/oracles/pd.go
+++ b/oracle/oracles/pd.go
@@ -620,7 +620,22 @@ func (o *pdOracle) getCurrentTSForValidation(ctx context.Context, opt *oracle.Op
 	}
 }
 
+// ValidateReadTSForTidbSnapshot is a flag in context, indicating whether the read ts is for tidb_snapshot.
+// This is a special approach for release branches to minimize code changes to reduce risks.
+type ValidateReadTSForTidbSnapshot struct{}
+
 func (o *pdOracle) ValidateReadTS(ctx context.Context, readTS uint64, isStaleRead bool, opt *oracle.Option) (errRet error) {
+	// For a mistake we've seen
+	if readTS >= math.MaxInt64 && readTS < math.MaxUint64 {
+		return errors.Errorf("MaxInt64 <= readTS < MaxUint64, readTS=%v", readTS)
+	}
+
+	// For release branches, only check stale reads and reads using `tidb_snapshot`
+	forTidbSnapshot := ctx.Value(ValidateReadTSForTidbSnapshot{}) != nil
+	if !forTidbSnapshot && !isStaleRead {
+		return nil
+	}
+
 	if readTS == math.MaxUint64 {
 		if isStaleRead {
 			return oracle.ErrLatestStaleRead{}
diff --git a/oracle/oracles/pd_test.go b/oracle/oracles/pd_test.go
index 25345f3b85..4afd587789 100644
--- a/oracle/oracles/pd_test.go
+++ b/oracle/oracles/pd_test.go
@@ -284,7 +284,6 @@ func TestValidateReadTS(t *testing.T) {
 	}
 
 	testImpl(true)
-	testImpl(false)
 }
 
 type MockPDClientWithPause struct {
@@ -436,9 +435,9 @@ func TestValidateReadTSForNormalReadDoNotAffectUpdateInterval(t *testing.T) {
 	assert.NoError(t, err)
 	mustNoNotify()
 
-	// It loads `ts + 1` from the mock PD, and the check cannot pass.
+	// It loads `ts + 1` from the mock PD, and the check is skipped.
 	err = o.ValidateReadTS(ctx, ts+2, false, opt)
-	assert.Error(t, err)
+	assert.NoError(t, err)
 	mustNoNotify()
 
 	// Do the check again. It loads `ts + 2` from the mock PD, and the check passes.