diff --git a/pkg/core/region.go b/pkg/core/region.go index 9e32bf7c2f5..8d0379f266f 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" @@ -996,6 +998,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 { @@ -1004,8 +1011,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 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/pkg/core/region_test.go b/pkg/core/region_test.go index 5588d9190ec..50302de920e 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" @@ -450,6 +451,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()