Skip to content

Increase reaper buffer size and non-blocking send#2748

Merged
crosbymichael merged 1 commit intocontainerd:masterfrom
crosbymichael:reaper-buffer
Oct 29, 2018
Merged

Increase reaper buffer size and non-blocking send#2748
crosbymichael merged 1 commit intocontainerd:masterfrom
crosbymichael:reaper-buffer

Conversation

@crosbymichael
Copy link
Member

Fixes #2709

This increases the buffer size for process exit subscribers. It also
implements a non-blocking send on the subscriber channel. It is better
to drop an exit even than it is to block a shim for one slow subscriber.

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

@estesp
Copy link
Member

estesp commented Oct 29, 2018

what about v2 reaper?

@crosbymichael
Copy link
Member Author

@estesp good catch

Fixes containerd#2709

This increases the buffer size for process exit subscribers. It also
implements a non-blocking send on the subscriber channel.  It is better
to drop an exit even than it is to block a shim for one slow subscriber.

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

codecov-io commented Oct 29, 2018

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2748      +/-   ##
==========================================
- 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 35a32a8...232a063. 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

@estesp estesp added this to the 1.2.1 milestone Oct 29, 2018
@crosbymichael crosbymichael merged commit 8afcade into containerd:master Oct 29, 2018
@crosbymichael crosbymichael deleted the reaper-buffer branch October 29, 2018 21:49
Random-Liu added a commit to Random-Liu/containerd that referenced this pull request Nov 8, 2018
Signed-off-by: Lantao Liu <lantaol@google.com>
crosbymichael added a commit that referenced this pull request Nov 9, 2018
Partially revert the event discard change in #2748.
estesp pushed a commit to estesp/containerd that referenced this pull request Nov 11, 2018
Signed-off-by: Lantao Liu <lantaol@google.com>
Signed-off-by: Phil Estes <estesp@linux.vnet.ibm.com>
estesp pushed a commit to estesp/containerd that referenced this pull request Nov 16, 2018
Signed-off-by: Lantao Liu <lantaol@google.com>
Signed-off-by: Phil Estes <estesp@linux.vnet.ibm.com>
crosbymichael added a commit to crosbymichael/containerd that referenced this pull request Nov 16, 2018
Partial revert of containerd#2748

Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
estesp pushed a commit to estesp/containerd that referenced this pull request Nov 17, 2018
Partial revert of containerd#2748

Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
Signed-off-by: Phil Estes <estesp@linux.vnet.ibm.com>
@keloyang keloyang mentioned this pull request Aug 14, 2019
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