Repository: tikv/client-go
Pull request #1480 (2024-10-22 12:34:58)

region cache: retry scan or batch scan regions when returned region has no leader
Ref: https://github.com/pingcap/tidb/issues/56757

When there is no leader information for all the returned regions, retry scan or batch scan region from PD but not return errors to the upper layer.  

ti-chi-bot[bot]: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by:
**Once this PR has been reviewed and has the lgtm label**, please ask for approval from [cfzjywxk](https://github.com/cfzjywxk), **ensuring that each of them provides their approval before proceeding**. For more information see [the Code Review Process](https://book.prow.tidb.net/#/workflows/pr).

The full list of commands accepted by this bot can be found [here](https://prow.tidb.net/command-help?repo=tikv%2Fclient-go).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/tikv/client-go/blob/master/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["cfzjywxk"]} -->
ti-chi-bot[bot]: [LGTM Timeline notifier]
---
Timeline:

- `2024-10-22 13:04:01.338091908 +0000 UTC m=+356242.034882516`: :ballot_box_with_check: agreed by [crazycs520](https://github.com/crazycs520).
ti-chi-bot[bot]: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/tikv/client-go/pull/1480#pullrequestreview-2385142261" title="Approved">crazycs520</a>*, *<a href="https://github.com/tikv/client-go/pull/1480#pullrequestreview-2386774547" title="Approved">you06</a>*

The full list of commands accepted by this bot can be found [here](https://prow.tidb.net/command-help?repo=tikv%2Fclient-go).

The pull request process is described [here](https://book.prow.tidb.net/#/workflows/pr)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/tikv/client-go/blob/master/OWNERS)~~ [crazycs520,you06]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
ti-chi-bot[bot]: [LGTM Timeline notifier]
---
Timeline:

- `2024-10-22 13:04:01.338091908 +0000 UTC m=+356242.034882516`: :ballot_box_with_check: agreed by [crazycs520](https://github.com/crazycs520).
- `2024-10-23 02:26:53.925594699 +0000 UTC m=+404414.622385323`: :ballot_box_with_check: agreed by [you06](https://github.com/you06).

Patch:
diff --git a/internal/locate/region_cache.go b/internal/locate/region_cache.go
index 01dfa38..c70e137 100644
--- a/internal/locate/region_cache.go
+++ b/internal/locate/region_cache.go
@@ -2223,7 +2223,18 @@ func (c *RegionCache) scanRegions(bo *retry.Backoffer, startKey, endKey []byte,
 			backoffErr = errors.Errorf("PD returned regions have gaps, limit: %d", limit)
 			continue
 		}
-		return c.handleRegionInfos(bo, regionsInfo, true)
+		validRegions, err := c.handleRegionInfos(bo, regionsInfo, true)
+		if err != nil {
+			return nil, err
+		}
+		// If the region information is loaded from the local disk and the current leader has not
+		// yet reported a heartbeat to PD, the region information scanned at this time will not include the leader.
+		// Retry if there is no valid regions with leaders.
+		if len(validRegions) == 0 {
+			backoffErr = errors.Errorf("All returned regions have no leaders, limit: %d", limit)
+			continue
+		}
+		return validRegions, nil
 	}
 }
 
@@ -2290,7 +2301,18 @@ func (c *RegionCache) batchScanRegions(bo *retry.Backoffer, keyRanges []pd.KeyRa
 			)
 			continue
 		}
-		return c.handleRegionInfos(bo, regionsInfo, opt.needRegionHasLeaderPeer)
+		validRegions, err := c.handleRegionInfos(bo, regionsInfo, opt.needRegionHasLeaderPeer)
+		if err != nil {
+			return nil, err
+		}
+		// If the region information is loaded from the local disk and the current leader has not
+		// yet reported a heartbeat to PD, the region information scanned at this time will not include the leader.
+		// Retry if there is no valid regions with leaders.
+		if len(validRegions) == 0 {
+			backoffErr = errors.Errorf("All returned regions have no leaders, limit: %d", limit)
+			continue
+		}
+		return validRegions, nil
 	}
 }
 
@@ -2397,7 +2419,7 @@ func (c *RegionCache) handleRegionInfos(bo *retry.Backoffer, regionsInfo []*pd.R
 		regions = append(regions, region)
 	}
 	if len(regions) == 0 {
-		return nil, errors.New("receive Regions with no peer")
+		return nil, nil
 	}
 	if len(regions) < len(regionsInfo) {
 		logutil.Logger(context.Background()).Debug(


Test patch:
diff --git a/internal/locate/region_cache_test.go b/internal/locate/region_cache_test.go
index 8e3bf1a..110a321 100644
--- a/internal/locate/region_cache_test.go
+++ b/internal/locate/region_cache_test.go
@@ -66,7 +66,8 @@ import (
 
 type inspectedPDClient struct {
 	pd.Client
-	getRegion func(ctx context.Context, cli pd.Client, key []byte, opts ...pd.GetRegionOption) (*pd.Region, error)
+	getRegion        func(ctx context.Context, cli pd.Client, key []byte, opts ...pd.GetRegionOption) (*pd.Region, error)
+	batchScanRegions func(ctx context.Context, keyRanges []pd.KeyRange, limit int, opts ...pd.GetRegionOption) ([]*pd.Region, error)
 }
 
 func (c *inspectedPDClient) GetRegion(ctx context.Context, key []byte, opts ...pd.GetRegionOption) (*pd.Region, error) {
@@ -76,6 +77,13 @@ func (c *inspectedPDClient) GetRegion(ctx context.Context, key []byte, opts ...p
 	return c.Client.GetRegion(ctx, key, opts...)
 }
 
+func (c *inspectedPDClient) BatchScanRegions(ctx context.Context, keyRanges []pd.KeyRange, limit int, opts ...pd.GetRegionOption) ([]*pd.Region, error) {
+	if c.batchScanRegions != nil {
+		return c.batchScanRegions(ctx, keyRanges, limit, opts...)
+	}
+	return c.Client.BatchScanRegions(ctx, keyRanges, limit, opts...)
+}
+
 func TestBackgroundRunner(t *testing.T) {
 	t.Run("ShutdownWait", func(t *testing.T) {
 		dur := 100 * time.Millisecond
@@ -459,6 +467,40 @@ func (s *testRegionCacheSuite) TestResolveStateTransition() {
 	s.cluster.AddStore(storeMeta.GetId(), storeMeta.GetAddress(), storeMeta.GetLabels()...)
 }
 
+func (s *testRegionCacheSuite) TestReturnRegionWithNoLeader() {
+	region := s.getRegion([]byte("x"))
+	NoLeaderRegion := &pd.Region{
+		Meta:   region.meta,
+		Leader: nil,
+	}
+
+	originalBatchScanRegions := s.cache.pdClient.BatchScanRegions
+
+	batchScanCnt := 0
+	s.cache.pdClient = &inspectedPDClient{
+		Client: s.cache.pdClient,
+		batchScanRegions: func(ctx context.Context, keyRanges []pd.KeyRange, limit int, opts ...pd.GetRegionOption) ([]*pd.Region, error) {
+			if batchScanCnt == 0 {
+				batchScanCnt++
+				return []*pd.Region{NoLeaderRegion}, nil
+			} else {
+				return originalBatchScanRegions(ctx, keyRanges, limit, opts...)
+			}
+		},
+	}
+
+	bo := retry.NewBackofferWithVars(context.Background(), 1000, nil)
+	returnedRegions, err := s.cache.scanRegions(bo, nil, nil, 100)
+	s.Nil(err)
+	s.Equal(len(returnedRegions), 1)
+	s.Equal(returnedRegions[0].meta.GetId(), region.GetID())
+
+	returnedRegions, err = s.cache.batchScanRegions(bo, []pd.KeyRange{{StartKey: nil, EndKey: nil}}, 100, WithNeedRegionHasLeaderPeer())
+	s.Nil(err)
+	s.Equal(len(returnedRegions), 1)
+	s.Equal(returnedRegions[0].meta.GetId(), region.GetID())
+}
+
 func (s *testRegionCacheSuite) TestNeedExpireRegionAfterTTL() {
 	s.onClosed = func() { SetRegionCacheTTLWithJitter(600, 60) }
 	SetRegionCacheTTLWithJitter(2, 0)