From 82889c432ee03b70eae2793b346bc565e9b7568f Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Fri, 21 Sep 2018 16:55:35 -0700 Subject: [PATCH 1/3] Denormalize jobs in plan and ignore resources of terminal allocs Denormalize jobs in AppendAllocs: AppendAlloc was originally only ever called for inplace upgrades and new allocations. Both these code paths would remove the job from the allocation. Now we use this to also add fields such as FollowupEvalID which did not normalize the job. This is only a performance enhancement. Ignore terminal allocs: Failed allocations are annotated with the followup Eval ID when one is created to replace the failed allocation. However, in the plan applier, when we check if allocations fit, these terminal allocations were not filtered. This could result in the plan being rejected if the node would be overcommited if the terminal allocations resources were considered. --- nomad/structs/funcs.go | 5 +++++ nomad/structs/structs.go | 4 ++++ nomad/worker.go | 2 +- 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/nomad/structs/funcs.go b/nomad/structs/funcs.go index 91298c97b45..5ace4bc8caa 100644 --- a/nomad/structs/funcs.go +++ b/nomad/structs/funcs.go @@ -111,6 +111,11 @@ func AllocsFit(node *Node, allocs []*Allocation, netIdx *NetworkIndex) (bool, st // For each alloc, add the resources for _, alloc := range allocs { + // Do not consider the resource impact of terminal allocations + if alloc.TerminalStatus() { + continue + } + if alloc.Resources != nil { if err := used.Add(alloc.Resources); err != nil { return false, "", nil, err diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 1588cc101ba..68fd8d529d2 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -7138,6 +7138,10 @@ func (p *Plan) PopUpdate(alloc *Allocation) { func (p *Plan) AppendAlloc(alloc *Allocation) { node := alloc.NodeID existing := p.NodeAllocation[node] + + // Normalize the job + alloc.Job = nil + p.NodeAllocation[node] = append(existing, alloc) } diff --git a/nomad/worker.go b/nomad/worker.go index b6bab0bbc94..911f4ee3a40 100644 --- a/nomad/worker.go +++ b/nomad/worker.go @@ -326,7 +326,7 @@ SUBMIT: } return nil, nil, err } else { - w.logger.Debug("submitted plan for evaluation", "plan_resp_index", resp.Index, "eval_id", plan.EvalID) + w.logger.Debug("submitted plan for evaluation", "eval_id", plan.EvalID) w.backoffReset() } From 06920ee46caa688e67fecc204f23ed432850e9f8 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Mon, 24 Sep 2018 13:27:41 -0700 Subject: [PATCH 2/3] Better comment on snapshotindex --- nomad/structs/structs.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 68fd8d529d2..d4ea11036c3 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -6882,8 +6882,11 @@ type Evaluation struct { LeaderACL string // SnapshotIndex is the Raft index of the snapshot used to process the - // evaluation. As such it will only be set once it has gone through the - // scheduler. + // evaluation. The index will either be set when it has gone through the + // scheduler or if a blocked evaluation is being created. The index is set + // in this case so we can determine if an early unblocking is required since + // capacity has changed since the evaluation was created. This can result in + // the SnapshotIndex being less than the CreateIndex. SnapshotIndex uint64 // Raft Indexes From 4c40d62f684e96c0ff20616d93044de681ad48d6 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Mon, 24 Sep 2018 13:59:01 -0700 Subject: [PATCH 3/3] test allocs fit --- nomad/structs/funcs_test.go | 85 +++++++++++++++++++++++++++++++++++++ 1 file changed, 85 insertions(+) diff --git a/nomad/structs/funcs_test.go b/nomad/structs/funcs_test.go index 7c0ba5dcab1..cbda393c64a 100644 --- a/nomad/structs/funcs_test.go +++ b/nomad/structs/funcs_test.go @@ -216,6 +216,91 @@ func TestAllocsFit(t *testing.T) { } +func TestAllocsFit_TerminalAlloc(t *testing.T) { + n := &Node{ + Resources: &Resources{ + CPU: 2000, + MemoryMB: 2048, + DiskMB: 10000, + IOPS: 100, + Networks: []*NetworkResource{ + { + Device: "eth0", + CIDR: "10.0.0.0/8", + MBits: 100, + }, + }, + }, + Reserved: &Resources{ + CPU: 1000, + MemoryMB: 1024, + DiskMB: 5000, + IOPS: 50, + Networks: []*NetworkResource{ + { + Device: "eth0", + IP: "10.0.0.1", + MBits: 50, + ReservedPorts: []Port{{"main", 80}}, + }, + }, + }, + } + + a1 := &Allocation{ + Resources: &Resources{ + CPU: 1000, + MemoryMB: 1024, + DiskMB: 5000, + IOPS: 50, + Networks: []*NetworkResource{ + { + Device: "eth0", + IP: "10.0.0.1", + MBits: 50, + ReservedPorts: []Port{{"main", 8000}}, + }, + }, + }, + } + + // Should fit one allocation + fit, _, used, err := AllocsFit(n, []*Allocation{a1}, nil) + if err != nil { + t.Fatalf("err: %v", err) + } + if !fit { + t.Fatalf("Bad") + } + + // Sanity check the used resources + if used.CPU != 2000 { + t.Fatalf("bad: %#v", used) + } + if used.MemoryMB != 2048 { + t.Fatalf("bad: %#v", used) + } + + // Should fit second allocation since it is terminal + a2 := a1.Copy() + a2.DesiredStatus = AllocDesiredStatusStop + fit, _, used, err = AllocsFit(n, []*Allocation{a1, a2}, nil) + if err != nil { + t.Fatalf("err: %v", err) + } + if !fit { + t.Fatalf("Bad") + } + + // Sanity check the used resources + if used.CPU != 2000 { + t.Fatalf("bad: %#v", used) + } + if used.MemoryMB != 2048 { + t.Fatalf("bad: %#v", used) + } +} + func TestScoreFit(t *testing.T) { node := &Node{} node.Resources = &Resources{