From 240510132837ccaf4c026892d88bfbdeb87e6094 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Fri, 16 Oct 2015 17:05:23 -0700 Subject: [PATCH] Remove base nodes from stack constructors --- scheduler/generic_sched.go | 2 +- scheduler/stack.go | 18 ++++----------- scheduler/stack_test.go | 45 +++++++++++++++++++++++--------------- scheduler/system_sched.go | 2 +- scheduler/util_test.go | 15 +++++++++---- 5 files changed, 44 insertions(+), 38 deletions(-) diff --git a/scheduler/generic_sched.go b/scheduler/generic_sched.go index a74e5baca..7957f2360 100644 --- a/scheduler/generic_sched.go +++ b/scheduler/generic_sched.go @@ -131,7 +131,7 @@ func (s *GenericScheduler) process() (bool, error) { s.ctx = NewEvalContext(s.state, s.plan, s.logger) // Construct the placement stack - s.stack = NewGenericStack(s.batch, s.ctx, nil) + s.stack = NewGenericStack(s.batch, s.ctx) if s.job != nil { s.stack.SetJob(s.job) } diff --git a/scheduler/stack.go b/scheduler/stack.go index 9cfde8356..2cda626c4 100644 --- a/scheduler/stack.go +++ b/scheduler/stack.go @@ -48,7 +48,7 @@ type GenericStack struct { } // NewGenericStack constructs a stack used for selecting service placements -func NewGenericStack(batch bool, ctx Context, baseNodes []*structs.Node) *GenericStack { +func NewGenericStack(batch bool, ctx Context) *GenericStack { // Create a new stack s := &GenericStack{ batch: batch, @@ -58,7 +58,7 @@ func NewGenericStack(batch bool, ctx Context, baseNodes []*structs.Node) *Generi // Create the source iterator. We randomize the order we visit nodes // to reduce collisions between schedulers and to do a basic load // balancing across eligible nodes. - s.source = NewRandomIterator(ctx, baseNodes) + s.source = NewRandomIterator(ctx, nil) // Attach the job constraints. The job is filled in later. s.jobConstraint = NewConstraintIterator(ctx, s.source, nil) @@ -92,11 +92,6 @@ func NewGenericStack(batch bool, ctx Context, baseNodes []*structs.Node) *Generi // Select the node with the maximum score for placement s.maxScore = NewMaxScoreIterator(ctx, s.limit) - - // Set the nodes if given - if len(baseNodes) != 0 { - s.SetNodes(baseNodes) - } return s } @@ -169,13 +164,13 @@ type SystemStack struct { } // NewSystemStack constructs a stack used for selecting service placements -func NewSystemStack(ctx Context, baseNodes []*structs.Node) *SystemStack { +func NewSystemStack(ctx Context) *SystemStack { // Create a new stack s := &SystemStack{ctx: ctx} // Create the source iterator. We visit nodes in a linear order because we // have to evaluate on all nodes. - s.source = NewStaticIterator(ctx, baseNodes) + s.source = NewStaticIterator(ctx, nil) // Attach the job constraints. The job is filled in later. s.jobConstraint = NewConstraintIterator(ctx, s.source, nil) @@ -193,11 +188,6 @@ func NewSystemStack(ctx Context, baseNodes []*structs.Node) *SystemStack { // by a particular task group. Enable eviction as system jobs are high // priority. s.binPack = NewBinPackIterator(ctx, rankSource, true, 0) - - // Set the nodes if given - if len(baseNodes) != 0 { - s.SetNodes(baseNodes) - } return s } diff --git a/scheduler/stack_test.go b/scheduler/stack_test.go index d63306c88..7b4b8acb3 100644 --- a/scheduler/stack_test.go +++ b/scheduler/stack_test.go @@ -10,7 +10,7 @@ import ( func TestServiceStack_SetNodes(t *testing.T) { _, ctx := testContext(t) - stack := NewGenericStack(false, ctx, nil) + stack := NewGenericStack(false, ctx) nodes := []*structs.Node{ mock.Node(), @@ -37,7 +37,7 @@ func TestServiceStack_SetNodes(t *testing.T) { func TestServiceStack_SetJob(t *testing.T) { _, ctx := testContext(t) - stack := NewGenericStack(false, ctx, nil) + stack := NewGenericStack(false, ctx) job := mock.Job() stack.SetJob(job) @@ -55,7 +55,8 @@ func TestServiceStack_Select_Size(t *testing.T) { nodes := []*structs.Node{ mock.Node(), } - stack := NewGenericStack(false, ctx, nodes) + stack := NewGenericStack(false, ctx) + stack.SetNodes(nodes) job := mock.Job() stack.SetJob(job) @@ -85,7 +86,8 @@ func TestServiceStack_Select_MetricsReset(t *testing.T) { mock.Node(), mock.Node(), } - stack := NewGenericStack(false, ctx, nodes) + stack := NewGenericStack(false, ctx) + stack.SetNodes(nodes) job := mock.Job() stack.SetJob(job) @@ -120,7 +122,8 @@ func TestServiceStack_Select_DriverFilter(t *testing.T) { zero := nodes[0] zero.Attributes["driver.foo"] = "1" - stack := NewGenericStack(false, ctx, nodes) + stack := NewGenericStack(false, ctx) + stack.SetNodes(nodes) job := mock.Job() job.TaskGroups[0].Tasks[0].Driver = "foo" @@ -145,7 +148,8 @@ func TestServiceStack_Select_ConstraintFilter(t *testing.T) { zero := nodes[0] zero.Attributes["kernel.name"] = "freebsd" - stack := NewGenericStack(false, ctx, nodes) + stack := NewGenericStack(false, ctx) + stack.SetNodes(nodes) job := mock.Job() job.Constraints[0].RTarget = "freebsd" @@ -182,7 +186,8 @@ func TestServiceStack_Select_BinPack_Overflow(t *testing.T) { one := nodes[1] one.Reserved = one.Resources - stack := NewGenericStack(false, ctx, nodes) + stack := NewGenericStack(false, ctx) + stack.SetNodes(nodes) job := mock.Job() stack.SetJob(job) @@ -210,7 +215,7 @@ func TestServiceStack_Select_BinPack_Overflow(t *testing.T) { func TestSystemStack_SetNodes(t *testing.T) { _, ctx := testContext(t) - stack := NewSystemStack(ctx, nil) + stack := NewSystemStack(ctx) nodes := []*structs.Node{ mock.Node(), @@ -232,7 +237,7 @@ func TestSystemStack_SetNodes(t *testing.T) { func TestSystemStack_SetJob(t *testing.T) { _, ctx := testContext(t) - stack := NewSystemStack(ctx, nil) + stack := NewSystemStack(ctx) job := mock.Job() stack.SetJob(job) @@ -247,10 +252,9 @@ func TestSystemStack_SetJob(t *testing.T) { func TestSystemStack_Select_Size(t *testing.T) { _, ctx := testContext(t) - nodes := []*structs.Node{ - mock.Node(), - } - stack := NewSystemStack(ctx, nodes) + nodes := []*structs.Node{mock.Node()} + stack := NewSystemStack(ctx) + stack.SetNodes(nodes) job := mock.Job() stack.SetJob(job) @@ -280,7 +284,8 @@ func TestSystemStack_Select_MetricsReset(t *testing.T) { mock.Node(), mock.Node(), } - stack := NewSystemStack(ctx, nodes) + stack := NewSystemStack(ctx) + stack.SetNodes(nodes) job := mock.Job() stack.SetJob(job) @@ -314,7 +319,8 @@ func TestSystemStack_Select_DriverFilter(t *testing.T) { zero := nodes[0] zero.Attributes["driver.foo"] = "1" - stack := NewSystemStack(ctx, nodes) + stack := NewSystemStack(ctx) + stack.SetNodes(nodes) job := mock.Job() job.TaskGroups[0].Tasks[0].Driver = "foo" @@ -330,7 +336,8 @@ func TestSystemStack_Select_DriverFilter(t *testing.T) { } zero.Attributes["driver.foo"] = "0" - stack = NewSystemStack(ctx, nodes) + stack = NewSystemStack(ctx) + stack.SetNodes(nodes) stack.SetJob(job) node, _ = stack.Select(job.TaskGroups[0]) if node != nil { @@ -347,7 +354,8 @@ func TestSystemStack_Select_ConstraintFilter(t *testing.T) { zero := nodes[1] zero.Attributes["kernel.name"] = "freebsd" - stack := NewSystemStack(ctx, nodes) + stack := NewSystemStack(ctx) + stack.SetNodes(nodes) job := mock.Job() job.Constraints[0].RTarget = "freebsd" @@ -384,7 +392,8 @@ func TestSystemStack_Select_BinPack_Overflow(t *testing.T) { zero.Reserved = zero.Resources one := nodes[1] - stack := NewSystemStack(ctx, nodes) + stack := NewSystemStack(ctx) + stack.SetNodes(nodes) job := mock.Job() stack.SetJob(job) diff --git a/scheduler/system_sched.go b/scheduler/system_sched.go index 0dfded98b..d3f6fb27e 100644 --- a/scheduler/system_sched.go +++ b/scheduler/system_sched.go @@ -99,7 +99,7 @@ func (s *SystemScheduler) process() (bool, error) { s.ctx = NewEvalContext(s.state, s.plan, s.logger) // Construct the placement stack - s.stack = NewSystemStack(s.ctx, nil) + s.stack = NewSystemStack(s.ctx) if s.job != nil { s.stack.SetJob(s.job) } diff --git a/scheduler/util_test.go b/scheduler/util_test.go index 9f6c12cad..417737dca 100644 --- a/scheduler/util_test.go +++ b/scheduler/util_test.go @@ -288,18 +288,25 @@ func TestTaintedNodes(t *testing.T) { } func TestShuffleNodes(t *testing.T) { + // Use a large number of nodes to make the probability of shuffling to the + // original order very low. nodes := []*structs.Node{ mock.Node(), mock.Node(), mock.Node(), mock.Node(), mock.Node(), + mock.Node(), + mock.Node(), + mock.Node(), + mock.Node(), + mock.Node(), } orig := make([]*structs.Node, len(nodes)) copy(orig, nodes) shuffleNodes(nodes) if reflect.DeepEqual(nodes, orig) { - t.Fatalf("shoudl not match") + t.Fatalf("should not match") } } @@ -457,7 +464,7 @@ func TestInplaceUpdate_ChangedTaskGroup(t *testing.T) { tg.Tasks = append(tg.Tasks, task) updates := []allocTuple{{Alloc: alloc, TaskGroup: tg}} - stack := NewGenericStack(false, ctx, nil) + stack := NewGenericStack(false, ctx) // Do the inplace update. unplaced := inplaceUpdate(ctx, eval, job, stack, updates) @@ -502,7 +509,7 @@ func TestInplaceUpdate_NoMatch(t *testing.T) { tg.Tasks[0].Resources = resource updates := []allocTuple{{Alloc: alloc, TaskGroup: tg}} - stack := NewGenericStack(false, ctx, nil) + stack := NewGenericStack(false, ctx) // Do the inplace update. unplaced := inplaceUpdate(ctx, eval, job, stack, updates) @@ -547,7 +554,7 @@ func TestInplaceUpdate_Success(t *testing.T) { tg.Tasks[0].Resources = resource updates := []allocTuple{{Alloc: alloc, TaskGroup: tg}} - stack := NewGenericStack(false, ctx, nil) + stack := NewGenericStack(false, ctx) // Do the inplace update. unplaced := inplaceUpdate(ctx, eval, job, stack, updates)