-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Fix leaking go routine in docker stats fetching #3492
Conversation
@@ -84,6 +84,7 @@ func FetchStats(client *docker.Client) ([]Stat, error) { | |||
}() | |||
|
|||
wg.Wait() | |||
close(queue) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider swapping things around so that you can call wg.Done()
from the worker goroutines. This more closely aligns with the Fan-in, Fan-out examples here.
for _, container := range containers {
go func(container docker.APIContainers) {
defer wg.Done()
queue <- exportContainerStats(client, &container)
}(container)
}
go func() {
wg.Wait()
close(queue)
}()
// This will break after the queue has been drained and queue is closed.
for container := range queue {
containersList = append(containersList, container)
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And up on line 69, consider changing it to containersList = make([]Stat, 0, len(containers))
since we know ahead of time how much capacity the slice needs. This will reduce the number of allocations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This definitively makes the code much cleaner.
Go routine on line 79 never stopped as queue was not closed. Fixes elastic#3489
1efb268
to
0f758b4
Compare
Go routine on line 79 never stopped as queue was not closed. Fixes elastic#3489 (cherry picked from commit 3366bdb)
…ic#3514) Go routine on line 79 never stopped as queue was not closed. Fixes elastic#3489 (cherry picked from commit 4e615a7)
See #3489