From 26f2704f6669f13c6c03664acae1c3e44cebbbbf Mon Sep 17 00:00:00 2001 From: buffer <1045931706@qq.com> Date: Thu, 28 Sep 2023 17:42:21 +0800 Subject: [PATCH] This is an automated cherry-pick of #7143 close tikv/pd#4399 Signed-off-by: ti-chi-bot --- server/core/region.go | 24 ++++++++++++++++++++++-- server/core/region_test.go | 13 +++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/server/core/region.go b/server/core/region.go index 81b3e1bacaa..90b5d904ddc 100644 --- a/server/core/region.go +++ b/server/core/region.go @@ -22,11 +22,13 @@ import ( "sort" "strings" "sync/atomic" + "time" "unsafe" "github.com/docker/go-units" "github.com/gogo/protobuf/proto" "github.com/pingcap/errors" + "github.com/pingcap/failpoint" "github.com/pingcap/kvproto/pkg/metapb" "github.com/pingcap/kvproto/pkg/pdpb" "github.com/pingcap/kvproto/pkg/replication_modepb" @@ -885,7 +887,16 @@ func (r *RegionsInfo) setRegionLocked(region *RegionInfo) (*RegionInfo, []*Regio } // UpdateSubTree updates the subtree. +<<<<<<< HEAD:server/core/region.go func (r *RegionsInfo) UpdateSubTree(region, origin *RegionInfo, toRemove []*RegionInfo, rangeChanged bool) { +======= +func (r *RegionsInfo) UpdateSubTree(region, origin *RegionInfo, overlaps []*RegionInfo, rangeChanged bool) { + failpoint.Inject("UpdateSubTree", func() { + if origin == nil { + time.Sleep(time.Second) + } + }) +>>>>>>> 54219d649 (region: fix the potential panic . (#7143)):pkg/core/region.go r.st.Lock() defer r.st.Unlock() if origin != nil { @@ -894,8 +905,17 @@ func (r *RegionsInfo) UpdateSubTree(region, origin *RegionInfo, toRemove []*Regi // TODO: Improve performance by deleting only the different peers. r.removeRegionFromSubTreeLocked(origin) } else { - r.updateSubTreeStat(origin, region) - r.subRegions[region.GetID()].RegionInfo = region + // The region tree and the subtree update is not atomic and the region tree is updated first. + // If there are two thread needs to update region tree, + // t1: thread-A update region tree + // t2: thread-B: update region tree again + // t3: thread-B: update subtree + // t4: thread-A: update region subtree + // to keep region tree consistent with subtree, we need to drop this update. + if tree, ok := r.subRegions[region.GetID()]; ok { + r.updateSubTreeStat(origin, region) + tree.RegionInfo = region + } return } } diff --git a/server/core/region_test.go b/server/core/region_test.go index e6535b6f423..505df4394fc 100644 --- a/server/core/region_test.go +++ b/server/core/region_test.go @@ -22,6 +22,7 @@ import ( "testing" "time" + "github.com/pingcap/failpoint" "github.com/pingcap/kvproto/pkg/metapb" "github.com/pingcap/kvproto/pkg/pdpb" "github.com/stretchr/testify/require" @@ -451,6 +452,18 @@ func TestRegionKey(t *testing.T) { } } +func TestSetRegionConcurrence(t *testing.T) { + re := require.New(t) + re.NoError(failpoint.Enable("github.com/tikv/pd/pkg/core/UpdateSubTree", `return()`)) + regions := NewRegionsInfo() + region := NewTestRegionInfo(1, 1, []byte("a"), []byte("b")) + go func() { + regions.AtomicCheckAndPutRegion(region) + }() + regions.AtomicCheckAndPutRegion(region) + re.NoError(failpoint.Disable("github.com/tikv/pd/pkg/core/UpdateSubTree")) +} + func TestSetRegion(t *testing.T) { re := require.New(t) regions := NewRegionsInfo()