-
Notifications
You must be signed in to change notification settings - Fork 13
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(metrics): Prevent NaNs in sampling from occuring #270
Conversation
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.
The commit might fix the issue, but the reasoning and commit message need to be sound. A division-by-zero by itself doesn't produce NaN values. To get a Nan here, diff_value
must be zero, as a product of plus or minus infinity, and zero would give NaN.
cbda419
to
06f7025
Compare
I don't want to be that picky, but the commit message could need some touches still. But keep that aside for a moment. What I'm wondering is this:
I don't understand it. Another thing, does the situation occur that |
A bug for which there is zero information of it ever existing, neither in kernel changelogs, Robert and Thomas' knowledge and even the ancient scriptures of the lo2s SVN repository. During trial runs of lo2s, it also did not occur.
Zero, I think. I don't see a case where a metric that was running 0% of the time during that sample interval should produce something else than |
Is anything blocking this? I kinda need it :/ |
someone should have a final look at the explanatory comment I have included in counter_buffer.hpp to make sure that it doesn't contain obvious bullshit. Other than that I think we can ship it. |
6f1eecf
to
b265e55
Compare
A code path in the metric readout code could lead to the generation of a metric value of "NaN" (see counter_buffer.hpp explanatory comment for more details). As metrics are recorded in an accumulated fashion, a NaN metric value will break the metric for the rest of the recording. This commit deletes that code path, as there is no record of the bug it tries to fix ever existing. This commit further improves the resilence of the metric readout code by introducing further checks for values of diff_running/enabled/value that can generate a NaN.
A code path could lead to a NaN if diff_running is 0 as this would result in a division-by-zero.
This commit solves this problem by just deleting that code path, as there is no evidence that the bug it addresses has existed in recent times, if ever.
This fixes #267