diff --git a/server/api/stats_test.go b/server/api/stats_test.go index bf92634ae58..19a27ab0c53 100644 --- a/server/api/stats_test.go +++ b/server/api/stats_test.go @@ -137,7 +137,7 @@ func (suite *statsTestSuite) TestRegionStats() { statsAll := &statistics.RegionStats{ Count: 4, EmptyCount: 1, - StorageSize: 351, + StorageSize: 350, StorageKeys: 221, StoreLeaderCount: map[uint64]int{1: 1, 4: 2, 5: 1}, StorePeerCount: map[uint64]int{1: 3, 2: 1, 3: 1, 4: 2, 5: 2}, @@ -150,7 +150,7 @@ func (suite *statsTestSuite) TestRegionStats() { stats23 := &statistics.RegionStats{ Count: 2, EmptyCount: 1, - StorageSize: 201, + StorageSize: 200, StorageKeys: 151, StoreLeaderCount: map[uint64]int{4: 1, 5: 1}, StorePeerCount: map[uint64]int{1: 2, 4: 1, 5: 2}, diff --git a/server/cluster/cluster.go b/server/cluster/cluster.go index 688bbcc4941..631dd85129d 100644 --- a/server/cluster/cluster.go +++ b/server/cluster/cluster.go @@ -843,7 +843,9 @@ func (c *RaftCluster) processRegionHeartbeat(region *core.RegionInfo) error { if err != nil { return err } - region.Inherit(origin, c.storeConfigManager.GetStoreConfig().IsEnableRegionBucket()) + if c.GetStoreConfig().IsEnableRegionBucket() { + region.InheritBuckets(origin) + } c.hotStat.CheckWriteAsync(statistics.NewCheckExpiredItemTask(region)) c.hotStat.CheckReadAsync(statistics.NewCheckExpiredItemTask(region)) diff --git a/server/core/region.go b/server/core/region.go index 07b522e47ca..7a1fb2c87c2 100644 --- a/server/core/region.go +++ b/server/core/region.go @@ -144,8 +144,9 @@ const ( func RegionFromHeartbeat(heartbeat *pdpb.RegionHeartbeatRequest, opts ...RegionCreateOption) *RegionInfo { // Convert unit to MB. // If region isn't empty and less than 1MB, use 1MB instead. - // The size of empty region will be correct by the previous RegionInfo. regionSize := heartbeat.GetApproximateSize() / units.MiB + // Due to https://github.com/tikv/tikv/pull/11170, if region size is not initialized, + // approximate size will be zero, and region size is zero not EmptyRegionApproximateSize if heartbeat.GetApproximateSize() > 0 && regionSize < EmptyRegionApproximateSize { regionSize = EmptyRegionApproximateSize } @@ -188,19 +189,9 @@ func RegionFromHeartbeat(heartbeat *pdpb.RegionHeartbeatRequest, opts ...RegionC return region } -// Inherit inherits the buckets and region size from the parent region if bucket enabled. -// correct approximate size and buckets by the previous size if here exists a reported RegionInfo. -// See https://github.com/tikv/tikv/issues/11114 -func (r *RegionInfo) Inherit(origin *RegionInfo, bucketEnable bool) { - // regionSize should not be zero if region is not empty. - if r.GetApproximateSize() == 0 { - if origin != nil { - r.approximateSize = origin.approximateSize - } else { - r.approximateSize = EmptyRegionApproximateSize - } - } - if bucketEnable && origin != nil && r.buckets == nil { +// InheritBuckets inherits the buckets from the parent region if bucket enabled. +func (r *RegionInfo) InheritBuckets(origin *RegionInfo) { + if origin != nil && r.buckets == nil { r.buckets = origin.buckets } } @@ -478,6 +469,13 @@ func (r *RegionInfo) GetApproximateSize() int64 { return r.approximateSize } +// IsEmptyRegion returns whether the region is empty. +func (r *RegionInfo) IsEmptyRegion() bool { + // When cluster resumes, the region size may be not initialized, but region heartbeat is send. + // So use `==` here. + return r.approximateSize == EmptyRegionApproximateSize +} + // GetApproximateKeys returns the approximate keys of the region. func (r *RegionInfo) GetApproximateKeys() int64 { return r.approximateKeys diff --git a/server/core/region_test.go b/server/core/region_test.go index d6030997fa3..8f63125e5d6 100644 --- a/server/core/region_test.go +++ b/server/core/region_test.go @@ -186,35 +186,9 @@ func TestSortedEqual(t *testing.T) { } } -func TestInherit(t *testing.T) { +func TestInheritBuckets(t *testing.T) { re := require.New(t) - // size in MB - // case for approximateSize - testCases := []struct { - originExists bool - originSize uint64 - size uint64 - expect uint64 - }{ - {false, 0, 0, 1}, - {false, 0, 2, 2}, - {true, 0, 2, 2}, - {true, 1, 2, 2}, - {true, 2, 0, 2}, - } - for _, testCase := range testCases { - var origin *RegionInfo - if testCase.originExists { - origin = NewRegionInfo(&metapb.Region{Id: 100}, nil) - origin.approximateSize = int64(testCase.originSize) - } - r := NewRegionInfo(&metapb.Region{Id: 100}, nil) - r.approximateSize = int64(testCase.size) - r.Inherit(origin, false) - re.Equal(int64(testCase.expect), r.approximateSize) - } - // bucket data := []struct { originBuckets *metapb.Buckets buckets *metapb.Buckets @@ -227,12 +201,11 @@ func TestInherit(t *testing.T) { for _, d := range data { origin := NewRegionInfo(&metapb.Region{Id: 100}, nil, SetBuckets(d.originBuckets)) r := NewRegionInfo(&metapb.Region{Id: 100}, nil) - r.Inherit(origin, true) + r.InheritBuckets(origin) re.Equal(d.originBuckets, r.GetBuckets()) // region will not inherit bucket keys. if origin.GetBuckets() != nil { newRegion := NewRegionInfo(&metapb.Region{Id: 100}, nil) - newRegion.Inherit(origin, false) re.NotEqual(d.originBuckets, newRegion.GetBuckets()) } } diff --git a/server/statistics/region.go b/server/statistics/region.go index 2822bb64ead..60e2f19935d 100644 --- a/server/statistics/region.go +++ b/server/statistics/region.go @@ -57,10 +57,12 @@ func (s *RegionStats) Observe(r *core.RegionInfo) { s.Count++ approximateKeys := r.GetApproximateKeys() approximateSize := r.GetApproximateSize() - if approximateSize <= core.EmptyRegionApproximateSize { + if approximateSize == core.EmptyRegionApproximateSize { s.EmptyCount++ } - s.StorageSize += approximateSize + if !r.IsEmptyRegion() { + s.StorageSize += approximateSize + } s.StorageKeys += approximateKeys leader := r.GetLeader() if leader != nil { diff --git a/server/statistics/region_collection.go b/server/statistics/region_collection.go index ab5e6b22d9c..56d5516c8b7 100644 --- a/server/statistics/region_collection.go +++ b/server/statistics/region_collection.go @@ -198,7 +198,7 @@ func (r *RegionStatistics) Observe(region *core.RegionInfo, stores []*core.Store DownPeer: len(region.GetDownPeers()) > 0, PendingPeer: len(region.GetPendingPeers()) > 0, LearnerPeer: len(region.GetLearners()) > 0, - EmptyRegion: region.GetApproximateSize() <= core.EmptyRegionApproximateSize, + EmptyRegion: region.IsEmptyRegion(), OversizedRegion: region.IsOversized( int64(r.storeConfigManager.GetStoreConfig().GetRegionMaxSize()), int64(r.storeConfigManager.GetStoreConfig().GetRegionMaxKeys()), @@ -206,7 +206,7 @@ func (r *RegionStatistics) Observe(region *core.RegionInfo, stores []*core.Store UndersizedRegion: region.NeedMerge( int64(r.opt.GetMaxMergeRegionSize()), int64(r.opt.GetMaxMergeRegionKeys()), - ), + ) && region.GetApproximateSize() >= core.EmptyRegionApproximateSize, } for typ, c := range conditions { diff --git a/server/statistics/region_collection_test.go b/server/statistics/region_collection_test.go index 7c686f1c9ce..179d78b539f 100644 --- a/server/statistics/region_collection_test.go +++ b/server/statistics/region_collection_test.go @@ -63,7 +63,7 @@ func TestRegionStatistics(t *testing.T) { stores[3] = store3 r1 := &metapb.Region{Id: 1, Peers: peers, StartKey: []byte("aa"), EndKey: []byte("bb")} r2 := &metapb.Region{Id: 2, Peers: peers[0:2], StartKey: []byte("cc"), EndKey: []byte("dd")} - region1 := core.NewRegionInfo(r1, peers[0]) + region1 := core.NewRegionInfo(r1, peers[0], core.SetApproximateSize(1)) region2 := core.NewRegionInfo(r2, peers[0]) regionStats := NewRegionStatistics(opt, manager, nil) regionStats.Observe(region1, stores) @@ -103,7 +103,8 @@ func TestRegionStatistics(t *testing.T) { re.Len(regionStats.stats[PendingPeer], 1) re.Len(regionStats.stats[LearnerPeer], 1) re.Len(regionStats.stats[OversizedRegion], 1) - re.Len(regionStats.stats[UndersizedRegion], 1) + re.Len(regionStats.stats[UndersizedRegion], 0) + re.Len(regionStats.stats[EmptyRegion], 0) re.Len(regionStats.offlineStats[ExtraPeer], 1) re.Empty(regionStats.offlineStats[MissPeer]) re.Len(regionStats.offlineStats[DownPeer], 1) diff --git a/tests/pdctl/scheduler/scheduler_test.go b/tests/pdctl/scheduler/scheduler_test.go index 1fc69c89e79..c904e80940b 100644 --- a/tests/pdctl/scheduler/scheduler_test.go +++ b/tests/pdctl/scheduler/scheduler_test.go @@ -24,6 +24,7 @@ import ( "github.com/stretchr/testify/require" "github.com/tikv/pd/pkg/testutil" "github.com/tikv/pd/server/config" + "github.com/tikv/pd/server/core" "github.com/tikv/pd/server/versioninfo" "github.com/tikv/pd/tests" "github.com/tikv/pd/tests/pdctl" @@ -128,7 +129,8 @@ func TestScheduler(t *testing.T) { pdctl.MustPutStore(re, leaderServer.GetServer(), store) } - pdctl.MustPutRegion(re, cluster, 1, 1, []byte("a"), []byte("b")) + // note: because pdqsort is a unstable sort algorithm, set ApproximateSize for this region. + pdctl.MustPutRegion(re, cluster, 1, 1, []byte("a"), []byte("b"), core.SetApproximateSize(10)) time.Sleep(3 * time.Second) // scheduler show command