Skip to content

[release/1.2] Add timeout and cancel to shim fifo open#2878

Merged
dmcgowan merged 1 commit intocontainerd:release/1.2from
andrewhsu:backport-1.2-fifo-timeout
Dec 14, 2018
Merged

[release/1.2] Add timeout and cancel to shim fifo open#2878
dmcgowan merged 1 commit intocontainerd:release/1.2from
andrewhsu:backport-1.2-fifo-timeout

Conversation

@andrewhsu
Copy link
Copy Markdown

@andrewhsu andrewhsu commented Dec 14, 2018

Backport of #2877

$ git cherry-pick -s -x a2a4241
[backport-1.2-fifo-timeout 18f57e20] Add timeout and cancel to shim fifo open
 Author: Michael Crosby <crosbymichael@gmail.com>
 Date: Thu Dec 13 14:43:41 2018 -0500
 2 files changed, 10 insertions(+)

There is still a special case where the client side fails to open or
load causes things to be slow and the shim can lock up when this
happens. This adds a timeout to the context for this case to abort fifo
creation.

Signed-off-by: Michael Crosby crosbymichael@gmail.com
(cherry picked from commit a2a4241)
Signed-off-by: Andrew Hsu andrewhsu@docker.com

@crosbymichael @andrewhsu
Add timeout and cancel to shim fifo open
18f57e2 
There is still a special case where the client side fails to open or
load causes things to be slow and the shim can lock up when this
happens.  This adds a timeout to the context for this case to abort fifo
creation.

Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
(cherry picked from commit a2a4241)
Signed-off-by: Andrew Hsu <andrewhsu@docker.com>
@andrewhsu
Copy link
Copy Markdown
Author

andrewhsu commented Dec 14, 2018

I can update this PR if the original patch in #2877 changes before merge.

@thaJeztah
Copy link
Copy Markdown
Member

thaJeztah commented Dec 14, 2018

Fix for failing CI: #2881

@codecov-io
Copy link
Copy Markdown

codecov-io commented Dec 14, 2018 •
edited
Loading

Codecov Report

Merging #2878 into release/1.2 will decrease coverage by 3.67%.
The diff coverage is n/a.

Impacted file tree graph

@@               Coverage Diff               @@
##           release/1.2    #2878      +/-   ##
===============================================
- Coverage        47.41%   43.74%   -3.68%     
===============================================
  Files               91      100       +9     
  Lines             8410    10728    +2318     
===============================================
+ Hits              3988     4693     +705     
- Misses            3699     5305    +1606     
- Partials           723      730       +7
Flag Coverage Δ
#linux 47.41% <ø> (ø) ⬆️
#windows 40.9% <ø> (?)
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 9b32062...18f57e2. Read the comment docs.

1 similar comment
@codecov-io
Copy link
Copy Markdown

codecov-io commented Dec 14, 2018

Codecov Report

Merging #2878 into release/1.2 will decrease coverage by 3.67%.
The diff coverage is n/a.

Impacted file tree graph

@@               Coverage Diff               @@
##           release/1.2    #2878      +/-   ##
===============================================
- Coverage        47.41%   43.74%   -3.68%     
===============================================
  Files               91      100       +9     
  Lines             8410    10728    +2318     
===============================================
+ Hits              3988     4693     +705     
- Misses            3699     5305    +1606     
- Partials           723      730       +7
Flag Coverage Δ
#linux 47.41% <ø> (ø) ⬆️
#windows 40.9% <ø> (?)
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 9b32062...18f57e2. Read the comment docs.

@crosbymichael
Copy link
Copy Markdown
Member

crosbymichael commented Dec 14, 2018

LGTM

2 similar comments
@ehazlett
Copy link
Copy Markdown
Member

ehazlett commented Dec 14, 2018

LGTM

@dmcgowan
Copy link
Copy Markdown
Member

dmcgowan commented Dec 14, 2018

LGTM

@dmcgowan dmcgowan merged commit aa5e000 into containerd:release/1.2 Dec 14, 2018
@andrewhsu andrewhsu deleted the backport-1.2-fifo-timeout branch December 14, 2018 20:03
@thaJeztah thaJeztah mentioned this pull request Jan 10, 2019
Open
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@andrewhsu @thaJeztah @codecov-io @crosbymichael @ehazlett @dmcgowan