fix(probe): Use a buffered chan to reduce the chance of losing events #3627
Conversation
probe/docker/registry.go
Outdated
| @@ -176,7 +176,8 @@ func (r *registry) listenForEvents() bool { | |||
| // Next, start listening for events. We do this before fetching | |||
| // the list of containers so we don't miss containers created | |||
| // after listing but before listening for events. | |||
| events := make(chan *docker_client.APIEvents) | |||
| // Use a buffered chan to reduce the chance of losing events | |||
| events := make(chan *docker_client.APIEvents, 32) | |||
There was a problem hiding this comment.
Where do the events get dropped? Is the sender using a non-blocking send?
There was a problem hiding this comment.
Unclear. Not in fsouza/go-dockerclient.
Maybe the other side of the socket.
There was a problem hiding this comment.
There was a problem hiding this comment.
Also // Each listener has 100 milliseconds to receive the event or it will be skipped.
There was a problem hiding this comment.
Is there any downside to making the buffer a lot larger (but not as large as to become a memory hog)?
There was a problem hiding this comment.
Like 1024? I don't see any issue.
Really we need to do a full sync periodically, so I see this change as an improvement but not a final fix.
There was a problem hiding this comment.
1024 sounds reasonable.
Pls file an issue for the periodic sync.
Event notifications from Docker will be dropped if not collected quickly enough; using a buffered chan reduces the chance of this happening.
bboreham
force-pushed
the
buffered-docker-events
branch
from
4862fb6 to
380dd83
Compare
Fixes #3576 (I know, we've been here before...)
Event notifications from Docker have been seen to get dropped; using a buffered chan seems to reduce the chance of this happening.