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

region cache: do not invalidate the valid region cache when new region cache is loaded (#1698)
Cherry pick #1698 to tidb-8.1

---

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 [ekexium](https://github.com/ekexium) 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-8.1/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":["ekexium"]} -->

Patch:
diff --git a/internal/locate/region_cache.go b/internal/locate/region_cache.go
index 44f736a..3bd2f8b 100644
--- a/internal/locate/region_cache.go
+++ b/internal/locate/region_cache.go
@@ -1570,7 +1570,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, false)
+		c.insertRegionToCache(region, false, false)
 	}
 
 	return


Test patch:
diff --git a/internal/locate/region_cache_test.go b/internal/locate/region_cache_test.go
index 8860427..46da431 100644
--- a/internal/locate/region_cache_test.go
+++ b/internal/locate/region_cache_test.go
@@ -2657,3 +2657,85 @@ 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.LocateKeyRange(s.bo, []byte("a"), []byte("e"))
+			s.Nil(err)
+		},
+		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())
+	}
+}