-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Automatically set GOMAXPROCS #1560
Automatically set GOMAXPROCS #1560
Conversation
@yurishkuro This is probably one you should answer, especially as it is using the Uber |
@rubenvp8510 did you test it on k8s? Does it use resource requests or limits? |
Codecov Report
@@ Coverage Diff @@
## master #1560 +/- ##
=======================================
Coverage 98.79% 98.79%
=======================================
Files 190 190
Lines 9063 9063
=======================================
Hits 8954 8954
Misses 85 85
Partials 24 24 Continue to review full report at Codecov.
|
@objectiser it makes sense, when we build collector internally we're also using this module to adjust GOMAXPROCS |
@pavolloffay Yes I did some tests and the library uses limits if they are defined. With: resources:
requests:
memory: "64Mi"
cpu: "250m"
limits:
memory: "128Mi"
cpu: "2000m" I got this: 2019/05/24 17:28:19 maxprocs: Updating GOMAXPROCS=2: determined from CPU quota @yurishkuro should I leave this as it is? Or should I add a flag to enable/disable this behaviour? |
Signed-off-by: Ruben Vargas <[email protected]>
4747239
to
597182f
Compare
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.
I think we should merge it. If someone needs this configurable in the future, they can create another issue.
Which problem is this PR solving?
Short description of the changes
Does we need to make this configurable? I mean enable/disable using flags? @pavolloffay @objectiser