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

Graphs may select MAX instead of AVERAGE as consolidation function even if there is no item with MAX present. #3032

Closed
tertius1 opened this issue Oct 15, 2019 · 9 comments
Labels
bug Undesired behaviour resolved A fixed issue
Milestone

Comments

@tertius1
Copy link

Since one of the 1.2.x series, some graphs will always pull the MAX CF from a *.rrd file instead of the AVERAGE CF, even if there is no graph item with a MAX CF present.

Demo graph template (with all 4 CFs referenced):
image

The first graph item:
image

This results in such a graph:
image

See the DEFs: only one def is created with MAX, and all LINEs are drawn with that.
Expected is this (with all 4 CFs graphed):
image

I tried to track down the issue and got to lib/functions.php/function generate_graph_best_cf() in line 1837.
There is this: $best_cf = '3' in line 1850. This case is visited for my graph. The step of my data source is 30, so is the parameter $ds_step, and so is $first_step.

Unfortunately, the code isn't comprehensible. I don't understand the purpose of that strange construct. If I simply deactivate that part, the graph seems to look as expected:

if ($ds_step == $first_step && isset($avail_cf_functions[1])) {
$best_cf = '3';

changed to:

if (false && $ds_step == $first_step && isset($avail_cf_functions[1])) {
$best_cf = '3';

But as I said, I don't really understand that part of the code, I just patched "something" so it works somehow better than before. I don't know if it's the correct way.

Cacti version: 1.27 (happened with 1.2.5 as well)
CentOS 7.7
rrdtool version: rrdtool-1.4.8-9.el7.x86_64

@netniV
Copy link
Member

netniV commented Oct 15, 2019

Basically, it looks like you disabled the ability to select the CF so it will drop to the next part. I looked at both graphs side by side and I must be tired as i'm missing the difference between them really.

@tertius1
Copy link
Author

There are 4 graph items in the graph template. They all draw the same data source, but the 4 different Consolidation Functions within the data source: AVERAGE, MIN, MAX, LAST. In the first graph (also look at the rrdtool debugging output), only the data of the MAX consolidation function is pulled from the rrd file with the DEF: statement. All 4 lines are drawn with this data, so you see only one line.

In the second graph, where I mangled with the code, the graph generation is correct: all 4 consolidation functions are pulled from the *.rrd file, and 4 different lines are drawn. You see them as different shades of blue in the second graph screenshot. You will also see the 4 different DEF: statements in the rrdtool debugging output of the second graph screenshot, with the 4 different consolidation functions, and the 4 LINE1 drawing instructions use every 4 of them. This is how it should be.
The issue is in the cited php function - it makes cacti always pull the MAX function in this special case instead of the requested ones.

@cigamit
Copy link
Member

cigamit commented Oct 16, 2019

This was a recent change when you are inside the first consolidation function. Since there is no aggregation, it 'should' not make a difference, but now that I think about it, the Advanced Ping template relies on the numbering of the RRDtool 'def' names, so when viewing the first RRA in the RRDfile, the template might break. Can you confirm?

@tertius1
Copy link
Author

tertius1 commented Oct 17, 2019

As far as it works on my system, this doesn't matter. For debugging I created a simple graph template that only uses the Advanced Ping data source template, not the sophisticated advanced ping graph template. I use no custom cdef's. I simply connect the test graph to one of the data sources within the existing advanced ping data source template, and it exhibits the above behavior.

Edit: (I realized later you speak of the first RRA of the first data source, not of the first data source in the *.rrd, so I redid tests and edited the original text)

In the test graph, I changed the CF from AVERAGE every other CF, so a different RRA is required. No change in behavior: Cacti continues to pull the MAX consolidation only.

Then I changed the graph template to use a different data source in the first place. No change in behavior as well.

From my system's point of view, the issue is unrelated to the advanced ping data template. It happens with every data source. It also happens regardless if it the graph pulls the first rra or any other rra. For every graph, only the MAX consolidation function is pulled into the graph, distorting every graph. To reproduce, you can take any graph, and it is immediately fixed if I re-add my hack from above, although I don't really know what it actually fixes.
What is supposed to be different between the first RRA and any other RRA?

@cigamit
Copy link
Member

cigamit commented Oct 19, 2019

How can upload a screen show of your Data Profile Edit Screen for your Default Profile?

@tertius1
Copy link
Author

This is the default (5 minutes collection):
image

And this is the data profile that is used for the RRD I detected the issue with (30 seconds polling):
image

@frontierliu
Copy link
Contributor

frontierliu commented Nov 1, 2019

I found the same problem as tertius1 and I aggree tertius1. It would make a graph DEF: statement CF always be MAX . I can not define a datasource use cf=avg/last/min.

In the Previous version, such as 1.2.5 ,the code of functions.php Line 1844 is:

if ($ds_step < 300 && isset($avail_cf_functions[1])) {

I think it was in the old version CACTI,that the minimum poller interval was 300s,and if a graph define a datasource which step<300s,no matter avg/max/min/last, always equal to MAX.
But the new version Cacti support poller interval lower than 300s, such as 60s.
So I suggest remove this code totally, direct drop to next part. should be work fine.

@cigamit
Copy link
Member

cigamit commented Nov 9, 2019

I'm going to revert the change. In the mean time, I have to find why I thought that this was a good idea in the first place. So, should be rolled back shortly.

@cigamit cigamit added bug Undesired behaviour resolved A fixed issue labels Nov 9, 2019
cigamit added a commit that referenced this issue Nov 9, 2019
Graph always shows 'MAX' CF for LINE graph instead of AVERAGE/MIN/LAST
@cigamit
Copy link
Member

cigamit commented Nov 9, 2019

Resolved.

@cigamit cigamit closed this as completed Nov 9, 2019
@cigamit cigamit added this to the v1.2.8 milestone Nov 9, 2019
@netniV netniV changed the title Graph always shows 'MAX' CF for LINE graph instead of AVERAGE/MIN/LAST Graphs may select MAX instead of AVERAGE as consolidation function even if there is no item with MAX present. Dec 7, 2019
@github-actions github-actions bot locked and limited conversation to collaborators Jun 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Undesired behaviour resolved A fixed issue
Projects
None yet
Development

No branches or pull requests

4 participants