From cc8b9491145b0e623277f2ffd0b64c403bfee438 Mon Sep 17 00:00:00 2001
From: ekexium <eke@fastmail.com>
Date: Tue, 4 Mar 2025 20:15:40 +0800
Subject: [PATCH] (tidb-8.5) Validate ts only for stale read (#1597)

ref pingcap/tidb#59402

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

diff --git a/internal/locate/region_request_test.go b/internal/locate/region_request_test.go
index ca03c660bf..c55483c4db 100644
--- a/internal/locate/region_request_test.go
+++ b/internal/locate/region_request_test.go
@@ -961,7 +961,8 @@ func (s *testRegionRequestToSingleStoreSuite) TestRegionRequestValidateReadTS()
 	testImpl(getTS, true, nil)
 	testImpl(func() uint64 { return addTS(getTS(), -time.Minute) }, false, nil)
 	testImpl(func() uint64 { return addTS(getTS(), -time.Minute) }, true, nil)
-	testImpl(func() uint64 { return addTS(getTS(), +time.Minute) }, false, oracle.ErrFutureTSRead{})
+	// check is skipped for normal read
+	testImpl(func() uint64 { return addTS(getTS(), +time.Minute) }, false, nil)
 	testImpl(func() uint64 { return addTS(getTS(), +time.Minute) }, true, oracle.ErrFutureTSRead{})
 	testImpl(func() uint64 { return math.MaxUint64 }, false, nil)
 	testImpl(func() uint64 { return math.MaxUint64 }, true, oracle.ErrLatestStaleRead{})
diff --git a/oracle/oracles/local.go b/oracle/oracles/local.go
index a980bbddbd..e5aa21f3c1 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"
 )
 
@@ -152,22 +150,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 757ce3158d..5ed8d75fe6 100644
--- a/oracle/oracles/mock.go
+++ b/oracle/oracles/mock.go
@@ -36,7 +36,6 @@ package oracles
 
 import (
 	"context"
-	"math"
 	"sync"
 	"time"
 
@@ -139,23 +138,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 75e297169a..b23d8ca2e2 100644
--- a/oracle/oracles/pd.go
+++ b/oracle/oracles/pd.go
@@ -673,7 +673,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 01c67c5e46..4520f9f854 100644
--- a/oracle/oracles/pd_test.go
+++ b/oracle/oracles/pd_test.go
@@ -389,7 +389,6 @@ func TestValidateReadTS(t *testing.T) {
 	}
 
 	testImpl(true)
-	testImpl(false)
 }
 
 type MockPDClientWithPause struct {
@@ -541,9 +540,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.