threads: restrict number of golang threads if not set #115
Conversation
|
For both this and its equiv over in the proxy, I ran the metrics density and footprint tests to check if there was visible (hopefully positive) impact. |
main.go
Outdated
| // code in shim.go | ||
| const maxthreads = 6 | ||
| if runtime.NumCPU() > maxthreads { | ||
| runtime.GOMAXPROCS(6) |
There was a problem hiding this comment.
runtime.GOMAXPROCS(maxthreads) maybe? :)
Same comment as for kata-containers/proxy#118 - might be worth moving the const to the top for better visibility.
There was a problem hiding this comment.
s/6/maxthreads/ indeed (oops ;-)).
The const - yeah, was not sure if better at the top or here.
I may also move the code to the actual main(), to fix the complexity CI niggle. It's not really clear to me why we have realMain() ;-)
There was a problem hiding this comment.
That function is used by this and other components to allow them to recover from panics and log the details.
If the max number of golang threads (GOMAXPROCS) is not set in the environment, then set it ourselves to a small set. This avoids us taking the default (the number of host cores) on large-core systems, which can end up with us having a lot of idle threads generated, consuming host PID space, and thus artificially restricting how many containers we can run in parallel. Fixes: kata-containers#114 Signed-off-by: Graham Whaley <graham.whaley@intel.com>
grahamwhaley
force-pushed
the
20181017_maxthreads
branch
from
ab40668 to
e57ab67
Compare
|
lgtm |
|
lgtm |
|
Keep up github! :) Let's make it a hat-trick... lgtm! |
|
lgtm |
Codecov Report
@@ Coverage Diff @@
## master #115 +/- ##
==========================================
- Coverage 51.25% 50.35% -0.91%
==========================================
Files 7 7
Lines 279 284 +5
==========================================
Hits 143 143
- Misses 124 129 +5
Partials 12 12 |
|
Hmm, all the metrics tests failed with the message "Failed to load JSON"... ? |
|
Hmm, oh. I'll go look. I made a change yesterday, but don't think that would have created that error.... |
|
/test |
|
initrd CI failed with what looked like a timeout, so I've nudged a rebuild. |
|
Slow week for PR reviews this week ... nudge nudge nudge.... |
|
metrics is broken right now, let's merge this anyway. |
If the max number of golang threads (GOMAXPROCS) is not set in
the environment, then set it ourselves to a small set, but
only if the host has more cpus than that max.
This avoids us taking the default (the number of host cores)
on large-core systems, which can end up with us having a lot of
idle threads generated, consuming host PID space, and thus
artificially restricting how many containers we can run
in parallel.
Fixes: #114
Signed-off-by: Graham Whaley graham.whaley@intel.com