Skip to content
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

Add timeout for I/O waitgroups #3361

Merged
merged 1 commit into from
Jun 21, 2019

Conversation

crosbymichael
Copy link
Member

Closes #3286

This and a combination of a couple Docker changes are needed to fully
resolve the issue on the Docker side. However, this ensures that after
processes exit, we still leave some time for the I/O to fully flush
before closing. Without this timeout, the delete methods would block
forever.

Signed-off-by: Michael Crosby crosbymichael@gmail.com

Closes containerd#3286

This and a combination of a couple Docker changes are needed to fully
resolve the issue on the Docker side.  However, this ensures that after
processes exit, we still leave some time for the I/O to fully flush
before closing.  Without this timeout, the delete methods would block
forever.

Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
@theopenlab-ci
Copy link

theopenlab-ci bot commented Jun 20, 2019

Build succeeded.

@codecov-io
Copy link

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3361      +/-   ##
==========================================
- Coverage   49.01%   45.08%   -3.94%     
==========================================
  Files         102      113      +11     
  Lines        9886    12542    +2656     
==========================================
+ Hits         4846     5654     +808     
- Misses       4193     6032    +1839     
- Partials      847      856       +9
Flag Coverage Δ
#linux 49.01% <ø> (ø) ⬆️
#windows 40.26% <ø> (?)
Impacted Files Coverage Δ
snapshots/native/native.go 43.04% <0%> (-9.99%) ⬇️
cio/io.go 46.47% <0%> (-8.53%) ⬇️
metadata/snapshot.go 47.52% <0%> (-8.26%) ⬇️
archive/tar.go 43.79% <0%> (-7.07%) ⬇️
metadata/containers.go 47.97% <0%> (-6.62%) ⬇️
content/local/writer.go 58.65% <0%> (-5.55%) ⬇️
content/local/store.go 49.52% <0%> (-5.29%) ⬇️
metadata/images.go 57.57% <0%> (-4.99%) ⬇️
archive/tar_opts.go 28.57% <0%> (-4.77%) ⬇️
archive/compression/compression.go 58.69% <0%> (-4.7%) ⬇️
... and 63 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 574bde0...2450522. Read the comment docs.

Copy link
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

crosbymichael added a commit to crosbymichael/docker that referenced this pull request Jun 21, 2019
This is the second part to
containerd/containerd#3361 and will help process
delete not block forever when the process exists but the I/O was
inherited by a subprocess that lives on.

Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
Copy link
Member

@mxpv mxpv left a comment

Choose a reason for hiding this comment

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

LGTM

@crosbymichael crosbymichael merged commit b88362f into containerd:master Jun 21, 2019
@crosbymichael crosbymichael deleted the io-wait branch June 21, 2019 18:28
docker-jenkins pushed a commit to docker-archive/docker-ce that referenced this pull request Jul 5, 2019
This is the second part to
containerd/containerd#3361 and will help process
delete not block forever when the process exists but the I/O was
inherited by a subprocess that lives on.

Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
Upstream-commit: b5f28865efebb14c66d5580dfa7bf0634b5e3241
Component: engine
@fuweid fuweid mentioned this pull request Jul 26, 2019
thaJeztah pushed a commit to thaJeztah/docker that referenced this pull request Sep 20, 2019
This is the second part to
containerd/containerd#3361 and will help process
delete not block forever when the process exists but the I/O was
inherited by a subprocess that lives on.

Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
(cherry picked from commit b5f2886)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
docker-jenkins pushed a commit to docker-archive/docker-ce that referenced this pull request Sep 23, 2019
This is the second part to
containerd/containerd#3361 and will help process
delete not block forever when the process exists but the I/O was
inherited by a subprocess that lives on.

Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
(cherry picked from commit b5f28865efebb14c66d5580dfa7bf0634b5e3241)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Upstream-commit: 34b31d0ee06d1fcbf9b94b34b130fbeaf65a83a8
Component: engine
crosbymichael added a commit to crosbymichael/containerd that referenced this pull request Sep 26, 2019
Backport for containerd#3361

This and a combination of a couple Docker changes are needed to fully
resolve the issue on the Docker side.  However, this ensures that after
processes exit, we still leave some time for the I/O to fully flush
before closing.  Without this timeout, the delete methods would block
forever.

Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
burnMyDread pushed a commit to burnMyDread/moby that referenced this pull request Oct 21, 2019
This is the second part to
containerd/containerd#3361 and will help process
delete not block forever when the process exists but the I/O was
inherited by a subprocess that lives on.

Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
Signed-off-by: zach <Zachary.Joyner@linux.com>
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.

docker exec hang if earlier docker exec left a zombie process
4 participants