Skip to content

Requeue events in shim#3193

Closed
crosbymichael wants to merge 1 commit intocontainerd:masterfrom
crosbymichael:ttrpc
Closed

Requeue events in shim#3193
crosbymichael wants to merge 1 commit intocontainerd:masterfrom
crosbymichael:ttrpc

Conversation

@crosbymichael
Copy link
Copy Markdown
Member

@crosbymichael crosbymichael commented Apr 8, 2019

This also updates some of the event publishing to make things parallel
and fast path finding the process on exits

Ref #3177

Signed-off-by: Michael Crosby [email protected]

This also updates some of the event publishing to make things parallel
and fast path finding the process on exits

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

codecov-io commented Apr 8, 2019

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff            @@
##           master   #3193     +/-   ##
========================================
- Coverage    45.2%   40.6%   -4.6%     
========================================
  Files         111      74     -37     
  Lines       11982    9906   -2076     
========================================
- Hits         5416    4022   -1394     
+ Misses       5732    5319    -413     
+ Partials      834     565    -269
Flag Coverage Δ
#linux ?
#windows 40.6% <ø> (+0.02%) ⬆️
Impacted Files Coverage Δ
snapshots/native/native.go 1.79% <0%> (-41.26%) ⬇️
archive/tar.go 16.99% <0%> (-26.8%) ⬇️
metadata/snapshot.go 21.53% <0%> (-24.28%) ⬇️
cio/io.go 1.52% <0%> (-21.38%) ⬇️
content/local/writer.go 56.86% <0%> (-0.99%) ⬇️
oci/spec_opts.go 30.33% <0%> (-0.25%) ⬇️
mount/temp_unix.go
sys/reaper_linux.go
services/server/server_linux.go
sys/env.go
... and 33 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 4d313c0...bce7145. Read the comment docs.

Copy link
Copy Markdown
Member

@Random-Liu Random-Liu left a comment

Choose a reason for hiding this comment

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

LGTM

go func() {
// short pause before requeue of the event
time.Sleep(100 * time.Millisecond)
s.events <- e
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Event may be out of order. It is fine, because we have timestamp on event, but we may want to keep this in mind.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we have any documented guarantees? Would it be useful if events were blocked on retry just for the same containerid?

@dmcgowan
Copy link
Copy Markdown
Member

LGTM

@crosbymichael
Copy link
Copy Markdown
Member Author

I want #3204 before this.

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