Skip to content

Commit

Permalink
region: fix the potential panic . (#7143)
Browse files Browse the repository at this point in the history
close #4399

Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>
  • Loading branch information
bufferflies and ti-chi-bot[bot] authored Sep 28, 2023
1 parent 3904a62 commit 54219d6
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 2 deletions.
20 changes: 18 additions & 2 deletions pkg/core/region.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand All @@ -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
}
}
Expand Down
13 changes: 13 additions & 0 deletions pkg/core/region_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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()
Expand Down

0 comments on commit 54219d6

Please sign in to comment.