Skip to content

Commit

Permalink
scheduler: Fix accounting when task ends up in multiple groups
Browse files Browse the repository at this point in the history
Tasks are split between task groups based on common specs. This allows
nodes to only be ranked once per group, not once per task.

This logic doesn't work correctly because maps are marshalled in a
random order. Currently, the same task can end up in a multiple groups
(say, if it's updated multiple times, and the marshalling ends up being
different). Add a check to make sure we don't try to schedule the same
task twice within the same batch, which can cause resource accounting to
be wrong.

Note this doesn't fix the brokenness of task spec deduplication based on
marshalling the protobuf. This is a fix for the symptom that can be
backported, and I'm going to replace the marshalling stuff in a
different PR.

Signed-off-by: Aaron Lehmann <[email protected]>
  • Loading branch information
aaronlehmann committed Mar 13, 2017
1 parent 2b1b24b commit 281c31e
Showing 1 changed file with 6 additions and 0 deletions.
6 changes: 6 additions & 0 deletions manager/scheduler/scheduler.go
Original file line number Diff line number Diff line change
Expand Up @@ -602,6 +602,12 @@ func (s *Scheduler) scheduleNTasksOnNodes(ctx context.Context, n int, taskGroup
nodeIter := 0
nodeCount := len(nodes)
for taskID, t := range taskGroup {
// Skip tasks which were already scheduled because they ended
// up in two groups at once.
if _, exists := schedulingDecisions[taskID]; exists {
continue
}

node := &nodes[nodeIter%nodeCount]

log.G(ctx).WithField("task.id", t.ID).Debugf("assigning to node %s", node.ID)
Expand Down

0 comments on commit 281c31e

Please sign in to comment.