Reuse the SN_EXISTS bit to track anomaly status. (#11154)

* Replace all usages of SN_EXISTS with SN_DEFAULT_FLAGS.

* Remove references to SN_NOT_EXISTS in comments.

* Replace raw zero constant with SN_EMPTY_SLOT.

* Use get_storage_number_flags only in storage_number.{c,h}

* Compare against SN_EMPTY_SLOT to check if a storage_number exists.

This is safe because:

  1. rrdset_done_interpolate() is the only place where we call store_metric(),
  2. All store_metric() calls, except for one, store an SN_EMPTY_SLOT value.
  3. When we are not storing an SN_EMPTY_SLOT value, the flags that we pass to
     pack_storage_number() can be either SN_EXISTS *or* SN_EXISTS_RESET.

* Compare only the SN_EXISTS_RESET bit to find reset values.

* Remove get_storage_number_flags from storage_number.h

* Do not set storage_number flags outside of rrdset_done_interpolate().

This is a NFC intended to limit the scope of storage_number flags
processing to just one function.

* Set reset bit without overwriting the rest of the flags.

* Rename SN_EXISTS to SN_ANOMALY_BIT.

* Use GOTOs in pack_storage_number to return from a single place.

* Teach pack_storage_number how to handle anomalous zero values.

Up until now, a storage_number had always either the SN_EXISTS or
SN_EXISTS_RESET bit set. This meant that it was not possible for any
packed storage_number to compare equal to the SN_EMPTY_SLOT.

However, the SN_ANOMALY_BIT can be set to zero. This is fine for every
value other than the anomalous 0 value, because it would compare equal to
SN_EMPTY_SLOT. We address this issue by mapping the anomalous zero value
to SN_EXISTS_100 (a number which was not possible to generate with the
previous versions of the agent, ie. it won't exist in older dbengine files).

This change was tested manually by intentionally flipping the anomaly
bit for odd/even iterations in rrdset_done_interpolate. Prior to this
change, charts whose dimensions had 0 values, where showing up in the
dashboard as gaps (SN_EMPTY_SLOT), whereas with this commit the values
are displayed correctly.

3 files changed, 27 insertions, 10 deletions

diff --git a/libnetdata/storage_number/storage_number.c b/libnetdata/storage_number/storage_number.c
index 8ef1353b09..3e6a9f45c4 100644
--- a/libnetdata/storage_number/storage_number.c
+++ b/libnetdata/storage_number/storage_number.c
@@ -2,17 +2,23 @@
 
 #include "../libnetdata.h"
 
+#define get_storage_number_flags(value) \
+    ((((storage_number)(value)) & (1 << 24)) | \
+     (((storage_number)(value)) & (1 << 25)) | \
+     (((storage_number)(value)) & (1 << 26)))
+
 storage_number pack_storage_number(calculated_number value, uint32_t flags) {
     // bit 32 = sign 0:positive, 1:negative
     // bit 31 = 0:divide, 1:multiply
     // bit 30, 29, 28 = (multiplier or divider) 0-7 (8 total)
     // bit 27 SN_EXISTS_100
     // bit 26 SN_EXISTS_RESET
-    // bit 25 SN_EXISTS
+    // bit 25 SN_ANOMALY_BIT = 0: anomalous, 1: not anomalous
     // bit 24 to bit 1 = the value
 
     storage_number r = get_storage_number_flags(flags);
-    if(!value) return r;
+    if(!value)
+        goto RET_SN;
 
     int m = 0;
     calculated_number n = value, factor = 10;
@@ -47,7 +53,7 @@ storage_number pack_storage_number(calculated_number value, uint32_t flags) {
             error("Number " CALCULATED_NUMBER_FORMAT " is too big.", value);
             #endif
             r += 0x00ffffff;
-            return r;
+            goto RET_SN;
         }
     }
     else {
@@ -78,6 +84,10 @@ storage_number pack_storage_number(calculated_number value, uint32_t flags) {
     r += (storage_number)n;
 #endif
 
+RET_SN:
+    if (r == SN_EMPTY_SLOT)
+        r = SN_ANOMALOUS_ZERO;
+
     return r;
 }
 
@@ -100,7 +110,7 @@ calculated_number unpack_storage_number(storage_number value) {
         factor = 100;
 
     // bit 26 SN_EXISTS_RESET
-    // bit 25 SN_EXISTS
+    // bit 25 SN_ANOMALY_BIT
 
     // bit 30, 29, 28 = (multiplier or divider) 0-7 (8 total)
     int mul = (value & ((1<<29)|(1<<28)|(1<<27))) >> 27;
diff --git a/libnetdata/storage_number/storage_number.h b/libnetdata/storage_number/storage_number.h
index 4ad7ff6246..4101f69e01 100644
--- a/libnetdata/storage_number/storage_number.h
+++ b/libnetdata/storage_number/storage_number.h
@@ -60,17 +60,24 @@ typedef long double collected_number;
 typedef uint32_t storage_number;
 #define STORAGE_NUMBER_FORMAT "%u"
 
-#define SN_EXISTS           (1 << 24) // the value exists
+#define SN_ANOMALY_BIT      (1 << 24) // the anomaly bit of the value
 #define SN_EXISTS_RESET     (1 << 25) // the value has been overflown
 #define SN_EXISTS_100       (1 << 26) // very large value (multiplier is 100 instead of 10)
 
-// extract the flags
-#define get_storage_number_flags(value) ((((storage_number)(value)) & (1 << 24)) | (((storage_number)(value)) & (1 << 25)) | (((storage_number)(value)) & (1 << 26)))
+#define SN_DEFAULT_FLAGS    SN_ANOMALY_BIT
+
 #define SN_EMPTY_SLOT 0x00000000
 
+// When the calculated number is zero and the value is anomalous (ie. it's bit
+// is zero) we want to return a storage_number representation that is
+// different from the empty slot. We achieve this by mapping zero to
+// SN_EXISTS_100. Unpacking the SN_EXISTS_100 value will return zero because
+// its fraction field (as well as its exponent factor field) will be zero.
+#define SN_ANOMALOUS_ZERO SN_EXISTS_100
+
 // checks
-#define does_storage_number_exist(value) ((get_storage_number_flags(value) != 0)?1:0)
-#define did_storage_number_reset(value)  ((get_storage_number_flags(value) == SN_EXISTS_RESET)?1:0)
+#define does_storage_number_exist(value) (((storage_number) (value)) != SN_EMPTY_SLOT)
+#define did_storage_number_reset(value)  ((((storage_number) (value)) & SN_EXISTS_RESET) != 0)
 
 storage_number pack_storage_number(calculated_number value, uint32_t flags);
 calculated_number unpack_storage_number(storage_number value);
diff --git a/libnetdata/storage_number/tests/test_storage_number.c b/libnetdata/storage_number/tests/test_storage_number.c
index 7ef18b1de9..f90521cabe 100644
--- a/libnetdata/storage_number/tests/test_storage_number.c
+++ b/libnetdata/storage_number/tests/test_storage_number.c
@@ -38,7 +38,7 @@ static void test_number_printing(void **state)
     print_calculated_number(value, -9999.9999999);
     assert_string_equal(value, "-9999.9999999");
 
-    print_calculated_number(value, unpack_storage_number(pack_storage_number(16.777218L, SN_EXISTS)));
+    print_calculated_number(value, unpack_storage_number(pack_storage_number(16.777218L, SN_DEFAULT_FLAGS)));
     assert_string_equal(value, "16.77722");
 }