reverts PR #988 to fix transaction is undetermined cause by region invalid error (#1298)

* Revert "dco (#988)"

This reverts commit 4c2ae43454.

Signed-off-by: crazycs520 <crazycs520@gmail.com>

* add comment

Signed-off-by: crazycs520 <crazycs520@gmail.com>

---------

Signed-off-by: crazycs520 <crazycs520@gmail.com>

internal/locate/region_request.go

@@ -1015,14 +1015,9 @@ func newReplicaSelector(
 	regionCache *RegionCache, regionID RegionVerID, req *tikvrpc.Request, opts ...StoreSelectorOption,
 ) (*replicaSelector, error) {
 	cachedRegion := regionCache.GetCachedRegionWithRLock(regionID)
-	if cachedRegion == nil {
-		return nil, errors.New("cached region not found")
-	} else if cachedRegion.checkSyncFlags(needReloadOnAccess) {
-		return nil, errors.New("cached region need reload")
-	} else if !cachedRegion.checkRegionCacheTTL(time.Now().Unix()) {
-		return nil, errors.New("cached region ttl expired")
+	if cachedRegion == nil || !cachedRegion.isValid() {
+		return nil, nil
 	}
-
 	replicas := buildTiKVReplicas(cachedRegion)
 	regionStore := cachedRegion.getStore()
 	option := storeSelectorOp{}
@@ -1066,6 +1061,10 @@ func newReplicaSelector(
 	}, nil
 }
 
+func (s *replicaSelector) isValid() bool {
+	return s != nil
+}
+
 func buildTiKVReplicas(region *Region) []*replica {
 	regionStore := region.getStore()
 	replicas := make([]*replica, 0, regionStore.accessStoreNum(tiKVOnly))
@@ -1423,10 +1422,9 @@ func (s *RegionRequestSender) getRPCContext(
 	switch et {
 	case tikvrpc.TiKV:
 		if s.replicaSelector == nil {
-			selector, err := NewReplicaSelector(s.regionCache, regionID, req, opts...)
-			if err != nil {
-				s.rpcError = err
-				return nil, nil
+			selector, err := NewReplicaSelector(s.regionCache, regionID, req, opts...) //nolint:staticcheck // ignore SA4023, never returns a nil interface value
+			if selector == nil || !selector.isValid() || err != nil {                  //nolint:staticcheck // ignore SA4023, never returns a nil interface value
+				return nil, err
 			}
 			s.replicaSelector = selector
 		}

internal/locate/region_request_test.go

@@ -826,5 +826,5 @@ func (s *testRegionRequestToSingleStoreSuite) TestRegionRequestSenderString() {
 	// invalid region cache before sending request.
 	s.cache.InvalidateCachedRegion(loc.Region)
 	sender.SendReqCtx(s.bo, tikvrpc.NewRequest(tikvrpc.CmdGet, &kvrpcpb.GetRequest{}), loc.Region, time.Second, tikvrpc.TiKV)
-	s.Equal("{rpcError:cached region invalid, replicaSelector: <nil>}", sender.String())
+	s.Equal("{rpcError:<nil>, replicaSelector: <nil>}", sender.String())
 }

internal/locate/replica_selector.go

@@ -44,9 +44,12 @@ type ReplicaSelector interface {
 	onDataIsNotReady()
 	onServerIsBusy(bo *retry.Backoffer, ctx *RPCContext, req *tikvrpc.Request, serverIsBusy *errorpb.ServerIsBusy) (shouldRetry bool, err error)
 	onReadReqConfigurableTimeout(req *tikvrpc.Request) bool
+	isValid() bool
 }
 
 // NewReplicaSelector returns a new ReplicaSelector.
+//
+//nolint:staticcheck // ignore SA4023, never returns a nil interface value
 func NewReplicaSelector(
 	regionCache *RegionCache, regionID RegionVerID, req *tikvrpc.Request, opts ...StoreSelectorOption,
 ) (ReplicaSelector, error) {
@@ -72,7 +75,7 @@ func newReplicaSelectorV2(
 ) (*replicaSelectorV2, error) {
 	cachedRegion := regionCache.GetCachedRegionWithRLock(regionID)
 	if cachedRegion == nil || !cachedRegion.isValid() {
-		return nil, errors.New("cached region invalid")
+		return nil, nil
 	}
 	replicas := buildTiKVReplicas(cachedRegion)
 	option := storeSelectorOp{}
@@ -98,6 +101,10 @@ func newReplicaSelectorV2(
 	}, nil
 }
 
+func (s *replicaSelectorV2) isValid() bool {
+	return s != nil
+}
+
 func (s *replicaSelectorV2) next(bo *retry.Backoffer, req *tikvrpc.Request) (rpcCtx *RPCContext, err error) {
 	if !s.region.isValid() {
 		metrics.TiKVReplicaSelectorFailureCounter.WithLabelValues("invalid").Inc()

internal/locate/replica_selector_test.go

@@ -119,10 +119,15 @@ func TestReplicaSelectorBasic(t *testing.T) {
 	s.NotNil(rc)
 	rc.invalidate(Other)
 	selector, err := newReplicaSelectorV2(s.cache, rc.VerID(), req)
-	s.NotNil(err)
-	s.Equal("cached region invalid", err.Error())
+	s.Nil(err)
 	s.Nil(selector)
 	s.Equal("", selector.String())
+	selector2, err := NewReplicaSelector(s.cache, rc.VerID(), req)
+	s.Nil(err)
+	s.Nil(selector2)
+	s.False(selector2 == nil) // since never returns a nil interface value
+	s.False(selector2.isValid())
+	s.Equal("", selector2.String())
 
 	rc = s.getRegion()
 	selector, err = newReplicaSelectorV2(s.cache, rc.VerID(), req)