From ea843a507a54b7eb3d8de8babf0f950e90624a40 Mon Sep 17 00:00:00 2001 From: Nick Ethier Date: Wed, 15 May 2019 13:00:24 -0400 Subject: [PATCH 1/3] scheduler: add check to prohibit returning inf during spread boost calculation --- scheduler/spread.go | 6 +++++- scheduler/spread_test.go | 24 ++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/scheduler/spread.go b/scheduler/spread.go index 669a7507904..36bdcc16bb5 100644 --- a/scheduler/spread.go +++ b/scheduler/spread.go @@ -194,7 +194,7 @@ func evenSpreadScoreBoost(pset *propertySet, option *structs.Node) float64 { for _, value := range combinedUseMap { if minCount == 0 || value < minCount { minCount = value - } + if maxCount == 0 || value > maxCount { maxCount = value } @@ -214,7 +214,11 @@ func evenSpreadScoreBoost(pset *propertySet, option *structs.Node) float64 { } else if minCount == maxCount { // Maximum possible penalty when the distribution is even return -1.0 + } else if minCount == 0 { + // Maximum possible boost + return 1.0 } + // Penalty based on delta from max value delta := int(maxCount - minCount) deltaBoost = float64(delta) / float64(minCount) diff --git a/scheduler/spread_test.go b/scheduler/spread_test.go index 83fb2749f94..bc7532314f0 100644 --- a/scheduler/spread_test.go +++ b/scheduler/spread_test.go @@ -1,6 +1,7 @@ package scheduler import ( + "math" "testing" "fmt" @@ -311,6 +312,7 @@ func TestSpreadIterator_EvenSpread(t *testing.T) { } for _, rn := range out { require.Equal(t, fmt.Sprintf("%.3f", expectedScores[rn.Node.Datacenter]), fmt.Sprintf("%.3f", rn.FinalScore)) + } // Update the plan to add allocs to nodes in dc1 @@ -543,3 +545,25 @@ func TestSpreadIterator_MaxPenalty(t *testing.T) { } } + +func Test_evenSpreadScoreBoost(t *testing.T) { + pset := &propertySet{ + existingValues: map[string]uint64{}, + proposedValues: map[string]uint64{ + "dc2": 1, + "dc1": 1, + "dc3": 1, + }, + clearedValues: map[string]uint64{ + "dc2": 1, + "dc3": 1, + }, + targetAttribute: "${node.datacenter}", + } + + opt := &structs.Node{ + Datacenter: "dc2", + } + boost := evenSpreadScoreBoost(pset, opt) + require.False(t, math.IsInf(boost, 1)) +} From 5709bf7b546b8f13ea552ccda30eb073e0306e7a Mon Sep 17 00:00:00 2001 From: Nick Ethier Date: Wed, 15 May 2019 13:02:04 -0400 Subject: [PATCH 2/3] fix missing brace --- scheduler/spread.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scheduler/spread.go b/scheduler/spread.go index 36bdcc16bb5..1e4961308e9 100644 --- a/scheduler/spread.go +++ b/scheduler/spread.go @@ -194,7 +194,7 @@ func evenSpreadScoreBoost(pset *propertySet, option *structs.Node) float64 { for _, value := range combinedUseMap { if minCount == 0 || value < minCount { minCount = value - + } if maxCount == 0 || value > maxCount { maxCount = value } From 566dd71486107caf3efb516664b28ecd6386d176 Mon Sep 17 00:00:00 2001 From: Preetha Appan Date: Wed, 15 May 2019 12:35:57 -0500 Subject: [PATCH 3/3] Fix comment and assert score in test case --- scheduler/spread.go | 3 ++- scheduler/spread_test.go | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/scheduler/spread.go b/scheduler/spread.go index 1e4961308e9..41846382dfa 100644 --- a/scheduler/spread.go +++ b/scheduler/spread.go @@ -215,7 +215,8 @@ func evenSpreadScoreBoost(pset *propertySet, option *structs.Node) float64 { // Maximum possible penalty when the distribution is even return -1.0 } else if minCount == 0 { - // Maximum possible boost + // Current attribute count is equal to min and both are zero. This means no allocations + // were placed for this attribute value yet. Should get the maximum possible boost. return 1.0 } diff --git a/scheduler/spread_test.go b/scheduler/spread_test.go index bc7532314f0..9b6d6309dc6 100644 --- a/scheduler/spread_test.go +++ b/scheduler/spread_test.go @@ -566,4 +566,5 @@ func Test_evenSpreadScoreBoost(t *testing.T) { } boost := evenSpreadScoreBoost(pset, opt) require.False(t, math.IsInf(boost, 1)) + require.Equal(t, 1.0, boost) }