*all_max_peak* percentile calculations incorrect

[ronytomen]

Original issue reported: http://bugs.cacti.net/view.php?id=2316

Description As far as I checked and understood the source code the calculations for 'all_max_peak' are wrong currently.

Steps To Reproduce The calculations are wrong. Reproduce by using them. See "Additional Information" of this bug for more details. In case someone argues for having examples we might work out something, but this would involve crafting very special data into a RRD file or view a very special time interval for given RRD data.

Additional Information I do not have example RRD files ready. The two methods do have different results for rare resp. special values only. I found tt difficult to verify the correctness of the calulations by looking at an RRDTool/Cacti graph. I did use the following definitions to print line and number for the percentiles:

• Item # 10 HRULE: |95:bits:0:max:2|
• Item # 11 COMMENT: |95:bits:6:max:2| mbit p95( max(traffic_in,traffic_out) )
• Item # 12 HRULE: |95:bits:0:all_max_peak:2|
• Item # 13 COMMENT: |95:bits:6:all_max_peak:2| mbit max( p95(traffic_in),p95(traffic_out) )

Using this you can see wether the calculations differ or not for a given graph.

[ronytomen]

Additional information from original reported issue:

Hello,

as far as I checked and understood the source code the calculations for 'all_max_peak' are wrong currently.

For type 'max' as well as 'all_max_peak' function nth_percentile() is called with one $local_data_id, returning a nth_percentile for the given datasource as well as a calculated field 'nth_percentile_maximum'.

When using 'max', this field 'nth_percentile_maximum' is used and this looks right for me.

When using 'all_max_peak', the nth_cache is iterated, seeking for a maximum. But within each iteration the nth_percentile_maximum is used which should be the same for every local_data_id of that RRD. I think the iteration should seek for the maximum of the n'th percentiles of the respective data sources.

The following patch makes that happen:

root@cacti:/var/www/cacti_test# diff -u lib/graph_variables.php.20121008-dist l> ib/graph_variables.php --- lib/graph_variables.php.20121008-dist 2012-10-25 15:18:56.000000000 +0200 +++ lib/graph_variables.php 2012-10-25 15:58:42.000000000 +0200 @@ -366,8 +366,8 @@ }elseif ($regexp_match_array[4] == "all_max_peak") { for ($t=0;($t<count($graph_items));$t++) { if ((preg_match("/(AREA|STACK|LINE[123])/", $graph_item_types{$graph_items[$t]["graph_type_id"]})) && (!empty($graph_items[$t]["data_template_rrd_id"]))) {

• if (! empty($nth_cache{$graph_items[$t]["local_data_id"]}["nth_percentile_maximum"])) {
• $local_nth = $nth_cache{$graph_items[$t]["local_data_id"]}["nth_percentile_maximum"];
• if (! empty($nth_cache{$graph_items[$t]["local_data_id"]}{$graph_items[$t]["data_source_name"]})) {
• $local_nth = $nth_cache{$graph_items[$t]["local_data_id"]}{$graph_items[$t]["data_source_name"]}; $local_nth = ($regexp_match_array[2] == "bits") ? $local_nth * 8 : $local_nth; $local_nth /= pow(10,intval($regexp_match_array[3]));

Is anyone able and willing to comment on this, check my investigations or correct me?

[cigamit]

Historically, Cacti's rrdtool fetch function returns the AVG and not the MAX values. However, there was a recent enhancement to the fetch function to accept a consolidation function (CF). So, now, it's possible to use the MAX consolidation function for fetch operations. This would be required if going beyond the first RRA, as the AVG and MAX will be the same in the first RRA always.

So, having taken a second look at this, we could do:

1) Check for the 'effective' RRA 2) Verify that the MAX CF is included 3) Use the MAX CF function to the fetch function if the CF is available 4) Better max values.

[f-roscher]

I have not touched Cacti's source code since I reported this. So I would have to look again to comment. On the other hand, you explanation, cigamit, does not seem like it needs a reply necessarily.

[cigamit]

Resolved. The RRDfile must include the MAX CF for the right data to be reported.