Skip to content

Make scheduler respect resource reservations#274

Merged
aaronlehmann merged 4 commits into
moby:masterfrom
aaronlehmann:scheduler-constraints
Apr 14, 2016
Merged

Make scheduler respect resource reservations#274
aaronlehmann merged 4 commits into
moby:masterfrom
aaronlehmann:scheduler-constraints

Conversation

@aaronlehmann
Copy link
Copy Markdown
Collaborator

Based on initial work by @aluzzardi:

  • Add filter interface and pipeline concept.
  • Turn nodeHeapItem into the exported type NodeInfo so it can be used to expose additional metadata about nodes to filters.
  • Add an AvailableResources field to NodeInfo. Keep it up to date in the scheduler loop.
  • Merge filter package into scheduler so that it can use the NodeInfo type without creating a circular dependency.
  • Hook up Pipeline to the scheduler so that the readiness check and resource availability check run against each scheduling candidate.

PTAL

aluzzardi and others added 3 commits April 4, 2016 19:13
Signed-off-by: Andrea Luzzardi <aluzzardi@gmail.com>
Signed-off-by: Andrea Luzzardi <aluzzardi@gmail.com>
This is in preparation for adding additional metadata to this type and
using it in the filtering pipeline.

Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>
@docker-codecov-bot
Copy link
Copy Markdown

Current coverage is 58.26%

Merging #274 into master will increase coverage by +0.94% as of c41b567

@@            master    #274   diff @@
======================================
  Files           45      48     +3
  Stmts         6102    5937   -165
  Branches       905     881    -24
  Methods          0       0       
======================================
- Hit           3498    3459    -39
+ Partial        497     473    -24
+ Missed        2107    2005   -102

Review entire Coverage Diff as of c41b567


Uncovered Suggestions

  1. +0.38% via agent/session.go#187...209
  2. +0.30% via ...ager/state/memory.go#279...296
  3. +0.30% via agent/agent.go#542...559
  4. See 7 more...

Powered by Codecov. Updated on successful CI builds.

Comment thread manager/scheduler/filter.go Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like we have this conversation every time, but isn't it cheaper to just pass a pointer rather than copying NodeInfo? :)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll change this function to take *NodeInfo.

Fix filter pipeline to consider available resources.

Hook up filter pipeline to scheduler.

Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>
@aaronlehmann aaronlehmann force-pushed the scheduler-constraints branch from 5f7ece9 to c41b567 Compare April 11, 2016 20:44
@aaronlehmann
Copy link
Copy Markdown
Collaborator Author

@aluzzardi: Updated, PTAL

@aluzzardi
Copy link
Copy Markdown
Member

Merge filter package into scheduler so that it can use the NodeInfo type without creating a circular dependency.

In v1 we had to move our NodeInfo equivalent into a node sub package to avoid the circular dependency.

Let's keep it this way though - we may re-assess once the scheduler package grows

@aaronlehmann
Copy link
Copy Markdown
Collaborator Author

@aluzzardi: Is there anything else I should address before merging this?

I saw you had a comment about updateTask. I'd prefer to keep the current suboptimal implementation for now, since any optimizations (including the possibility of totally removing updateTask) would be affected by the task lifecycle discussion.

@aluzzardi
Copy link
Copy Markdown
Member

LGTM

Notes for later (don't hold back the merge):

We should update this later to use state timestamps that will be added by #320 (/cc @stevvooe). It will be useful to know "this task is pending since ".

Another cool feature would be to make use of the state's message: "this task is pending since because: there are not enough resources in the cluster".

We had a way in v1 filters to explain why filters didn't pass (e.g. constraints not met, resources not available) however it turns out the design of that feature was flawed. @dongluochen and I had an idea on how to use filter bitmaps to figure out which combination of filters were making the task un-schedulable.

Not urgent, we should probably put something into our "icebox"

@aaronlehmann aaronlehmann merged commit 483e775 into moby:master Apr 14, 2016
@aaronlehmann aaronlehmann deleted the scheduler-constraints branch April 14, 2016 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants