Skip to content

Commit 6c16a2f

Browse files
authored
rrd: Fix absolute rate calculations (#6646)
Absolute metric should work as follows: Timestamp = 300, 600, 900, 1200 Step = 300 seconds ABSOLUTE DS = 1, 2, 3, 4 But they do not seem to have ever worked correctly - they were previously divided by the interval but incorrectly handled NaNs, resulting in wrong behaviour. Then the refactoring (including 73ca3cc ("CA-404597: rrd - Pass Gauge and Absolute data source values as-is")) broke them further. Divide absolute metrics by the interval in process_ds_value. Fix the unit test to expect the right behaviour - it passes with this fix. --- This means absolute metrics are now correctly calculated as unit/s, changing the behaviour in #6640 I am not 100% sure this fix goes all the way either - unit tests should cover expected values in RRAs as well, and the logic is still really confusing to me.
2 parents c2c0422 + 4226002 commit 6c16a2f

File tree

2 files changed

+15
-37
lines changed

2 files changed

+15
-37
lines changed

ocaml/libs/xapi-rrd/lib/rrd.ml

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -342,9 +342,9 @@ let rra_update rrd proc_pdp_st elapsed_pdp_st pdps =
342342

343343
(* We assume that the data being given is of the form of a rate; that is,
344344
it's dependent on the time interval between updates.
345-
Gauge and Absolute data sources are simply kept as is without any
346-
time-based calculations, while Derive data sources will be changed according
347-
to the time passed since the last measurement. (see CA-404597) *)
345+
Gauge data sources are simply kept as is without any time-based
346+
calculations, while Absolute and Derive data sources will be changed
347+
according to the time passed since the last measurement. (see CA-404597) *)
348348
let process_ds_value ds value interval new_rrd =
349349
if interval > ds.ds_mrhb then
350350
nan
@@ -361,8 +361,10 @@ let process_ds_value ds value interval new_rrd =
361361

362362
let rate =
363363
match (ds.ds_ty, new_rrd) with
364-
| Absolute, _ | Derive, true | Gauge, _ ->
364+
| Derive, true | Gauge, _ ->
365365
value_raw
366+
| Absolute, _ ->
367+
value_raw /. interval
366368
| Derive, false -> (
367369
match (ds.ds_last, value) with
368370
| VT_Int64 x, VT_Int64 y ->

ocaml/libs/xapi-rrd/lib_test/unit_tests.ml

Lines changed: 9 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -147,46 +147,22 @@ let absolute_rrd =
147147

148148
let absolute_rrd_CA_404597 () =
149149
let rra = rra_create CF_Average 100 1 0.5 in
150-
let rra2 = rra_create CF_Average 100 10 0.5 in
151-
let rra3 = rra_create CF_Average 100 100 0.5 in
152-
let rra4 = rra_create CF_Average 100 1000 0.5 in
153-
let ts = 1000000000.0 in
150+
let ts = 0.0 in
154151
let ds =
155-
ds_create "foo" Absolute ~mrhb:10.0 ~min:0. ~max:infinity (VT_Float 0.0)
152+
ds_create "foo" Absolute ~mrhb:1000.0 ~min:0. ~max:infinity (VT_Float 0.0)
156153
in
157-
let ds2 =
158-
ds_create "bar" Absolute ~mrhb:10.0 ~min:0. ~max:infinity (VT_Float 0.0)
159-
in
160-
let ds3 =
161-
ds_create "baz" Absolute ~mrhb:10.0 ~min:0. ~max:infinity (VT_Float 0.0)
162-
in
163-
let ds4 =
164-
ds_create "boo" Absolute ~mrhb:10.0 ~min:0. ~max:infinity (VT_Float 0.0)
165-
in
166-
let rrd = rrd_create [|ds; ds2; ds3; ds4|] [|rra; rra2; rra3; rra4|] 1L ts in
154+
let rrd = rrd_create [|ds|] [|rra|] 1L ts in
167155
let id = Identity in
168156
for i = 1 to 100000 do
169-
let t = 1000000.0 +. (0.7 *. float_of_int i) in
157+
let t = 300. *. float_of_int i in
170158
let ((_, val1) as v1) =
171-
(0, {value= VT_Float (0.5 +. (0.5 *. sin (t /. 10.0))); transform= id})
159+
(0, {value= VT_Float (300. *. float_of_int i); transform= id})
172160
in
173-
let ((_, val2) as v2) =
174-
(1, {value= VT_Float (1.5 +. (0.5 *. cos (t /. 80.0))); transform= id})
175-
in
176-
let ((_, val3) as v3) =
177-
(2, {value= VT_Float (3.5 +. (0.5 *. sin (t /. 700.0))); transform= id})
178-
in
179-
let ((_, val4) as v4) =
180-
(3, {value= VT_Float (6.5 +. (0.5 *. cos (t /. 5000.0))); transform= id})
181-
in
182-
ds_update rrd t [|v1; v2; v3; v4|] false ;
161+
ds_update rrd t [|v1|] false ;
183162

184-
Array.iter2
185-
(fun ds value ->
186-
compare_float __LOC__ ds.ds_value
187-
(float_of_string (ds_value_to_string value.value))
188-
)
189-
rrd.rrd_dss [|val1; val2; val3; val4|]
163+
compare_float __LOC__
164+
(float_of_string (ds_value_to_string val1.value) /. 300.)
165+
rrd.rrd_dss.(0).ds_value
190166
done
191167

192168
(** Verify that Gauge data soruce values are correctly handled by the RRD lib

0 commit comments

Comments
 (0)