diff --git a/pkg/core/basic_cluster.go b/pkg/core/basic_cluster.go index 4bc192c88c..a07e725c2d 100644 --- a/pkg/core/basic_cluster.go +++ b/pkg/core/basic_cluster.go @@ -97,6 +97,7 @@ type RegionSetInformer interface { RandWitnessRegions(storeID uint64, ranges []keyutil.KeyRange) []*RegionInfo RandPendingRegions(storeID uint64, ranges []keyutil.KeyRange) []*RegionInfo GetAverageRegionSize() int64 + GetNonEmptyAverageRegionSize() int64 GetStoreRegionCount(storeID uint64) int GetRegion(id uint64) *RegionInfo GetAdjacentRegions(region *RegionInfo) (*RegionInfo, *RegionInfo) diff --git a/pkg/core/region.go b/pkg/core/region.go index 40701a5137..8b2a94f7ac 100644 --- a/pkg/core/region.go +++ b/pkg/core/region.go @@ -2350,6 +2350,19 @@ func (r *RegionsInfo) GetAverageRegionSize() int64 { return r.tree.TotalSize() / int64(r.tree.length()) } +// GetNonEmptyAverageRegionSize returns the average approximate size of +// non-empty regions only. Empty regions (e.g. freshly split, unwritten +// regions) are excluded so a cluster with many of them doesn't get this +// average diluted toward noise levels. +func (r *RegionsInfo) GetNonEmptyAverageRegionSize() int64 { + r.t.RLock() + defer r.t.RUnlock() + if r.tree.nonEmptyRegionsCnt == 0 { + return 0 + } + return r.tree.nonEmptyTotalSize / int64(r.tree.nonEmptyRegionsCnt) +} + // ValidRegion is used to decide if the region is valid. func (r *RegionsInfo) ValidRegion(region *metapb.Region) error { startKey := region.GetStartKey() diff --git a/pkg/core/region_tree.go b/pkg/core/region_tree.go index e7bebf09fd..4bfee0b178 100644 --- a/pkg/core/region_tree.go +++ b/pkg/core/region_tree.go @@ -62,7 +62,14 @@ const ( type regionTree struct { tree *btree.BTreeG[*regionItem] // Statistics - totalSize int64 + totalSize int64 + // nonEmptyTotalSize and nonEmptyRegionsCnt mirror totalSize/length but + // exclude empty regions (approximateSize <= EmptyRegionApproximateSize), + // so GetAverageRegionSize can reflect only regions that actually hold + // data instead of being diluted by a large number of freshly-split, + // unwritten regions. + nonEmptyTotalSize int64 + nonEmptyRegionsCnt int totalWriteBytesRate float64 totalWriteKeysRate float64 // count the number of regions that not loaded from storage. @@ -75,6 +82,8 @@ func newRegionTree() *regionTree { return ®ionTree{ tree: btree.NewG[*regionItem](defaultBTreeDegree), totalSize: 0, + nonEmptyTotalSize: 0, + nonEmptyRegionsCnt: 0, totalWriteBytesRate: 0, totalWriteKeysRate: 0, notFromStorageRegionsCnt: 0, @@ -85,6 +94,8 @@ func newRegionTreeWithCountRef() *regionTree { return ®ionTree{ tree: btree.NewG[*regionItem](defaultBTreeDegree), totalSize: 0, + nonEmptyTotalSize: 0, + nonEmptyRegionsCnt: 0, totalWriteBytesRate: 0, totalWriteKeysRate: 0, notFromStorageRegionsCnt: 0, @@ -174,6 +185,10 @@ func (t *regionTree) updateRef(origin, region *RegionInfo) { func (t *regionTree) update(item *regionItem, withOverlaps bool, overlaps ...*RegionInfo) []*RegionInfo { region := item.RegionInfo t.totalSize += region.approximateSize + if region.approximateSize > EmptyRegionApproximateSize { + t.nonEmptyTotalSize += region.approximateSize + t.nonEmptyRegionsCnt++ + } regionWriteBytesRate, regionWriteKeysRate := region.GetWriteRate() t.totalWriteBytesRate += regionWriteBytesRate t.totalWriteKeysRate += regionWriteKeysRate @@ -201,6 +216,10 @@ func (t *regionTree) update(item *regionItem, withOverlaps bool, overlaps ...*Re logutil.ZapRedactStringer("delete-region", RegionToHexMeta(old.GetMeta())), logutil.ZapRedactStringer("update-region", RegionToHexMeta(region.GetMeta()))) t.totalSize -= old.approximateSize + if old.approximateSize > EmptyRegionApproximateSize { + t.nonEmptyTotalSize -= old.approximateSize + t.nonEmptyRegionsCnt-- + } regionWriteBytesRate, regionWriteKeysRate = old.GetWriteRate() t.totalWriteBytesRate -= regionWriteBytesRate t.totalWriteKeysRate -= regionWriteKeysRate @@ -218,11 +237,19 @@ func (t *regionTree) update(item *regionItem, withOverlaps bool, overlaps ...*Re // updateStat is used to update statistics when RegionInfo is directly replaced. func (t *regionTree) updateStat(origin *RegionInfo, region *RegionInfo) { t.totalSize += region.approximateSize + if region.approximateSize > EmptyRegionApproximateSize { + t.nonEmptyTotalSize += region.approximateSize + t.nonEmptyRegionsCnt++ + } regionWriteBytesRate, regionWriteKeysRate := region.GetWriteRate() t.totalWriteBytesRate += regionWriteBytesRate t.totalWriteKeysRate += regionWriteKeysRate t.totalSize -= origin.approximateSize + if origin.approximateSize > EmptyRegionApproximateSize { + t.nonEmptyTotalSize -= origin.approximateSize + t.nonEmptyRegionsCnt-- + } regionWriteBytesRate, regionWriteKeysRate = origin.GetWriteRate() t.totalWriteBytesRate -= regionWriteBytesRate t.totalWriteKeysRate -= regionWriteKeysRate @@ -252,6 +279,10 @@ func (t *regionTree) remove(region *RegionInfo) { } t.totalSize -= result.GetApproximateSize() + if result.GetApproximateSize() > EmptyRegionApproximateSize { + t.nonEmptyTotalSize -= result.GetApproximateSize() + t.nonEmptyRegionsCnt-- + } regionWriteBytesRate, regionWriteKeysRate := result.GetWriteRate() t.totalWriteBytesRate -= regionWriteBytesRate t.totalWriteKeysRate -= regionWriteKeysRate diff --git a/pkg/schedule/schedulers/balance_region_test.go b/pkg/schedule/schedulers/balance_region_test.go index e5ce687db8..b25c79a8b5 100644 --- a/pkg/schedule/schedulers/balance_region_test.go +++ b/pkg/schedule/schedulers/balance_region_test.go @@ -17,10 +17,13 @@ package schedulers import ( "fmt" "testing" + "time" + "github.com/docker/go-units" "github.com/stretchr/testify/require" "github.com/pingcap/kvproto/pkg/metapb" + "github.com/pingcap/kvproto/pkg/pdpb" "github.com/tikv/pd/pkg/core" "github.com/tikv/pd/pkg/core/constant" @@ -77,6 +80,109 @@ func TestInfluenceAmp(t *testing.T) { re.Less(solver.sourceScore-solver.targetScore, float64(1)) } +// TestBalanceRegionOrdinaryMoveNotBlockedByCandidateSize guards against +// double-counting the candidate region's size on top of tolerantResource in +// targetStoreScore. tolerantResource already represents roughly one average +// region's worth of margin, so a candidate whose size sits at or below that +// margin must not additionally raise the bar the target has to clear. Here +// the source holds three ordinary, equal-sized regions and the target is +// empty: moving one region is a clear improvement (192 vs 96 after the move) +// and must still be scheduled. +func TestBalanceRegionOrdinaryMoveNotBlockedByCandidateSize(t *testing.T) { + cancel, _, tc, oc := prepareSchedulersTest() + defer cancel() + re := require.New(t) + + tc.SetTolerantSizeRatio(1) + tc.SetRegionScoreFormulaVersion("v1") + + tc.AddRegionStore(1, 3, 288) + tc.AddRegionStore(2, 0, 0) + tc.AddLeaderRegion(1, 1) + region := tc.GetRegion(1).Clone(core.SetApproximateSize(96)) + tc.PutRegion(region) + + kind := constant.NewScheduleKind(constant.RegionKind, constant.BySize) + influence := oc.GetOpInfluence(tc.GetBasicCluster()) + basePlan := plan.NewBalanceSchedulerPlan() + solver := newSolver(basePlan, kind, tc, influence) + solver.Source, solver.Target, solver.Region = tc.GetStore(1), tc.GetStore(2), tc.GetRegion(1) + solver.sourceScore, solver.targetScore = solver.sourceStoreScore(""), solver.targetStoreScore("") + re.True(solver.shouldBalance("")) +} + +// TestSingleRegionOnLargeEmptyDiskDoesNotMigrate verifies that when a store's +// disk is mostly empty except for a single small region (e.g. 6TiB capacity +// with only a 10MiB region), balance-region does NOT move that region to an +// otherwise-identical, entirely empty peer store. Moving it would not fix any +// real imbalance (10MiB vs 6TiB capacity is a negligible utilization +// difference either way) and would just relocate the same "which store holds +// the only real data" state onto a different empty store — inviting the kind +// of pointless churn reported in #11135, since every other empty store looks +// equally attractive as a target on the next scheduling pass. +func TestSingleRegionOnLargeEmptyDiskDoesNotMigrate(t *testing.T) { + cancel, _, tc, oc := prepareSchedulersTest() + defer cancel() + re := require.New(t) + + const ( + capacity = 6 * units.TiB + regionSizeMB = 10 // MiB, matches core.StoreInfo.GetRegionSize()'s unit + ) + + mkStore := func(id uint64, usedMB int64) *core.StoreInfo { + usedBytes := uint64(usedMB) * units.MiB + stats := &pdpb.StoreStats{ + Capacity: capacity, + UsedSize: usedBytes, + Available: capacity - usedBytes, + } + return core.NewStoreInfo( + &metapb.Store{Id: id, State: metapb.StoreState_Up}, + core.SetStoreStats(stats), + core.SetRegionCount(10), + core.SetRegionSize(usedMB), + core.SetLastHeartbeatTS(time.Now()), + ) + } + + // storeA holds the cluster's only non-empty region; storeB is otherwise + // identical (same capacity, same region count) but has no data at all, + // and does not already hold a peer of region 1 — a real Schedule() run + // would still consider it a legitimate, unfiltered candidate target. + tc.PutStore(mkStore(1, regionSizeMB)) + tc.PutStore(mkStore(2, 0)) + + tc.AddLeaderRegion(1, 1) + region := tc.GetRegion(1).Clone(core.SetApproximateSize(regionSizeMB)) + tc.PutRegion(region) + + // Mirror the real-world setup that motivated this test: each store already + // holds 10 regions (region *count* is balanced), but region 1 above is the + // only one with any data — the other 19 are freshly-split, empty regions. + var nextID uint64 = 2 + for range 9 { + tc.AddLeaderRegion(nextID, 1) + empty := tc.GetRegion(nextID).Clone(core.SetApproximateSize(0)) + tc.PutRegion(empty) + nextID++ + } + for range 10 { + tc.AddLeaderRegion(nextID, 2) + empty := tc.GetRegion(nextID).Clone(core.SetApproximateSize(0)) + tc.PutRegion(empty) + nextID++ + } + + kind := constant.NewScheduleKind(constant.RegionKind, constant.BySize) + influence := oc.GetOpInfluence(tc.GetBasicCluster()) + basePlan := plan.NewBalanceSchedulerPlan() + solver := newSolver(basePlan, kind, tc, influence) + solver.Source, solver.Target, solver.Region = tc.GetStore(1), tc.GetStore(2), tc.GetRegion(1) + solver.sourceScore, solver.targetScore = solver.sourceStoreScore(""), solver.targetStoreScore("") + re.False(solver.shouldBalance("")) +} + func TestShouldBalance(t *testing.T) { // store size = 100GiB // region size = 96MiB diff --git a/pkg/schedule/schedulers/range_cluster.go b/pkg/schedule/schedulers/range_cluster.go index 684d4417e0..ef3afc3dc5 100644 --- a/pkg/schedule/schedulers/range_cluster.go +++ b/pkg/schedule/schedulers/range_cluster.go @@ -117,6 +117,11 @@ func (r *rangeCluster) GetAverageRegionSize() int64 { return r.subCluster.GetAverageRegionSize() } +// GetNonEmptyAverageRegionSize returns the average approximate size of non-empty regions. +func (r *rangeCluster) GetNonEmptyAverageRegionSize() int64 { + return r.subCluster.GetNonEmptyAverageRegionSize() +} + // GetAvgNetworkSlowScore returns the average network slow score. func (r *rangeCluster) GetAvgNetworkSlowScore(id uint64) uint64 { return r.subCluster.GetAvgNetworkSlowScore(id) diff --git a/pkg/schedule/schedulers/utils.go b/pkg/schedule/schedulers/utils.go index c12d80e66e..901eaf21d0 100644 --- a/pkg/schedule/schedulers/utils.go +++ b/pkg/schedule/schedulers/utils.go @@ -139,7 +139,14 @@ func (p *solver) targetStoreScore(scheduleName string) float64 { targetDelta := influence + tolerantResource score = p.Target.LeaderScore(p.kind.Policy, targetDelta) case constant.RegionKind: - targetDelta := influence*influenceAmp + tolerantResource + // tolerantResource already represents "about one region's worth" of + // margin (averageRegionSize * ratio). Adding the candidate's own size + // on top of it double-counts the same quantity and rejects ordinary + // moves between equal-sized regions. Take the larger of the two + // instead: fall back to the candidate's real size only when it + // exceeds the average-based margin, so a target doesn't look + // artificially light just because this move hasn't landed yet. + targetDelta := influence*influenceAmp + max(tolerantResource, p.Region.GetApproximateSize()) score = p.Target.RegionScore(p.GetSchedulerConfig().GetRegionScoreFormulaVersion(), p.GetSchedulerConfig().GetHighSpaceRatio(), p.GetSchedulerConfig().GetLowSpaceRatio(), targetDelta) case constant.WitnessKind: targetDelta := influence + tolerantResource @@ -178,7 +185,10 @@ func (p *solver) getTolerantResource() int64 { if (p.kind.Resource == constant.LeaderKind || p.kind.Resource == constant.WitnessKind) && p.kind.Policy == constant.ByCount { p.tolerantSource = int64(p.tolerantSizeRatio) } else { - regionSize := p.GetAverageRegionSize() + // Use the non-empty average so a cluster full of freshly-split, + // unwritten regions doesn't collapse the tolerant margin toward + // noise levels. + regionSize := p.GetNonEmptyAverageRegionSize() p.tolerantSource = int64(float64(regionSize) * p.tolerantSizeRatio) } return p.tolerantSource