Skip to content

Lock KillAll.#2811

Merged
estesp merged 1 commit intocontainerd:masterfrom
Random-Liu:lock-kill-all
Nov 20, 2018
Merged

Lock KillAll.#2811
estesp merged 1 commit intocontainerd:masterfrom
Random-Liu:lock-kill-all

Conversation

@Random-Liu
Copy link
Copy Markdown
Member

@Random-Liu Random-Liu commented Nov 19, 2018

KillAll should be locked.

Previously, it was called in delete (process state locked) and checkProcesses (shim locked), both are locked.

However, recently we optimized the locks. checkProcesses doesn't hold a shim lock for process operations any more.

In release/1.2 and HEAD, KillAll is not called in delete any more, so we can simply hold a lock in KillAll directly.
In release/1.1, since KillAll is still called in delete (process state locked), we need a private killAll function without process state lock for delete, and a public KillAll function with lock for checkProcesses.

I'll chery-pick this into supported branches once LGTM.

Signed-off-by: Lantao Liu [email protected]

Signed-off-by: Lantao Liu <[email protected]>
@codecov-io
Copy link
Copy Markdown

codecov-io commented Nov 19, 2018

Codecov Report

Merging #2811 into master will decrease coverage by 3.67%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2811      +/-   ##
==========================================
- Coverage   47.41%   43.73%   -3.68%     
==========================================
  Files          91      100       +9     
  Lines        8411    10730    +2319     
==========================================
+ Hits         3988     4693     +705     
- Misses       3700     5307    +1607     
- Partials      723      730       +7
Flag Coverage Δ
#linux 47.41% <ø> (ø) ⬆️
#windows 40.89% <ø> (?)
Impacted Files Coverage Δ
snapshots/native/native.go 43.3% <0%> (-10%) ⬇️
metadata/snapshot.go 45.8% <0%> (-8.96%) ⬇️
archive/tar.go 43.13% <0%> (-6.87%) ⬇️
metadata/containers.go 47.97% <0%> (-6.62%) ⬇️
content/local/writer.go 57.84% <0%> (-6.36%) ⬇️
content/local/store.go 48.51% <0%> (-5.03%) ⬇️
remotes/docker/resolver.go 58.36% <0%> (-4.99%) ⬇️
archive/tar_opts.go 28.57% <0%> (-4.77%) ⬇️
archive/compression/compression.go 58.69% <0%> (-4.7%) ⬇️
metadata/images.go 58.46% <0%> (-4.7%) ⬇️
... and 56 more

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 f801661...7d91d63. Read the comment docs.

Copy link
Copy Markdown
Member

@estesp estesp left a comment

Choose a reason for hiding this comment

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

LGTM

@estesp estesp merged commit 1a62577 into containerd:master Nov 20, 2018
@Random-Liu Random-Liu deleted the lock-kill-all branch November 20, 2018 23:30
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.

5 participants