Validate ts only for stale read (#1592)

ref pingcap/tidb#59402 Signed-off-by: ekexium <eke@fastmail.com>
tikv · Feb 25, 2025 · 8f4f769 · 8f4f769
1 parent 7be1c54
commit 8f4f769
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 41 deletions.
diff --git a/oracle/oracles/local.go b/oracle/oracles/local.go
@@ -36,13 +36,10 @@ package oracles
 
 import (
 	"context"
-	"math"
 	"sync"
 	"time"
 
-	"github.com/tikv/client-go/v2/internal/logutil"
 	"github.com/tikv/client-go/v2/oracle"
-	"go.uber.org/zap"
 )
 
 var _ oracle.Oracle = &localOracle{}
@@ -139,20 +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 {
-			logutil.Logger(ctx).Error("stale read use MaxUint64 as readTS")
-		}
-		return nil
-	}
-
-	currentTS, err := l.GetTimestamp(ctx, opt)
-	if err != nil {
-		logutil.Logger(ctx).Error("fail to get timestamp when validating", zap.Error(err))
-		return nil
-	}
-	if currentTS < readTS {
-		logutil.Logger(ctx).Error("ts validation failed", zap.Uint64("readTS", readTS), zap.Uint64("currentTS", currentTS))
-	}
+	// local oracle is not supposed to be used
 	return nil
 }
diff --git a/oracle/oracles/mock.go b/oracle/oracles/mock.go
@@ -36,14 +36,11 @@ package oracles
 
 import (
 	"context"
-	"math"
 	"sync"
 	"time"
 
 	"github.com/pkg/errors"
-	"github.com/tikv/client-go/v2/internal/logutil"
 	"github.com/tikv/client-go/v2/oracle"
-	"go.uber.org/zap"
 )
 
 var errStopped = errors.New("stopped")
@@ -127,21 +124,6 @@ func (o *MockOracle) GetLowResolutionTimestampAsync(ctx context.Context, opt *or
 
 // ValidateReadTS implements ReadTSValidator interface.
 func (o *MockOracle) ValidateReadTS(ctx context.Context, readTS uint64, isStaleRead bool, opt *oracle.Option) error {
-	if readTS == math.MaxUint64 {
-		if isStaleRead {
-			logutil.Logger(ctx).Error("stale read use MaxUint64 as readTS")
-		}
-		return nil
-	}
-
-	currentTS, err := o.GetTimestamp(ctx, opt)
-	if err != nil {
-		logutil.Logger(ctx).Error("fail to get timestamp when validating", zap.Error(err))
-		return nil
-	}
-	if currentTS < readTS {
-		logutil.Logger(ctx).Error("ts validation failed", zap.Uint64("readTS", readTS), zap.Uint64("currentTS", currentTS))
-	}
 	return nil
 }
 

diff --git a/oracle/oracles/pd.go b/oracle/oracles/pd.go
@@ -646,10 +646,25 @@ 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 {
-			logutil.Logger(ctx).Error("stale read use MaxUint64 as readTS")
+			return oracle.ErrLatestStaleRead{}
 		}
 		return nil
 	}
@@ -661,15 +676,16 @@ func (o *pdOracle) ValidateReadTS(ctx context.Context, readTS uint64, isStaleRea
 	if !exists || readTS > latestTSInfo.tso {
 		currentTS, err := o.getCurrentTSForValidation(ctx, opt)
 		if err != nil {
-			logutil.Logger(ctx).Error("fail to get timestamp when validating", zap.Error(err))
-			return nil
+			return errors.Errorf("fail to validate read timestamp: %v", err)
 		}
 		if isStaleRead {
 			o.adjustUpdateLowResolutionTSIntervalWithRequestedStaleness(readTS, currentTS, time.Now())
 		}
 		if readTS > currentTS {
-			logutil.Logger(ctx).Error("ts validation failed", zap.Uint64("readTS", readTS), zap.Uint64("currentTS", currentTS))
-			return nil
+			return oracle.ErrFutureTSRead{
+				ReadTS:    readTS,
+				CurrentTS: currentTS,
+			}
 		}
 	} else if isStaleRead {
 		estimatedCurrentTS, err := o.getStaleTimestampWithLastTS(latestTSInfo, 0)