New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

scheduler: Tasks with a desired state < RUNNING should count #1980

Merged
merged 1 commit into from Feb 23, 2017

Conversation

Projects
None yet
4 participants
@aaronlehmann
Collaborator

aaronlehmann commented Feb 23, 2017

Currently, the scheduler counts tasks on each node with desired state of
RUNNING. This works fine for sequential updates, but in parallel
updates, the scheduler can assign tasks unfairly, because the ones it
already assigned may not have changed their desired state to RUNNING
yet. In an update with unlimited parallelism, all the tasks may end up
on the same node, even when there are multiple nodes that can accomodate
the tasks.

Fix this by considering all tasks with a desired state at or below
RUNNING to be active. For example, tasks at READY should be seen the
same way as RUNNING tasks by the scheduler - they were assigned to that
node and will eventually start running.

cc @jlhawn @aluzzardi @dongluochen

scheduler: Tasks with a desired state < RUNNING should count
Currently, the scheduler counts tasks on each node with desired state of
RUNNING. This works fine for sequential updates, but in parallel
updates, the scheduler can assign tasks unfairly, because the ones it
already assigned may not have changed their desired state to RUNNING
yet. In an update with unlimited parallelism, all the tasks may end up
on the same node, even when there are multiple nodes that can accomodate
the tasks.

Fix this by considering all tasks with a desired state at or below
RUNNING to be active. For example, tasks at READY should be seen the
same way as RUNNING tasks by the scheduler - they were assigned to that
node and will eventually start running.

Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>

@aaronlehmann aaronlehmann added this to the 1.13.2 milestone Feb 23, 2017

@codecov-io

This comment has been minimized.

Show comment
Hide comment
@codecov-io

codecov-io Feb 23, 2017

Codecov Report

Merging #1980 into master will decrease coverage by -0.11%.
The diff coverage is 72.72%.

@@            Coverage Diff             @@
##           master    #1980      +/-   ##
==========================================
- Coverage   53.69%   53.59%   -0.11%     
==========================================
  Files         109      109              
  Lines       18919    18919              
==========================================
- Hits        10158    10139      -19     
- Misses       7526     7543      +17     
- Partials     1235     1237       +2

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4dbc0a3...69bd653. Read the comment docs.

codecov-io commented Feb 23, 2017

Codecov Report

Merging #1980 into master will decrease coverage by -0.11%.
The diff coverage is 72.72%.

@@            Coverage Diff             @@
##           master    #1980      +/-   ##
==========================================
- Coverage   53.69%   53.59%   -0.11%     
==========================================
  Files         109      109              
  Lines       18919    18919              
==========================================
- Hits        10158    10139      -19     
- Misses       7526     7543      +17     
- Partials     1235     1237       +2

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4dbc0a3...69bd653. Read the comment docs.

@dongluochen

This comment has been minimized.

Show comment
Hide comment
@dongluochen

dongluochen Feb 23, 2017

Contributor

LGTM

Contributor

dongluochen commented Feb 23, 2017

LGTM

@aluzzardi

This comment has been minimized.

Show comment
Hide comment
@aluzzardi

aluzzardi Feb 23, 2017

Contributor

LGTM

Cherry pick?

Contributor

aluzzardi commented Feb 23, 2017

LGTM

Cherry pick?

@aluzzardi aluzzardi merged commit d29a857 into docker:master Feb 23, 2017

3 checks passed

ci/circleci Your tests passed on CircleCI!
Details
codecov/project 53.59% (target 0%)
Details
dco-signed All commits are signed

@aaronlehmann aaronlehmann deleted the aaronlehmann:bad-scheduling-decisions-parallel-update branch Feb 23, 2017

@aaronlehmann

This comment has been minimized.

Show comment
Hide comment
@aaronlehmann

aaronlehmann Feb 23, 2017

Collaborator

Cherry picked

Collaborator

aaronlehmann commented Feb 23, 2017

Cherry picked

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment