Skip to content

address unassigned task leak when service is removed (v17.06 backport)#2708

Merged
anshulpundir merged 1 commit into
moby:bump_v17.06from
dani-docker:task_leak-v17.06
Jul 17, 2018
Merged

address unassigned task leak when service is removed (v17.06 backport)#2708
anshulpundir merged 1 commit into
moby:bump_v17.06from
dani-docker:task_leak-v17.06

Conversation

@dani-docker
Copy link
Copy Markdown
Contributor

Signed-off-by: Dani Louca dani.louca@docker.com

- What I did
BACKPORT v17.06
If a task is not yet assigned (for ex: noSuitableNode ) and its service has been removed, the task stays in the unassignedTasks map until a new leader election.

- How I did it

Before re-adding the task to unassignedTasks map, the fix checks if the task has a valid service in the store.

- How to test it

steps to repro:

  • set daemon.log to debug
  • create a service with a node constraint that does not exist:
    ex: docker service create -d --constraint 'node.labels.type == queue' alpine sleep 10000
  • watch the logs, you should see an error
    no suitable node available for task
  • delete the service and invoke the tick() by creating another service; watch the log and notice the same warning taskID in pops up after service is deleted

- Description for the changelog

Avoid a leak when a service with unassigned tasks is deleted

@GordonTheTurtle
Copy link
Copy Markdown

Please sign your commits following these rules:
https://github.com/moby/moby/blob/master/CONTRIBUTING.md#sign-your-work
The easiest way to do this is to amend the last commit:

$ git clone -b "task_leak-v17.06" git@github.com:dani-docker/swarmkit.git somewhere
$ cd somewhere
$ git commit --amend -s --no-edit
$ git push -f

Amending updates the existing PR. You DO NOT need to open a new one.

@codecov
Copy link
Copy Markdown

codecov Bot commented Jul 17, 2018

Codecov Report

Merging #2708 into bump_v17.06 will increase coverage by 3.75%.
The diff coverage is 100%.

@@               Coverage Diff               @@
##           bump_v17.06    #2708      +/-   ##
===============================================
+ Coverage        57.72%   61.47%   +3.75%     
===============================================
  Files               10      121     +111     
  Lines             1230    20203   +18973     
===============================================
+ Hits               710    12420   +11710     
- Misses             451     6423    +5972     
- Partials            69     1360    +1291

Signed-off-by: Dani Louca <dani.louca@docker.com>
@anshulpundir anshulpundir merged commit 0f566a0 into moby:bump_v17.06 Jul 17, 2018
@thaJeztah
Copy link
Copy Markdown
Member

@dani-docker was this cherry-picked or a different implementation as on master? If you did cherry-pick, could you use the -x option for future back ports? That makes it include a reference to the original commit in the commit-message.

@thaJeztah
Copy link
Copy Markdown
Member

For future reference; PR against master was
#2706

@dani-docker
Copy link
Copy Markdown
Contributor Author

dani-docker commented Jul 17, 2018

@thaJeztah was cherry picked but had to resolve conflicts

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants