-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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 Stats Collector to not divide by zero #10492
Fix Stats Collector to not divide by zero #10492
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 4.20 #10492 +/- ##
============================================
+ Coverage 15.98% 16.00% +0.01%
- Complexity 13086 13105 +19
============================================
Files 5650 5651 +1
Lines 495756 495841 +85
Branches 60018 60045 +27
============================================
+ Hits 79261 79366 +105
+ Misses 407641 407614 -27
- Partials 8854 8861 +7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
@lucas-a-martins Could you please rebase this off 4.20 - I believe this could be an issue in 4.20 as well. |
c0e83d5
to
53f8296
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.
code lgtm.
@blueorangutan package |
@Pearl1594 a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 12642 |
After having a conversation with @DaanHoogland on this offline, I understand that reporting a 0 could imply that there were no queries made at all at a specific time, would may not be entirely correct. Having a -1 would indicate an erroneous stats collection for that iteration. |
I applied your suggestions by changing the value to |
@blueorangutan package |
@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 12682 |
@blueorangutan test keepEnv |
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
[SF] Trillian test result (tid-12608)
|
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.
code lgtm
the refresh fix works. I have no idea how to force the NPE situation (apart from changing the code/core in a debugger. @lucas-a-martins have you seen this happen without debugger? |
@DaanHoogland only one time and it wasn't in my environment. I don't know how to force it too |
@Pearl1594 , I am fine with merging this, in spite of the corner caaase not being verified. Normal functionality is still working.... |
Description
Currently, the Stats Collector can throw an arithmetic exception when trying to divide by zero if
interval = 0
.This PR fixes this issue by setting the
loadHistory.add
value to zero when theinterval
variable is equal to zero.Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Screenshots (if appropriate):
How Has This Been Tested?
In debug mode, I tested by setting the
interval
value to zero. Before the changes, an arithmetic exception was thrown. After the changes, I got no exception and could check that the value added toloadHistory
was equal to0
.