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

Nectar Aggregate 95th emailed report broken #4791

Closed
iskandarbasman opened this issue Jun 1, 2022 · 10 comments
Closed

Nectar Aggregate 95th emailed report broken #4791

iskandarbasman opened this issue Jun 1, 2022 · 10 comments
Labels
bug Undesired behaviour confirmed Bug is confirm by dev team resolved A fixed issue
Milestone

Comments

@iskandarbasman
Copy link

Refer to #3733 & #3702

This is old reported bug for Nectar emailed reported two year ago.
Is there any road map to the fix ?

@iskandarbasman iskandarbasman added bug Undesired behaviour unverified Some days we don't have a clue labels Jun 1, 2022
@TheWitness
Copy link
Member

Can you show me an image of the Graph before Emailing and the RRDtool create syntax, and then an image of what shows up?

@iskandarbasman
Copy link
Author

Hi @TheWitness below was you previously mentioned in #3702
image

Issue still exists in cacti v1.2.21
Below is the side by side comparison of the Nectar report preview and what I received in email.
Bug shows the 95th value in the emailed report duplicating the values above it.

"RRDtool create syntax" <- How and where should I do this.

image

Nectar report configuration
image

image

@netniV
Copy link
Member

netniV commented Jun 13, 2022

The bug there appears to be simply that the total isn't being cleared if all the values were zero so it's picking up the previous one?

@TheWitness
Copy link
Member

@netniV, I think you may be right, @ronytomen had introduced a cache several years ago, and maybe the index on aggregates is gummed up.

The last number might simply be due to another sample towards the end increasing the nth ptile a smidge.

@TheWitness
Copy link
Member

Here is a test that I would like you to peform. In your setup edit the file lib/graph_variables.php and search for the word static. For each instance, comment out the static lines. Then, save the file and re-run your test. Let us know what comes back.

@iskandarbasman
Copy link
Author

Here is a test that I would like you to peform. In your setup edit the file lib/graph_variables.php and search for the word static. For each instance, comment out the static lines. Then, save the file and re-run your test. Let us know what comes back.

Wow.. I am seeing very positive results after commenting out the 3 instances of static $vstats = array(); from lib/graph_variables.php

It seems we have a fix for this issue.
No more bugged duplicated 95th percentile values in the nectar emailed report. (I will monitor further on my daily runs.)

Any side effect for commenting out those lines ?

@TheWitness
Copy link
Member

Not on any well behaved modern system.

TheWitness added a commit that referenced this issue Jun 14, 2022
Nectar Aggregate 95th emailed report broken
@TheWitness TheWitness added resolved A fixed issue confirmed Bug is confirm by dev team and removed unverified Some days we don't have a clue labels Jun 14, 2022
@TheWitness TheWitness added this to the v1.2.22 milestone Jun 14, 2022
@TheWitness
Copy link
Member

I've marked this resolved. Grab the latest and keep testing.

@netniV
Copy link
Member

netniV commented Jun 14, 2022

My only concern is about it slowing the reporting down further since the same data is no longer cached. But at least we have sorted the problem out.

@TheWitness
Copy link
Member

It won't be a material slowdown as reports can only be so large. Nor to the UI.

@github-actions github-actions bot locked and limited conversation to collaborators Nov 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Undesired behaviour confirmed Bug is confirm by dev team resolved A fixed issue
Projects
None yet
Development

No branches or pull requests

3 participants