From 4c7584e12043b4cc8eea06e30275f28128d2858a Mon Sep 17 00:00:00 2001 From: bufferflies <1045931706@qq.com> Date: Fri, 22 Sep 2023 21:17:01 +0800 Subject: [PATCH 1/2] panic Signed-off-by: bufferflies <1045931706@qq.com> --- pkg/core/region.go | 20 ++++++++++++++++++-- pkg/core/region_test.go | 12 ++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/pkg/core/region.go b/pkg/core/region.go index 57ba5cb3db0..72e499c558d 100644 --- a/pkg/core/region.go +++ b/pkg/core/region.go @@ -23,11 +23,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" @@ -968,6 +970,11 @@ func (r *RegionsInfo) setRegionLocked(region *RegionInfo, withOverlaps bool, ol // UpdateSubTree updates the subtree. func (r *RegionsInfo) UpdateSubTree(region, origin *RegionInfo, overlaps []*RegionInfo, rangeChanged bool) { + failpoint.Inject("UpdateSubTree", func() { + if origin == nil { + time.Sleep(time.Second) + } + }) r.st.Lock() defer r.st.Unlock() if origin != nil { @@ -976,8 +983,17 @@ func (r *RegionsInfo) UpdateSubTree(region, origin *RegionInfo, overlaps []*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 three 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/pkg/core/region_test.go b/pkg/core/region_test.go index 1e6b43fbf96..179a738bffd 100644 --- a/pkg/core/region_test.go +++ b/pkg/core/region_test.go @@ -21,6 +21,7 @@ import ( "strconv" "testing" + "github.com/pingcap/failpoint" "github.com/pingcap/kvproto/pkg/metapb" "github.com/pingcap/kvproto/pkg/pdpb" "github.com/stretchr/testify/require" @@ -423,6 +424,17 @@ 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) +} + func TestSetRegion(t *testing.T) { re := require.New(t) regions := NewRegionsInfo() From 355abcb35d7898b0759d1ecb45a5fca21a1fbc00 Mon Sep 17 00:00:00 2001 From: bufferflies <1045931706@qq.com> Date: Tue, 26 Sep 2023 17:37:45 +0800 Subject: [PATCH 2/2] add disable fail point Signed-off-by: bufferflies <1045931706@qq.com> --- pkg/core/region.go | 2 +- pkg/core/region_test.go | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/core/region.go b/pkg/core/region.go index 72e499c558d..5bb2eeb6d63 100644 --- a/pkg/core/region.go +++ b/pkg/core/region.go @@ -986,7 +986,7 @@ func (r *RegionsInfo) UpdateSubTree(region, origin *RegionInfo, overlaps []*Regi // 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 three again + // 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. diff --git a/pkg/core/region_test.go b/pkg/core/region_test.go index 179a738bffd..90ebbc0c4d8 100644 --- a/pkg/core/region_test.go +++ b/pkg/core/region_test.go @@ -433,6 +433,7 @@ func TestSetRegionConcurrence(t *testing.T) { regions.AtomicCheckAndPutRegion(region) }() regions.AtomicCheckAndPutRegion(region) + re.NoError(failpoint.Disable("github.com/tikv/pd/pkg/core/UpdateSubTree")) } func TestSetRegion(t *testing.T) {