Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Sum up TotalLoad over all CPUs and divide it by number of threads#669

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
LordHepipud merged 1 commit intomasterfromcpu_total_multiple_sockets
Mar 13, 2024

Conversation

@RincewindsHat
Copy link
Member

On uncommon system configurations (number of sockets > 1) the TotalLoad perfdata of Invoke-IcingaCheckCpu was to high due to it being just summed up over all sockets.

This patch changes this computation, the load weighted by the number of sockets is summed up and then divided by the total number of CPUs.

AlexMilotin reacted with thumbs up emoji
@cla-bot
Copy link

cla-botbot commentedOct 11, 2023

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors:Lorenz Kästle.
This is most likely caused by a git client misconfiguration; please make sure to:

  1. check if your git client is configured with an email to sign commitsgit config --list | grep email
  2. If not, set it up usinggit config --global user.email email@example.com
  3. Make sure that the git commit email is configured in your GitHub account settings, seehttps://github.com/settings/emails

@LordHepipud
Copy link
Collaborator

LordHepipud commentedNov 3, 2023
edited
Loading

Thank you for the PR. Wouldn't it be enough to simple divide the total load by the number of sockets?

$TotalCpuLoad/=$SocketList.Count;

At least in my tests I see no reason against that.

@LordHepipudLordHepipud added the BugThere is an issue present labelNov 3, 2023
@LordHepipudLordHepipud added this to thev1.11.1 milestoneNov 3, 2023
@RincewindsHat
Copy link
MemberAuthor

you could do that, but if I understand this correctly you would loose the weighting by number of threads/socket.
If you get two sockets, one with one Thread and one with four, there would be no balancing between them and the single one would weigh as much as the four others.

@LordHepipud
Copy link
Collaborator

I honestly don't see a problem with that. I agree, that calculating based on threads is more precise for the total load over all sockets. How ever, I doubt you will ever come across a system where the amount of threads a CPU provides is not identical to each other. You wouldn't build a server with two sockets, containing different CPU's and even on virtual machines you can only specify the number of sockets and amount of threads assigned in total.

@LordHepipudLordHepipud modified the milestones:v1.11.1,v1.12.0Nov 7, 2023
@RincewindsHat
Copy link
MemberAuthor

I would kindly disagree. This patch is the result of encountering multiple systems where, without this patch,Invoke-IcingaCheckCPU caused alarms due to excessively high (and wrong) numbers.

AlexMilotin reacted with thumbs up emoji

@lazyfrosch
Copy link

The main problem, is that the total over multiple sockets is summed and not averaged currently :)

@AlexMilotin
Copy link

What if we would do the divide in the Invoke-IcingaCheckCPU. Not fancy enough, but it would work.
You have the socket numbers collected from the metrics anyway, you just need to divide it before output the result.
image
And if where count > 1 to divide would be also fine, yet something divided by 1 is itself, so it would hurt to to exclude the if
image

@AlexMilotin
Copy link

Also not sure, going on assuming that if hyper-Threading is enabled dividing by thread count you might be end up and dividing against double the number of needed threads (logical processor)
image

@lazyfrosch
Copy link

@LordHepipud ping

AlexMilotin reacted with laugh emoji

@lazyfrosch
Copy link

ref/NC/809410

@LordHepipudLordHepipudforce-pushed thecpu_total_multiple_sockets branch 2 times, most recently from7755e40 to9697134CompareFebruary 26, 2024 11:42
@LordHepipudLordHepipud self-assigned thisFeb 26, 2024
@LordHepipudLordHepipudforce-pushed thecpu_total_multiple_sockets branch from9697134 to97134d7CompareMarch 13, 2024 09:59
@LordHepipudLordHepipud merged commit323acca intomasterMar 13, 2024
@LordHepipudLordHepipud deleted the cpu_total_multiple_sockets branchMarch 13, 2024 10:01
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@LordHepipudLordHepipudAwaiting requested review from LordHepipud

Labels

BugThere is an issue presentcla/signed

Projects

None yet

Milestone

v1.12.0

Development

Successfully merging this pull request may close these issues.

6 participants

@RincewindsHat@LordHepipud@lazyfrosch@AlexMilotin@Crited

[8]ページ先頭

©2009-2025 Movatter.jp