Repository: tikv/client-go
Pull request #1701 (2025-07-08 05:00:49)

region cache: do not invalidate the valid region cache when new region cache is loaded (#1698)
Cherry pick #1698 to tidb-7.5

---

For some functions that bypass the region cache, we should leave the exist region valid.

| Initial State | Bypass Region Cache | Finish State (old region) | Finish State (new region) |
| - | - | - | - |
| valid | yes | valid | valid |
| valid | no | valid | valid |
| invalid | yes | valid | invalid |
| invalid | no | valid | invalid |


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 assign [brson](https://github.com/brson) for approval. For more information see [the Code Review Process](https://book.prow.tidb.net/#/workflows/pr).
**Please ensure that each of them provides their approval before proceeding.**

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/tidb-7.5/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":["brson"]} -->
ti-chi-bot[bot]: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: *<a href="https://github.com/tikv/client-go/pull/1701#pullrequestreview-2997189252" title="Approved">zyguan</a>*
**Once this PR has been reviewed and has the lgtm label**, please assign [brson](https://github.com/brson) for approval. For more information see [the Code Review Process](https://book.prow.tidb.net/#/workflows/pr).
**Please ensure that each of them provides their approval before proceeding.**

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/tidb-7.5/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":["brson"]} -->
ti-chi-bot[bot]: [LGTM Timeline notifier]
---
Timeline:

- `2025-07-08 11:25:34.831143158 +0000 UTC m=+1998987.554322140`: :ballot_box_with_check: agreed by [zyguan](https://github.com/zyguan).
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/1701#pullrequestreview-3000076219" title="Approved">cfzjywxk</a>*, *<a href="https://github.com/tikv/client-go/pull/1701#pullrequestreview-2997189252" title="Approved">zyguan</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/tidb-7.5/OWNERS)~~ [cfzjywxk]

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:

- `2025-07-08 11:25:34.831143158 +0000 UTC m=+1998987.554322140`: :ballot_box_with_check: agreed by [zyguan](https://github.com/zyguan).
- `2025-07-09 06:00:39.417900907 +0000 UTC m=+2065892.141079888`: :ballot_box_with_check: agreed by [cfzjywxk](https://github.com/cfzjywxk).

Patch:
diff --git a/internal/locate/region_cache.go b/internal/locate/region_cache.go
index 126f4bf..5038c97 100644
--- a/internal/locate/region_cache.go
+++ b/internal/locate/region_cache.go
@@ -1408,7 +1408,7 @@ func (c *RegionCache) BatchLoadRegionsWithKeyRange(bo *retry.Backoffer, startKey
 	// TODO(youjiali1995): scanRegions always fetch regions from PD and these regions don't contain buckets information
 	// for less traffic, so newly inserted regions in region cache don't have buckets information. We should improve it.
 	for _, region := range regions {
-		c.insertRegionToCache(region, true)
+		c.insertRegionToCache(region, false)
 	}
 
 	return


Test patch:
diff --git a/internal/locate/region_cache_test.go b/internal/locate/region_cache_test.go
index 9c1aff6..54d485d 100644
--- a/internal/locate/region_cache_test.go
+++ b/internal/locate/region_cache_test.go
@@ -2025,3 +2025,81 @@ func (s *testRegionCacheSuite) TestIssue1401() {
 	}, 3*time.Second, time.Second)
 	s.Require().True(isStoreContainLabel(newStore1.labels, "host", "0.0.0.0:20161"))
 }
+
+func (s *testRegionCacheSuite) TestRegionCacheValidAfterLoading() {
+	s.cache.clear()
+
+	// Split regions at "a", "b", "c", ..., "j"
+	regions := make([]uint64, 0, 11)
+	region1 := s.region1
+	regions = append(regions, region1)
+	for i := 0; i < 10; i++ {
+		region2 := s.cluster.AllocID()
+		newPeers := s.cluster.AllocIDs(2)
+		s.cluster.Split(region1, region2, []byte{'a' + byte(i)}, newPeers, newPeers[0])
+		region1 = region2
+		regions = append(regions, region1)
+	}
+
+	fns := []func(){
+		func() {
+			_, err := s.cache.LocateKey(s.bo, []byte("b"))
+			s.Nil(err)
+		},
+		func() {
+			_, err := s.cache.LocateEndKey(s.bo, []byte("c"))
+			s.Nil(err)
+		},
+		func() {
+			for _, regionID := range regions {
+				_, err := s.cache.LocateRegionByID(s.bo, regionID)
+				s.Nil(err)
+			}
+		},
+		func() {
+			_, _, err := s.cache.GroupKeysByRegion(s.bo, [][]byte{[]byte("a"), []byte("b"), []byte("c")}, nil)
+			s.Nil(err)
+		},
+		func() {
+			_, err := s.cache.ListRegionIDsInKeyRange(s.bo, []byte("a"), []byte("e"))
+			s.Nil(err)
+		},
+		func() {
+			_, err := s.cache.LoadRegionsInKeyRange(s.bo, []byte("a"), []byte("e"))
+			s.Nil(err)
+		},
+		func() {
+			_, err := s.cache.BatchLoadRegionsWithKeyRange(s.bo, []byte("a"), []byte("e"), 10)
+			s.Nil(err)
+		},
+		func() {
+			_, err := s.cache.BatchLoadRegionsFromKey(s.bo, []byte("a"), 10)
+			s.Nil(err)
+		},
+	}
+
+	// Whether the region is loaded from PD(bypass the region cache) or from the region cache,
+	// the existing valid region should not be invalidated.
+	for _, fn := range fns {
+		loc, err := s.cache.LocateKey(s.bo, []byte("b"))
+		s.Nil(err)
+		region := s.cache.GetCachedRegionWithRLock(loc.Region)
+		fn()
+		s.True(region.isValid())
+	}
+
+	// If the region is invalidated already, it should be reloaded from PD and inserted into region cache anyway.
+	for _, fn := range fns {
+		loc, err := s.cache.LocateKey(s.bo, []byte("b"))
+		s.Nil(err)
+		region := s.cache.GetCachedRegionWithRLock(loc.Region)
+		region.invalidate(Other)
+		s.False(region.isValid())
+		fn()
+		s.False(region.isValid())
+		newLoc := s.cache.TryLocateKey([]byte("b"))
+		s.NotNil(newLoc)
+		region = s.cache.GetCachedRegionWithRLock(newLoc.Region)
+		s.True(region.isValid())
+	}
+}