CA-408126 follow-up: Fix negative ds_min and RRD values in historical archives #6359
Conversation
last-genius
force-pushed
the
private/asultanov/CA-408126-historical-workaround
branch
from
1182ea7 to
74299ed
Compare
ocaml/libs/xapi-rrd/lib/rrd.ml
Outdated
| historical values to NaNs.*) | ||
| let min = float_of_string (get_el "min" i) in | ||
| let min = | ||
| if min < 0. && name <> "DCMI-power-reading" then |
There was a problem hiding this comment.
I'm not overjoyed about this specific hardcoding. Can this be done by type of aggregation (only gauges are allowed to be negative, for example)
There was a problem hiding this comment.
Not really, the DCMI example is a Gauge, but other Gauges were affected. I could add a quality-gate check that checks if no other datasources supply non-zero values to the ~min parameter.
There was a problem hiding this comment.
I don't think that will be necessary. Maybe we need some regression test to ensure out-of-range values don't enter the archives.
There was a problem hiding this comment.
min is 0 for DCMI-power-reading though, and max is 65534. I don't see why it needs special casing.
There was a problem hiding this comment.
It's min_float, which is apparently the smallest positive non-zero float value. Will drop this on Monday.
There was a problem hiding this comment.
hmm. 'xe host-data-source-list' reports the wrong thing then, it reports it as 0 :(
There was a problem hiding this comment.
But anyway negative power usage doesn't make sense, so we should fix rrdp_dcmi.ml to use 0 as well in the plugin itself for the minimum, and then we can drop the hardcoding here.
… archives When reading RRD archives from XML, make sure ds_min is never negative, this will convert negative values to NaNs as well. Signed-off-by: Andrii Sultanov <andrii.sultanov@cloud.com>
last-genius
force-pushed
the
private/asultanov/CA-408126-historical-workaround
branch
from
74299ed to
c1f02ed
Compare
|
Dropped the check for DCMI datasource, there actually aren't any negative datasources at all |
When reading RRD archives from XML, make sure ds_min is (almost never) negative, this will convert negative values to NaNs as well.