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 1/2] 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() From 0ec87260ac8a448524cbd94494eca77d8a378952 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=AB=A5=E5=89=91?= <1045931706@qq.com> Date: Sun, 26 Jan 2025 08:19:44 +0800 Subject: [PATCH 2/2] resolve conflict MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: 童剑 <1045931706@qq.com> --- server/core/region.go | 4 ---- server/core/region_test.go | 2 +- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/server/core/region.go b/server/core/region.go index 90b5d904ddc..653e61d3f14 100644 --- a/server/core/region.go +++ b/server/core/region.go @@ -887,16 +887,12 @@ 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 { diff --git a/server/core/region_test.go b/server/core/region_test.go index 505df4394fc..ed1859bb8b6 100644 --- a/server/core/region_test.go +++ b/server/core/region_test.go @@ -456,7 +456,7 @@ 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")) + region := NewTestRegionInfo([]byte("a"), []byte("b")) go func() { regions.AtomicCheckAndPutRegion(region) }()