Skip to content

Commit

Permalink
CA-404597: rrd - Pass Gauge and Absolute data source values as-is
Browse files Browse the repository at this point in the history
Some recent changes related to RRDs likely exposed a long-standing latent issue
where the RRD library would process the passed-in values for Gauge and Absolute
data sources incorrectly leading to constant values changing from update to
update, for example:
```
$ rrd2csv memory_total_kib
timestamp, AVERAGE:host:8b533333-91e1-4698-bd17-95b9732ffbb6:memory_total_kib
2025-01-15T08:41:40Z, 33351000
2025-01-15T08:41:45Z, 33350000
2025-01-15T08:41:50Z, 33346000
2025-01-15T08:41:55Z, 33352000
```

Instead of treating Gauge and Absolute data sources as a
variation on the rate-based Derive data source type, expecting time-based
calculations to cancel each other out, do not undertake any calculations on
non-rate data sources at all.

This makes the unit test added in the previous commit pass.

Signed-off-by: Andrii Sultanov <[email protected]>
  • Loading branch information
last-genius committed Jan 16, 2025
1 parent 0326d27 commit 73ca3cc
Showing 1 changed file with 26 additions and 10 deletions.
36 changes: 26 additions & 10 deletions ocaml/libs/xapi-rrd/lib/rrd.ml
Original file line number Diff line number Diff line change
Expand Up @@ -341,9 +341,10 @@ let rra_update rrd proc_pdp_st elapsed_pdp_st pdps =
Array.iter updatefn rrd.rrd_rras

(* We assume that the data being given is of the form of a rate; that is,
it's dependent on the time interval between updates. To be able to
deal with gauge DSs, we multiply by the interval so that it cancels
the subsequent divide by interval later on *)
it's dependent on the time interval between updates.
Gauge and Absolute data sources are simply kept as is without any
time-based calculations, while Derive data sources will be changed according
to the time passed since the last measurement. (see CA-404597) *)
let process_ds_value ds value interval new_rrd =
if interval > ds.ds_mrhb then
nan
Expand All @@ -360,10 +361,8 @@ let process_ds_value ds value interval new_rrd =

let rate =
match (ds.ds_ty, new_rrd) with
| Absolute, _ | Derive, true ->
| Absolute, _ | Derive, true | Gauge, _ ->
value_raw
| Gauge, _ ->
value_raw *. interval
| Derive, false -> (
match (ds.ds_last, value) with
| VT_Int64 x, VT_Int64 y ->
Expand Down Expand Up @@ -433,7 +432,14 @@ let ds_update rrd timestamp valuesandtransforms new_rrd =
if Utils.isnan value then
ds.ds_unknown_sec <- pre_int
else
ds.ds_value <- ds.ds_value +. (pre_int *. value /. interval)
(* CA-404597 - Gauge and Absolute values should be passed as-is,
without being involved in time-based calculations at all.
This applies to calculations below as well *)
match ds.ds_ty with
| Gauge | Absolute ->
ds.ds_value <- value
| Derive ->
ds.ds_value <- ds.ds_value +. (pre_int *. value /. interval)
)
v2s ;

Expand All @@ -450,7 +456,13 @@ let ds_update rrd timestamp valuesandtransforms new_rrd =
let raw =
let proc_pdp_st = get_float_time last_updated rrd.timestep in
let occu_pdp_st = get_float_time timestamp rrd.timestep in
ds.ds_value /. (occu_pdp_st -. proc_pdp_st -. ds.ds_unknown_sec)

match ds.ds_ty with
| Gauge | Absolute ->
ds.ds_value
| Derive ->
ds.ds_value
/. (occu_pdp_st -. proc_pdp_st -. ds.ds_unknown_sec)
in
(* Apply the transform after the raw value has been calculated *)
let raw = apply_transform_function transform raw in
Expand All @@ -473,8 +485,12 @@ let ds_update rrd timestamp valuesandtransforms new_rrd =
ds.ds_value <- 0.0 ;
ds.ds_unknown_sec <- post_int
) else (
ds.ds_value <- post_int *. value /. interval ;
ds.ds_unknown_sec <- 0.0
ds.ds_unknown_sec <- 0.0 ;
match ds.ds_ty with
| Gauge | Absolute ->
ds.ds_value <- value
| Derive ->
ds.ds_value <- post_int *. value /. interval
)
)
v2s
Expand Down

0 comments on commit 73ca3cc

Please sign in to comment.