From 61a5934b4ccaadcf6b273d6b7d090e9d8ff42b73 Mon Sep 17 00:00:00 2001 From: bufferflies <1045931706@qq.com> Date: Wed, 12 Aug 2026 09:22:15 +0200 Subject: [PATCH 1/3] schedulers: account for candidate region size in target balance score MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit targetStoreScore() only reflected already-pending operators' influence, never the size of the region currently being evaluated for the move. On a cluster where most regions are near-empty (e.g. freshly pre-split, mostly unwritten), the average region size collapses, tolerantResource shrinks with it, and a target store can look artificially light right up until the move lands — letting balance-region pick a target that becomes overloaded the instant the pending region is counted, which triggers a follow-up move to shed it again. Add the candidate region's own approximate size to the target delta (unamplified, since it is not "other pending influence" but the exact size about to be received) so a target's projected post-move score is what actually decides whether it's picked. Recalibrate TestInfluenceAmp's boundary counts: the new term shifted the pre-existing count+size boundary this test pins down by exactly one region-size step. Signed-off-by: bufferflies <1045931706@qq.com> --- .../schedulers/balance_region_test.go | 78 ++++++++++++++++++- pkg/schedule/schedulers/utils.go | 6 +- 2 files changed, 81 insertions(+), 3 deletions(-) diff --git a/pkg/schedule/schedulers/balance_region_test.go b/pkg/schedule/schedulers/balance_region_test.go index e5ce687db8..5f5b318c90 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" @@ -57,7 +60,7 @@ func TestInfluenceAmp(t *testing.T) { // It will schedule if the diff region count is greater than the sum // of TolerantSizeRatio and influenceAmp*2. - tc.AddRegionStore(1, int(100+influenceAmp+3)) + tc.AddRegionStore(1, int(100+influenceAmp+4)) tc.AddRegionStore(2, int(100-influenceAmp)) tc.AddLeaderRegion(1, 1, 2) region := tc.GetRegion(1).Clone(core.SetApproximateSize(R)) @@ -70,13 +73,84 @@ func TestInfluenceAmp(t *testing.T) { // It will not schedule if the diff region count is greater than the sum // of TolerantSizeRatio and influenceAmp*2. - tc.AddRegionStore(1, int(100+influenceAmp+2)) + tc.AddRegionStore(1, int(100+influenceAmp+3)) solver.Source = tc.GetStore(1) solver.sourceScore, solver.targetScore = solver.sourceStoreScore(""), solver.targetStoreScore("") re.False(solver.shouldBalance("")) re.Less(solver.sourceScore-solver.targetScore, float64(1)) } +// TestSingleRegionOnLargeEmptyDiskCanMigrate 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), region-score-v2 still produces enough of a score gap against an +// otherwise-identical, entirely empty peer store for balance-region to want to +// move that region — i.e. the score is dominated by the presence of the one +// region rather than by real disk utilization (10MiB vs 6TiB is a negligible +// utilization difference). +func TestSingleRegionOnLargeEmptyDiskCanMigrate(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. + tc.PutStore(mkStore(1, regionSizeMB)) + tc.PutStore(mkStore(2, 0)) + + tc.AddLeaderRegion(1, 1, 2) + 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 18 are freshly-split, empty regions. + // Without these, region 1 would be the cluster's only region and + // GetAverageRegionSize() would just equal its own size, defeating the + // tolerant-resource margin and masking the scenario this test documents. + 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 9 { + 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.True(solver.shouldBalance("")) +} + func TestShouldBalance(t *testing.T) { // store size = 100GiB // region size = 96MiB diff --git a/pkg/schedule/schedulers/utils.go b/pkg/schedule/schedulers/utils.go index c12d80e66e..ba60405cf1 100644 --- a/pkg/schedule/schedulers/utils.go +++ b/pkg/schedule/schedulers/utils.go @@ -139,7 +139,11 @@ 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 + // account for the candidate region's own size, so a target doesn't + // look artificially light just because this move hasn't landed yet. + // Unlike opInfluence (other, already-pending operators), this is not + // amplified: it is the literal size the target is about to receive. + targetDelta := influence*influenceAmp + tolerantResource + p.Region.GetApproximateSize() score = p.Target.RegionScore(p.GetSchedulerConfig().GetRegionScoreFormulaVersion(), p.GetSchedulerConfig().GetHighSpaceRatio(), p.GetSchedulerConfig().GetLowSpaceRatio(), targetDelta) case constant.WitnessKind: targetDelta := influence + tolerantResource From 8532b76782c265040d7603ba3628e5bea29d7629 Mon Sep 17 00:00:00 2001 From: bufferflies <1045931706@qq.com> Date: Wed, 12 Aug 2026 11:40:56 +0200 Subject: [PATCH 2/3] core, schedulers: exclude empty regions from balance-region's tolerant margin MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit getTolerantResource() derived its margin from GetAverageRegionSize(), which averages over every region in the cluster. On a cluster with many freshly-split, unwritten regions, that average collapses toward zero, so the tolerant margin stops damping marginal score differences between otherwise-equivalent stores. Add RegionsInfo.GetNonEmptyAverageRegionSize(), backed by a second pair of incrementally-maintained totals on regionTree (nonEmptyTotalSize / nonEmptyRegionsCnt) alongside the existing totalSize/length(), so it stays O(1). GetAverageRegionSize() itself is untouched; the new method is plumbed through the RegionSetInformer interface and rangeCluster's pass-through wrapper, and getTolerantResource() is switched to use it. Rework TestSingleRegionOnLargeEmptyDiskCanMigrate into TestSingleRegionOnLargeEmptyDiskDoesNotMigrate: with the tolerant margin no longer diluted, a single non-empty region isolated among many empty ones and stores correctly stays put — moving it would not fix a real imbalance and would just relocate the same "which store holds the only real data" state onto a different empty store, inviting the churn reported in #11135. Also drops the region's phantom peer on the target store from the fixture, per review feedback on the prior version of this test. Signed-off-by: bufferflies <1045931706@qq.com> --- pkg/core/basic_cluster.go | 1 + pkg/core/region.go | 13 ++++++++ pkg/core/region_tree.go | 33 ++++++++++++++++++- .../schedulers/balance_region_test.go | 33 ++++++++++--------- pkg/schedule/schedulers/range_cluster.go | 5 +++ pkg/schedule/schedulers/utils.go | 5 ++- 6 files changed, 72 insertions(+), 18 deletions(-) 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 5f5b318c90..9bae79819a 100644 --- a/pkg/schedule/schedulers/balance_region_test.go +++ b/pkg/schedule/schedulers/balance_region_test.go @@ -80,14 +80,16 @@ func TestInfluenceAmp(t *testing.T) { re.Less(solver.sourceScore-solver.targetScore, float64(1)) } -// TestSingleRegionOnLargeEmptyDiskCanMigrate 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), region-score-v2 still produces enough of a score gap against an -// otherwise-identical, entirely empty peer store for balance-region to want to -// move that region — i.e. the score is dominated by the presence of the one -// region rather than by real disk utilization (10MiB vs 6TiB is a negligible -// utilization difference). -func TestSingleRegionOnLargeEmptyDiskCanMigrate(t *testing.T) { +// 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) @@ -114,20 +116,19 @@ func TestSingleRegionOnLargeEmptyDiskCanMigrate(t *testing.T) { } // storeA holds the cluster's only non-empty region; storeB is otherwise - // identical (same capacity, same region count) but has no data at all. + // 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, 2) + 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 18 are freshly-split, empty regions. - // Without these, region 1 would be the cluster's only region and - // GetAverageRegionSize() would just equal its own size, defeating the - // tolerant-resource margin and masking the scenario this test documents. + // only one with any data — the other 19 are freshly-split, empty regions. var nextID uint64 = 2 for range 9 { tc.AddLeaderRegion(nextID, 1) @@ -135,7 +136,7 @@ func TestSingleRegionOnLargeEmptyDiskCanMigrate(t *testing.T) { tc.PutRegion(empty) nextID++ } - for range 9 { + for range 10 { tc.AddLeaderRegion(nextID, 2) empty := tc.GetRegion(nextID).Clone(core.SetApproximateSize(0)) tc.PutRegion(empty) @@ -148,7 +149,7 @@ func TestSingleRegionOnLargeEmptyDiskCanMigrate(t *testing.T) { 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("")) + re.False(solver.shouldBalance("")) } func TestShouldBalance(t *testing.T) { 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 ba60405cf1..a3a7c18614 100644 --- a/pkg/schedule/schedulers/utils.go +++ b/pkg/schedule/schedulers/utils.go @@ -182,7 +182,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 From dafffd2cbcac533fa7fb7f91eacbc2116d6bd160 Mon Sep 17 00:00:00 2001 From: bufferflies <1045931706@qq.com> Date: Wed, 12 Aug 2026 15:04:38 +0200 Subject: [PATCH 3/3] schedulers: stop double-counting candidate region size against tolerantResource targetStoreScore added the candidate region's own approximate size on top of tolerantResource, but tolerantResource (averageRegionSize * ratio) already represents roughly one region's worth of margin. In any ordinary cluster where the candidate is close to the average region size, this summed to about two regions' worth, silently doubling the score gap required before balance-region would act and rejecting legitimate moves between equal-sized regions (reported by rleungx: three 96MiB regions on a source vs an empty target, 192 vs 96 after the move, no longer scheduled). Take the larger of the two instead of adding them. This matches the long-standing but never-implemented intent documented in shouldBalance()'s own comment ("we use max(regionSize, averageRegionSize)"): fall back to the candidate's real size only when it exceeds the average-based margin. Verified against both the regression case above (now matches pre-PR behavior) and the originally-motivating churn scenario in TestSingleRegionOnLargeEmptyDiskDoesNotMigrate (still correctly stays put). TestInfluenceAmp's boundary counts revert to their original, pre-PR values, since max() is behaviorally identical to the base formula whenever the candidate doesn't exceed tolerantResource. Add TestBalanceRegionOrdinaryMoveNotBlockedByCandidateSize as a permanent regression test for this, since no existing test previously covered balancing between several ordinary, similarly-sized regions. Signed-off-by: bufferflies <1045931706@qq.com> --- .../schedulers/balance_region_test.go | 35 +++++++++++++++++-- pkg/schedule/schedulers/utils.go | 13 ++++--- 2 files changed, 41 insertions(+), 7 deletions(-) diff --git a/pkg/schedule/schedulers/balance_region_test.go b/pkg/schedule/schedulers/balance_region_test.go index 9bae79819a..b25c79a8b5 100644 --- a/pkg/schedule/schedulers/balance_region_test.go +++ b/pkg/schedule/schedulers/balance_region_test.go @@ -60,7 +60,7 @@ func TestInfluenceAmp(t *testing.T) { // It will schedule if the diff region count is greater than the sum // of TolerantSizeRatio and influenceAmp*2. - tc.AddRegionStore(1, int(100+influenceAmp+4)) + tc.AddRegionStore(1, int(100+influenceAmp+3)) tc.AddRegionStore(2, int(100-influenceAmp)) tc.AddLeaderRegion(1, 1, 2) region := tc.GetRegion(1).Clone(core.SetApproximateSize(R)) @@ -73,13 +73,44 @@ func TestInfluenceAmp(t *testing.T) { // It will not schedule if the diff region count is greater than the sum // of TolerantSizeRatio and influenceAmp*2. - tc.AddRegionStore(1, int(100+influenceAmp+3)) + tc.AddRegionStore(1, int(100+influenceAmp+2)) solver.Source = tc.GetStore(1) solver.sourceScore, solver.targetScore = solver.sourceStoreScore(""), solver.targetStoreScore("") re.False(solver.shouldBalance("")) 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 diff --git a/pkg/schedule/schedulers/utils.go b/pkg/schedule/schedulers/utils.go index a3a7c18614..901eaf21d0 100644 --- a/pkg/schedule/schedulers/utils.go +++ b/pkg/schedule/schedulers/utils.go @@ -139,11 +139,14 @@ func (p *solver) targetStoreScore(scheduleName string) float64 { targetDelta := influence + tolerantResource score = p.Target.LeaderScore(p.kind.Policy, targetDelta) case constant.RegionKind: - // account for the candidate region's own size, so a target doesn't - // look artificially light just because this move hasn't landed yet. - // Unlike opInfluence (other, already-pending operators), this is not - // amplified: it is the literal size the target is about to receive. - targetDelta := influence*influenceAmp + tolerantResource + p.Region.GetApproximateSize() + // 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