diff --git a/.changelog/17198.txt b/.changelog/17198.txt new file mode 100644 index 000000000000..7b8fba50841e --- /dev/null +++ b/.changelog/17198.txt @@ -0,0 +1,3 @@ +```release-note:bug +scheduler: Fixed a bug where scores for spread scheduling could be -Inf +``` diff --git a/scheduler/spread.go b/scheduler/spread.go index 5bc50b970414..b2f4e8a8074b 100644 --- a/scheduler/spread.go +++ b/scheduler/spread.go @@ -30,6 +30,9 @@ type SpreadIterator struct { // blocks sumSpreadWeights int32 + // lowestSpreadBoost tracks the lowest spread boost across all spread blocks + lowestSpreadBoost float64 + // hasSpread is used to early return when the job/task group // does not have spread configured hasSpread bool @@ -53,6 +56,7 @@ func NewSpreadIterator(ctx Context, source RankIterator) *SpreadIterator { source: source, groupPropertySets: make(map[string][]*propertySet), tgSpreadInfo: make(map[string]spreadAttributeMap), + lowestSpreadBoost: -1.0, } return iter } @@ -114,6 +118,7 @@ func (iter *SpreadIterator) hasSpreads() bool { } func (iter *SpreadIterator) Next() *RankedNode { + for { option := iter.source.Next() @@ -162,7 +167,7 @@ func (iter *SpreadIterator) Next() *RankedNode { desiredCount, ok = spreadDetails.desiredCounts[implicitTarget] if !ok { // The desired count for this attribute is zero if it gets here - // so use the maximum possible penalty for this node + // so use the default negative penalty for this node totalSpreadScore -= 1.0 continue } @@ -171,12 +176,20 @@ func (iter *SpreadIterator) Next() *RankedNode { // Calculate the relative weight of this specific spread attribute spreadWeight := float64(spreadDetails.weight) / float64(iter.sumSpreadWeights) + if desiredCount == 0 { + totalSpreadScore += iter.lowestSpreadBoost + continue + } + // Score Boost is proportional the difference between current and desired count // It is negative when the used count is greater than the desired count // It is multiplied with the spread weight to account for cases where the job has // more than one spread attribute scoreBoost := ((desiredCount - float64(usedCount)) / desiredCount) * spreadWeight totalSpreadScore += scoreBoost + if scoreBoost < iter.lowestSpreadBoost { + iter.lowestSpreadBoost = scoreBoost + } } } diff --git a/scheduler/spread_test.go b/scheduler/spread_test.go index ea581b9a1513..5fa39b6714b6 100644 --- a/scheduler/spread_test.go +++ b/scheduler/spread_test.go @@ -13,6 +13,8 @@ import ( "github.com/hashicorp/nomad/helper/uuid" "github.com/hashicorp/nomad/nomad/mock" "github.com/hashicorp/nomad/nomad/structs" + "github.com/shoenig/test" + "github.com/shoenig/test/must" "github.com/stretchr/testify/require" ) @@ -558,6 +560,104 @@ func TestSpreadIterator_MaxPenalty(t *testing.T) { } +func TestSpreadIterator_NoInfinity(t *testing.T) { + ci.Parallel(t) + + store, ctx := testContext(t) + var nodes []*RankedNode + + // Add 3 nodes in different DCs to the state store + for i := 1; i < 4; i++ { + node := mock.Node() + node.Datacenter = fmt.Sprintf("dc%d", i) + must.NoError(t, store.UpsertNode(structs.MsgTypeTestSetup, uint64(100+i), node)) + nodes = append(nodes, &RankedNode{Node: node}) + } + + static := NewStaticRankIterator(ctx, nodes) + + job := mock.Job() + tg := job.TaskGroups[0] + job.TaskGroups[0].Count = 8 + + // Create spread target of 50% in dc1, 50% in dc2, and 0% in the implicit target + spread := &structs.Spread{ + Weight: 100, + Attribute: "${node.datacenter}", + SpreadTarget: []*structs.SpreadTarget{ + { + Value: "dc1", + Percent: 50, + }, + { + Value: "dc2", + Percent: 50, + }, + { + Value: "*", + Percent: 0, + }, + }, + } + tg.Spreads = []*structs.Spread{spread} + spreadIter := NewSpreadIterator(ctx, static) + spreadIter.SetJob(job) + spreadIter.SetTaskGroup(tg) + + scoreNorm := NewScoreNormalizationIterator(ctx, spreadIter) + + out := collectRanked(scoreNorm) + + // Scores should be even between dc1 and dc2 nodes, without an -Inf on dc3 + must.Len(t, 3, out) + test.Eq(t, 0.75, out[0].FinalScore) + test.Eq(t, 0.75, out[1].FinalScore) + test.Eq(t, -1, out[2].FinalScore) + + // Reset scores + for _, node := range nodes { + node.Scores = nil + node.FinalScore = 0 + } + + // Create very unbalanced spread target to force large negative scores + spread = &structs.Spread{ + Weight: 100, + Attribute: "${node.datacenter}", + SpreadTarget: []*structs.SpreadTarget{ + { + Value: "dc1", + Percent: 99, + }, + { + Value: "dc2", + Percent: 1, + }, + { + Value: "*", + Percent: 0, + }, + }, + } + tg.Spreads = []*structs.Spread{spread} + static = NewStaticRankIterator(ctx, nodes) + spreadIter = NewSpreadIterator(ctx, static) + spreadIter.SetJob(job) + spreadIter.SetTaskGroup(tg) + + scoreNorm = NewScoreNormalizationIterator(ctx, spreadIter) + + out = collectRanked(scoreNorm) + + // Scores should heavily favor dc1, with an -Inf on dc3 + must.Len(t, 3, out) + desired := 8 * 0.99 // 8 allocs * 99% + test.Eq(t, (desired-1)/desired, out[0].FinalScore) + test.Eq(t, -11.5, out[1].FinalScore) + test.LessEq(t, out[1].FinalScore, out[2].FinalScore, + test.Sprintf("expected implicit dc3 to be <= dc2")) +} + func Test_evenSpreadScoreBoost(t *testing.T) { ci.Parallel(t)