From cab1a8da95fd9aa55a9cde091e6e32864da4e3a1 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Thu, 9 Mar 2017 21:36:27 -0800 Subject: [PATCH] Feedback addressed --- scheduler/feasible.go | 56 +++++++++++++++++++++------------------- scheduler/propertyset.go | 51 +++++++++++++++++------------------- 2 files changed, 54 insertions(+), 53 deletions(-) diff --git a/scheduler/feasible.go b/scheduler/feasible.go index 16e5e282f..bd60fc2ea 100644 --- a/scheduler/feasible.go +++ b/scheduler/feasible.go @@ -268,6 +268,19 @@ func NewDistinctPropertyIterator(ctx Context, source FeasibleIterator) *Distinct func (iter *DistinctPropertyIterator) SetTaskGroup(tg *structs.TaskGroup) { iter.tg = tg + // Build the property set at the taskgroup level + if _, ok := iter.groupPropertySets[tg.Name]; !ok { + for _, c := range tg.Constraints { + if c.Operand != structs.ConstraintDistinctProperty { + continue + } + + pset := NewPropertySet(iter.ctx, iter.job) + pset.SetTGConstraint(c, tg.Name) + iter.groupPropertySets[tg.Name] = append(iter.groupPropertySets[tg.Name], pset) + } + } + // Check if there is a distinct property iter.hasDistinctPropertyConstraints = len(iter.jobPropertySets) != 0 || len(iter.groupPropertySets[tg.Name]) != 0 } @@ -275,7 +288,7 @@ func (iter *DistinctPropertyIterator) SetTaskGroup(tg *structs.TaskGroup) { func (iter *DistinctPropertyIterator) SetJob(job *structs.Job) { iter.job = job - // Build the property sets + // Build the property set at the job level for _, c := range job.Constraints { if c.Operand != structs.ConstraintDistinctProperty { continue @@ -285,22 +298,9 @@ func (iter *DistinctPropertyIterator) SetJob(job *structs.Job) { pset.SetJobConstraint(c) iter.jobPropertySets = append(iter.jobPropertySets, pset) } - - for _, tg := range job.TaskGroups { - for _, c := range tg.Constraints { - if c.Operand != structs.ConstraintDistinctProperty { - continue - } - - pset := NewPropertySet(iter.ctx, job) - pset.SetTGConstraint(c, tg.Name) - iter.groupPropertySets[tg.Name] = append(iter.groupPropertySets[tg.Name], pset) - } - } } func (iter *DistinctPropertyIterator) Next() *structs.Node { -OUTER: for { // Get the next option from the source option := iter.source.Next() @@ -311,24 +311,28 @@ OUTER: } // Check if the constraints are met - for _, ps := range iter.jobPropertySets { - if satisfies, reason := ps.SatisfiesDistinctProperties(option, iter.tg.Name); !satisfies { - iter.ctx.Metrics().FilterNode(option, reason) - continue OUTER - } - } - - for _, ps := range iter.groupPropertySets[iter.tg.Name] { - if satisfies, reason := ps.SatisfiesDistinctProperties(option, iter.tg.Name); !satisfies { - iter.ctx.Metrics().FilterNode(option, reason) - continue OUTER - } + if !iter.satisfiesProperties(option, iter.jobPropertySets) || + !iter.satisfiesProperties(option, iter.groupPropertySets[iter.tg.Name]) { + continue } return option } } +// satisfiesProperties returns whether the option satisfies the set of +// properties. If not it will be filtered. +func (iter *DistinctPropertyIterator) satisfiesProperties(option *structs.Node, set []*propertySet) bool { + for _, ps := range set { + if satisfies, reason := ps.SatisfiesDistinctProperties(option, iter.tg.Name); !satisfies { + iter.ctx.Metrics().FilterNode(option, reason) + return false + } + } + + return true +} + func (iter *DistinctPropertyIterator) Reset() { iter.source.Reset() diff --git a/scheduler/propertyset.go b/scheduler/propertyset.go index 5e42b5eb7..aa4a0eab1 100644 --- a/scheduler/propertyset.go +++ b/scheduler/propertyset.go @@ -94,14 +94,8 @@ func (p *propertySet) populateExisting(constraint *structs.Constraint) { return } - for _, alloc := range allocs { - nProperty, ok := p.getProperty(nodes[alloc.NodeID], constraint.LTarget) - if !ok { - continue - } - - p.existingValues[nProperty] = struct{}{} - } + // Build existing properties map + p.populateProperties(allocs, nodes, p.existingValues) } // PopulateProposed populates the proposed values and recomputes any cleared @@ -139,26 +133,14 @@ func (p *propertySet) PopulateProposed() { } // Populate the cleared values - for _, alloc := range stopping { - nProperty, ok := p.getProperty(nodes[alloc.NodeID], p.constraint.LTarget) - if !ok { - continue - } - - p.clearedValues[nProperty] = struct{}{} - } + p.populateProperties(stopping, nodes, p.clearedValues) // Populate the proposed values - for _, alloc := range proposed { - nProperty, ok := p.getProperty(nodes[alloc.NodeID], p.constraint.LTarget) - if !ok { - continue - } + p.populateProperties(proposed, nodes, p.proposedValues) - p.proposedValues[nProperty] = struct{}{} - - // If it was cleared, it is now being used - delete(p.clearedValues, nProperty) + // Remove any cleared value that is now being used by the proposed allocs + for value := range p.proposedValues { + delete(p.clearedValues, value) } } @@ -173,7 +155,7 @@ func (p *propertySet) SatisfiesDistinctProperties(option *structs.Node, tg strin } // Get the nodes property value - nValue, ok := p.getProperty(option, p.constraint.LTarget) + nValue, ok := getProperty(option, p.constraint.LTarget) if !ok { return false, fmt.Sprintf("missing property %q", p.constraint.LTarget) } @@ -246,8 +228,23 @@ func (p *propertySet) buildNodeMap(allocs []*structs.Allocation) (map[string]*st return nodes, nil } +// populateProperties goes through all allocations and builds up the used +// properties from the nodes storing the results in the passed properties map. +func (p *propertySet) populateProperties(allocs []*structs.Allocation, nodes map[string]*structs.Node, + properties map[string]struct{}) { + + for _, alloc := range allocs { + nProperty, ok := getProperty(nodes[alloc.NodeID], p.constraint.LTarget) + if !ok { + continue + } + + properties[nProperty] = struct{}{} + } +} + // getProperty is used to lookup the property value on the node -func (p *propertySet) getProperty(n *structs.Node, property string) (string, bool) { +func getProperty(n *structs.Node, property string) (string, bool) { if n == nil || property == "" { return "", false }