Skip to content
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

Change max load from 5000 to 50000 #8769

Merged
merged 2 commits into from Jun 10, 2018

Conversation

Projects
None yet
4 participants
@isarandi
Copy link
Contributor

isarandi commented May 26, 2018

Perhaps a load value of 50000% seems excessive, however it can happen. (Example: a heavily loaded compute node with 48 CPU cores that produces loads of ~15000%, printed as 150 by tools like htop).

DO NOT DELETE THIS TEXT

Please note

Please read this information carefully. You can run ./scripts/pre-commit.php to check your code before submitting.

Testers

If you would like to test this pull request then please run: ./scripts/github-apply <pr_id>, i.e ./scripts/github-apply 5926

Change max load from 5000 to 50000
Perhaps a load value of 50000% seems excessive, however it can happen. (Example: a heavily loaded compute node with 48 CPU cores that produces loads of ~15000%, printed as 150 by tools htop).
@CLAassistant

This comment has been minimized.

Copy link

CLAassistant commented May 26, 2018

CLA assistant check
All committers have signed the CLA.

@murrant

This comment has been minimized.

Copy link
Member

murrant commented May 26, 2018

why set a max at all?

@isarandi

This comment has been minimized.

Copy link
Contributor Author

isarandi commented May 26, 2018

Honestly, I have no idea why there is even an option to set any collection limits for any monitored quantity. Values outside the range are simply thrown away, which perhaps makes sense if we suspect that the unreasonably high or low value is somehow due to misconfiguration and doesn't represent reality.

But as this example demonstrates, sometimes extreme values do occur.

In short, if it's possible in this place of the API to not set a max value, that would probably be a good idea.

@murrant

This comment has been minimized.

Copy link
Member

murrant commented May 26, 2018

yes, simple delete it from the function call. I think the only limit that makes sense here is minimum of 0.

@isarandi

This comment has been minimized.

Copy link
Contributor Author

isarandi commented Jun 8, 2018

Should I close this and submit a new pull request or can I somehow make an edit here (to remove the max value)?

@murrant

This comment has been minimized.

Copy link
Member

murrant commented Jun 8, 2018

You can go to the files changed tab and click the edit button.

Or update your branch, commit, and push it if using git on your computer.

@isarandi

This comment has been minimized.

Copy link
Contributor Author

isarandi commented Jun 8, 2018

Done!

@murrant

This comment has been minimized.

Copy link
Member

murrant commented Jun 10, 2018

Won't affect existing files.

@laf laf merged commit 78e3df4 into librenms:master Jun 10, 2018

3 checks passed

WIP ready for review
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
license/cla Contributor License Agreement is signed.
Details

@laf laf added the Bug 🐞 label Jun 10, 2018

@isarandi isarandi deleted the isarandi:patch-1 branch Jun 11, 2018

mattie47 added a commit to mattie47/librenms that referenced this pull request Jul 2, 2018

Change UCD max load from 5000 to 50000 (librenms#8769)
* Change max load from 5000 to 50000

Perhaps a load value of 50000% seems excessive, however it can happen. (Example: a heavily loaded compute node with 48 CPU cores that produces loads of ~15000%, printed as 150 by tools htop).

* Remove maximum value for load averages

@lock lock bot locked as resolved and limited conversation to collaborators Aug 10, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.