Commit aee471db authored by Seth Hoenig's avatar Seth Hoenig Committed by Mahmood Ali
Browse files

Merge pull request #10904 from hashicorp/b-no-affinity-intern

core: remove internalization of affinity strings
parent c13b7705
Showing with 23 additions and 28 deletions
+23 -28
```release-note:bug
core: Fixed a bug where affinity memoization may cause planning problems
```
...@@ -761,14 +761,12 @@ func TestJobDiff(t *testing.T) { ...@@ -761,14 +761,12 @@ func TestJobDiff(t *testing.T) {
RTarget: "foo", RTarget: "foo",
Operand: "foo", Operand: "foo",
Weight: 20, Weight: 20,
str: "foo",
}, },
{ {
LTarget: "bar", LTarget: "bar",
RTarget: "bar", RTarget: "bar",
Operand: "bar", Operand: "bar",
Weight: 20, Weight: 20,
str: "bar",
}, },
}, },
}, },
...@@ -779,14 +777,12 @@ func TestJobDiff(t *testing.T) { ...@@ -779,14 +777,12 @@ func TestJobDiff(t *testing.T) {
RTarget: "foo", RTarget: "foo",
Operand: "foo", Operand: "foo",
Weight: 20, Weight: 20,
str: "foo",
}, },
{ {
LTarget: "baz", LTarget: "baz",
RTarget: "baz", RTarget: "baz",
Operand: "baz", Operand: "baz",
Weight: 20, Weight: 20,
str: "baz",
}, },
}, },
}, },
...@@ -1558,14 +1554,12 @@ func TestTaskGroupDiff(t *testing.T) { ...@@ -1558,14 +1554,12 @@ func TestTaskGroupDiff(t *testing.T) {
RTarget: "foo", RTarget: "foo",
Operand: "foo", Operand: "foo",
Weight: 20, Weight: 20,
str: "foo",
}, },
{ {
LTarget: "bar", LTarget: "bar",
RTarget: "bar", RTarget: "bar",
Operand: "bar", Operand: "bar",
Weight: 20, Weight: 20,
str: "bar",
}, },
}, },
}, },
...@@ -1576,14 +1570,12 @@ func TestTaskGroupDiff(t *testing.T) { ...@@ -1576,14 +1570,12 @@ func TestTaskGroupDiff(t *testing.T) {
RTarget: "foo", RTarget: "foo",
Operand: "foo", Operand: "foo",
Weight: 20, Weight: 20,
str: "foo",
}, },
{ {
LTarget: "baz", LTarget: "baz",
RTarget: "baz", RTarget: "baz",
Operand: "baz", Operand: "baz",
Weight: 20, Weight: 20,
str: "baz",
}, },
}, },
}, },
...@@ -4040,14 +4032,12 @@ func TestTaskDiff(t *testing.T) { ...@@ -4040,14 +4032,12 @@ func TestTaskDiff(t *testing.T) {
RTarget: "foo", RTarget: "foo",
Operand: "foo", Operand: "foo",
Weight: 20, Weight: 20,
str: "foo",
}, },
{ {
LTarget: "bar", LTarget: "bar",
RTarget: "bar", RTarget: "bar",
Operand: "bar", Operand: "bar",
Weight: 20, Weight: 20,
str: "bar",
}, },
}, },
}, },
...@@ -4058,14 +4048,12 @@ func TestTaskDiff(t *testing.T) { ...@@ -4058,14 +4048,12 @@ func TestTaskDiff(t *testing.T) {
RTarget: "foo", RTarget: "foo",
Operand: "foo", Operand: "foo",
Weight: 20, Weight: 20,
str: "foo",
}, },
{ {
LTarget: "baz", LTarget: "baz",
RTarget: "baz", RTarget: "baz",
Operand: "baz", Operand: "baz",
Weight: 20, Weight: 20,
str: "baz",
}, },
}, },
}, },
......
...@@ -8226,10 +8226,9 @@ type Affinity struct { ...@@ -8226,10 +8226,9 @@ type Affinity struct {
RTarget string // Right-hand target RTarget string // Right-hand target
Operand string // Affinity operand (<=, <, =, !=, >, >=), set_contains_all, set_contains_any Operand string // Affinity operand (<=, <, =, !=, >, >=), set_contains_all, set_contains_any
Weight int8 // Weight applied to nodes that match the affinity. Can be negative Weight int8 // Weight applied to nodes that match the affinity. Can be negative
str string // Memoized string
} }
// Equal checks if two affinities are equal // Equals checks if two affinities are equal.
func (a *Affinity) Equals(o *Affinity) bool { func (a *Affinity) Equals(o *Affinity) bool {
return a == o || return a == o ||
a.LTarget == o.LTarget && a.LTarget == o.LTarget &&
...@@ -8246,17 +8245,16 @@ func (a *Affinity) Copy() *Affinity { ...@@ -8246,17 +8245,16 @@ func (a *Affinity) Copy() *Affinity {
if a == nil { if a == nil {
return nil return nil
} }
na := new(Affinity) return &Affinity{
*na = *a LTarget: a.LTarget,
return na RTarget: a.RTarget,
Operand: a.Operand,
Weight: a.Weight,
}
} }
func (a *Affinity) String() string { func (a *Affinity) String() string {
if a.str != "" { return fmt.Sprintf("%s %s %s %v", a.LTarget, a.Operand, a.RTarget, a.Weight)
return a.str
}
a.str = fmt.Sprintf("%s %s %s %v", a.LTarget, a.Operand, a.RTarget, a.Weight)
return a.str
} }
func (a *Affinity) Validate() error { func (a *Affinity) Validate() error {
......
...@@ -284,6 +284,12 @@ func TestJob_SpecChanged(t *testing.T) { ...@@ -284,6 +284,12 @@ func TestJob_SpecChanged(t *testing.T) {
Original: &Job{Constraints: []*Constraint{{"A", "B", "="}}}, Original: &Job{Constraints: []*Constraint{{"A", "B", "="}}},
New: &Job{Constraints: []*Constraint{{"A", "B", "="}}}, New: &Job{Constraints: []*Constraint{{"A", "B", "="}}},
}, },
{
Name: "With Affinities",
Changed: false,
Original: &Job{Affinities: []*Affinity{{"A", "B", "=", 1}}},
New: &Job{Affinities: []*Affinity{{"A", "B", "=", 1}}},
},
} }
for _, c := range cases { for _, c := range cases {
...@@ -814,14 +820,14 @@ func TestJob_PartEqual(t *testing.T) { ...@@ -814,14 +820,14 @@ func TestJob_PartEqual(t *testing.T) {
})) }))
as := &Affinities{ as := &Affinities{
&Affinity{"left0", "right0", "=", 0, ""}, &Affinity{"left0", "right0", "=", 0},
&Affinity{"left1", "right1", "=", 0, ""}, &Affinity{"left1", "right1", "=", 0},
&Affinity{"left2", "right2", "=", 0, ""}, &Affinity{"left2", "right2", "=", 0},
} }
require.True(t, as.Equals(&Affinities{ require.True(t, as.Equals(&Affinities{
&Affinity{"left0", "right0", "=", 0, ""}, &Affinity{"left0", "right0", "=", 0},
&Affinity{"left2", "right2", "=", 0, ""}, &Affinity{"left2", "right2", "=", 0},
&Affinity{"left1", "right1", "=", 0, ""}, &Affinity{"left1", "right1", "=", 0},
})) }))
} }
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment