diff options
-rw-r--r-- | daemon/unit_test.c | 172 | ||||
-rw-r--r-- | database/rrdset.c | 38 | ||||
-rw-r--r-- | libnetdata/storage_number/storage_number.h | 4 |
3 files changed, 70 insertions, 144 deletions
diff --git a/daemon/unit_test.c b/daemon/unit_test.c index 31718eeeae..2e59273214 100644 --- a/daemon/unit_test.c +++ b/daemon/unit_test.c @@ -588,28 +588,28 @@ struct test test4 = { // test5 - 32 bit overflows struct feed_values test5_feed[] = { - { 0, 0x00000000FFFFFFFFULL / 3 * 0 }, - { 1000000, 0x00000000FFFFFFFFULL / 3 * 1 }, - { 1000000, 0x00000000FFFFFFFFULL / 3 * 2 }, - { 1000000, 0x00000000FFFFFFFFULL / 3 * 0 }, - { 1000000, 0x00000000FFFFFFFFULL / 3 * 1 }, - { 1000000, 0x00000000FFFFFFFFULL / 3 * 2 }, - { 1000000, 0x00000000FFFFFFFFULL / 3 * 0 }, - { 1000000, 0x00000000FFFFFFFFULL / 3 * 1 }, - { 1000000, 0x00000000FFFFFFFFULL / 3 * 2 }, - { 1000000, 0x00000000FFFFFFFFULL / 3 * 0 }, + { 0, 0x00000000FFFFFFFFULL / 15 * 0 }, + { 1000000, 0x00000000FFFFFFFFULL / 15 * 7 }, + { 1000000, 0x00000000FFFFFFFFULL / 15 * 14 }, + { 1000000, 0x00000000FFFFFFFFULL / 15 * 0 }, + { 1000000, 0x00000000FFFFFFFFULL / 15 * 7 }, + { 1000000, 0x00000000FFFFFFFFULL / 15 * 14 }, + { 1000000, 0x00000000FFFFFFFFULL / 15 * 0 }, + { 1000000, 0x00000000FFFFFFFFULL / 15 * 7 }, + { 1000000, 0x00000000FFFFFFFFULL / 15 * 14 }, + { 1000000, 0x00000000FFFFFFFFULL / 15 * 0 }, }; calculated_number test5_results[] = { - 0x00000000FFFFFFFFULL / 3, - 0x00000000FFFFFFFFULL / 3, - 0x00000000FFFFFFFFULL / 3, - 0x00000000FFFFFFFFULL / 3, - 0x00000000FFFFFFFFULL / 3, - 0x00000000FFFFFFFFULL / 3, - 0x00000000FFFFFFFFULL / 3, - 0x00000000FFFFFFFFULL / 3, - 0x00000000FFFFFFFFULL / 3, + 0x00000000FFFFFFFFULL / 15 * 7, + 0x00000000FFFFFFFFULL / 15 * 7, + 0x00000000FFFFFFFFULL / 15, + 0x00000000FFFFFFFFULL / 15 * 7, + 0x00000000FFFFFFFFULL / 15 * 7, + 0x00000000FFFFFFFFULL / 15, + 0x00000000FFFFFFFFULL / 15 * 7, + 0x00000000FFFFFFFFULL / 15 * 7, + 0x00000000FFFFFFFFULL / 15, }; struct test test5 = { @@ -628,36 +628,36 @@ struct test test5 = { }; // -------------------------------------------------------------------------------------------------------------------- -// test5b - 16 bit overflows +// test5b - 64 bit overflows struct feed_values test5b_feed[] = { - { 0, 0x000000000000FFFFULL / 3 * 0 }, - { 1000000, 0x000000000000FFFFULL / 3 * 1 }, - { 1000000, 0x000000000000FFFFULL / 3 * 2 }, - { 1000000, 0x000000000000FFFFULL / 3 * 0 }, - { 1000000, 0x000000000000FFFFULL / 3 * 1 }, - { 1000000, 0x000000000000FFFFULL / 3 * 2 }, - { 1000000, 0x000000000000FFFFULL / 3 * 0 }, - { 1000000, 0x000000000000FFFFULL / 3 * 1 }, - { 1000000, 0x000000000000FFFFULL / 3 * 2 }, - { 1000000, 0x000000000000FFFFULL / 3 * 0 }, + { 0, 0xFFFFFFFFFFFFFFFFULL / 15 * 0 }, + { 1000000, 0xFFFFFFFFFFFFFFFFULL / 15 * 7 }, + { 1000000, 0xFFFFFFFFFFFFFFFFULL / 15 * 14 }, + { 1000000, 0xFFFFFFFFFFFFFFFFULL / 15 * 0 }, + { 1000000, 0xFFFFFFFFFFFFFFFFULL / 15 * 7 }, + { 1000000, 0xFFFFFFFFFFFFFFFFULL / 15 * 14 }, + { 1000000, 0xFFFFFFFFFFFFFFFFULL / 15 * 0 }, + { 1000000, 0xFFFFFFFFFFFFFFFFULL / 15 * 7 }, + { 1000000, 0xFFFFFFFFFFFFFFFFULL / 15 * 14 }, + { 1000000, 0xFFFFFFFFFFFFFFFFULL / 15 * 0 }, }; calculated_number test5b_results[] = { - 0x000000000000FFFFULL / 3, - 0x000000000000FFFFULL / 3, - 0x000000000000FFFFULL / 3, - 0x000000000000FFFFULL / 3, - 0x000000000000FFFFULL / 3, - 0x000000000000FFFFULL / 3, - 0x000000000000FFFFULL / 3, - 0x000000000000FFFFULL / 3, - 0x000000000000FFFFULL / 3, + 0xFFFFFFFFFFFFFFFFULL / 15 * 7, + 0xFFFFFFFFFFFFFFFFULL / 15 * 7, + 0xFFFFFFFFFFFFFFFFULL / 15, + 0xFFFFFFFFFFFFFFFFULL / 15 * 7, + 0xFFFFFFFFFFFFFFFFULL / 15 * 7, + 0xFFFFFFFFFFFFFFFFULL / 15, + 0xFFFFFFFFFFFFFFFFULL / 15 * 7, + 0xFFFFFFFFFFFFFFFFULL / 15 * 7, + 0xFFFFFFFFFFFFFFFFULL / 15, }; struct test test5b = { "test5b", // name - "test 16-bit incremental values overflow", + "test 64-bit incremental values overflow", 1, // update_every 1, // multiplier 1, // divisor @@ -671,92 +671,6 @@ struct test test5b = { }; // -------------------------------------------------------------------------------------------------------------------- -// test5c - 8 bit overflows - -struct feed_values test5c_feed[] = { - { 0, 0x00000000000000FFULL / 3 * 0 }, - { 1000000, 0x00000000000000FFULL / 3 * 1 }, - { 1000000, 0x00000000000000FFULL / 3 * 2 }, - { 1000000, 0x00000000000000FFULL / 3 * 0 }, - { 1000000, 0x00000000000000FFULL / 3 * 1 }, - { 1000000, 0x00000000000000FFULL / 3 * 2 }, - { 1000000, 0x00000000000000FFULL / 3 * 0 }, - { 1000000, 0x00000000000000FFULL / 3 * 1 }, - { 1000000, 0x00000000000000FFULL / 3 * 2 }, - { 1000000, 0x00000000000000FFULL / 3 * 0 }, -}; - -calculated_number test5c_results[] = { - 0x00000000000000FFULL / 3, - 0x00000000000000FFULL / 3, - 0x00000000000000FFULL / 3, - 0x00000000000000FFULL / 3, - 0x00000000000000FFULL / 3, - 0x00000000000000FFULL / 3, - 0x00000000000000FFULL / 3, - 0x00000000000000FFULL / 3, - 0x00000000000000FFULL / 3, -}; - -struct test test5c = { - "test5c", // name - "test 8-bit incremental values overflow", - 1, // update_every - 1, // multiplier - 1, // divisor - RRD_ALGORITHM_INCREMENTAL, // algorithm - 10, // feed entries - 9, // result entries - test5c_feed, // feed - test5c_results, // results - NULL, // feed2 - NULL // results2 -}; - -// -------------------------------------------------------------------------------------------------------------------- -// test5d - 64 bit overflows - -struct feed_values test5d_feed[] = { - { 0, 0xFFFFFFFFFFFFFFFFULL / 3 * 0 }, - { 1000000, 0xFFFFFFFFFFFFFFFFULL / 3 * 1 }, - { 1000000, 0xFFFFFFFFFFFFFFFFULL / 3 * 2 }, - { 1000000, 0xFFFFFFFFFFFFFFFFULL / 3 * 0 }, - { 1000000, 0xFFFFFFFFFFFFFFFFULL / 3 * 1 }, - { 1000000, 0xFFFFFFFFFFFFFFFFULL / 3 * 2 }, - { 1000000, 0xFFFFFFFFFFFFFFFFULL / 3 * 0 }, - { 1000000, 0xFFFFFFFFFFFFFFFFULL / 3 * 1 }, - { 1000000, 0xFFFFFFFFFFFFFFFFULL / 3 * 2 }, - { 1000000, 0xFFFFFFFFFFFFFFFFULL / 3 * 0 }, -}; - -calculated_number test5d_results[] = { - 0xFFFFFFFFFFFFFFFFULL / 3, - 0xFFFFFFFFFFFFFFFFULL / 3, - 0xFFFFFFFFFFFFFFFFULL / 3, - 0xFFFFFFFFFFFFFFFFULL / 3, - 0xFFFFFFFFFFFFFFFFULL / 3, - 0xFFFFFFFFFFFFFFFFULL / 3, - 0xFFFFFFFFFFFFFFFFULL / 3, - 0xFFFFFFFFFFFFFFFFULL / 3, - 0xFFFFFFFFFFFFFFFFULL / 3, -}; - -struct test test5d = { - "test5d", // name - "test 64-bit incremental values overflow", - 1, // update_every - 1, // multiplier - 1, // divisor - RRD_ALGORITHM_INCREMENTAL, // algorithm - 10, // feed entries - 9, // result entries - test5d_feed, // feed - test5d_results, // results - NULL, // feed2 - NULL // results2 -}; - -// -------------------------------------------------------------------------------------------------------------------- // test6 struct feed_values test6_feed[] = { @@ -1417,12 +1331,6 @@ int run_all_mockup_tests(void) if(run_test(&test5b)) return 1; - if(run_test(&test5c)) - return 1; - - if(run_test(&test5d)) - return 1; - if(run_test(&test6)) return 1; diff --git a/database/rrdset.c b/database/rrdset.c index e96d707bed..0f52970304 100644 --- a/database/rrdset.c +++ b/database/rrdset.c @@ -1447,9 +1447,11 @@ void rrdset_done(RRDSET *st) { continue; } - // if the new is smaller than the old (an overflow, or reset), set the old equal to the new - // to reset the calculation (it will give zero as the calculation for this second) - if(unlikely(rd->last_collected_value > rd->collected_value)) { + // If the new is smaller than the old (an overflow, or reset), set the old equal to the new + // to reset the calculation (it will give zero as the calculation for this second). + // It is imperative to set the comparison to uint64_t since type collected_number is signed and + // produces wrong results as far as incremental counters are concerned. + if(unlikely((uint64_t)rd->last_collected_value > (uint64_t)rd->collected_value)) { debug(D_RRD_STATS, "%s.%s: RESET or OVERFLOW. Last collected value = " COLLECTED_NUMBER_FORMAT ", current = " COLLECTED_NUMBER_FORMAT , st->name, rd->name , rd->last_collected_value @@ -1463,17 +1465,29 @@ void rrdset_done(RRDSET *st) { uint64_t max = (uint64_t)rd->collected_value_max; uint64_t cap = 0; - if(max > 0x00000000FFFFFFFFULL) cap = 0xFFFFFFFFFFFFFFFFULL; - else if(max > 0x000000000000FFFFULL) cap = 0x00000000FFFFFFFFULL; - else if(max > 0x00000000000000FFULL) cap = 0x000000000000FFFFULL; - else cap = 0x00000000000000FFULL; + // Signed values are handled by exploiting two's complement which will produce positive deltas + if (max > 0x00000000FFFFFFFFULL) + cap = 0xFFFFFFFFFFFFFFFFULL; // handles signed and unsigned 64-bit counters + else + cap = 0x00000000FFFFFFFFULL; // handles signed and unsigned 32-bit counters uint64_t delta = cap - last + new; - - rd->calculated_value += - (calculated_number) delta - * (calculated_number) rd->multiplier - / (calculated_number) rd->divisor; + uint64_t max_acceptable_rate = (cap / 100) * MAX_INCREMENTAL_PERCENT_RATE; + + // If the delta is less than the maximum acceptable rate and the previous value was near the cap + // then this is an overflow. There can be false positives such that a reset is detected as an + // overflow. + // TODO: remember recent history of rates and compare with current rate to reduce this chance. + if (delta < max_acceptable_rate) { + rd->calculated_value += + (calculated_number) delta + * (calculated_number) rd->multiplier + / (calculated_number) rd->divisor; + } else { + // This is a reset. Any overflow with a rate greater than MAX_INCREMENTAL_PERCENT_RATE will also + // be detected as a reset instead. + rd->calculated_value += (calculated_number)0; + } } else { rd->calculated_value += diff --git a/libnetdata/storage_number/storage_number.h b/libnetdata/storage_number/storage_number.h index da3ba23fa8..28b7f267c0 100644 --- a/libnetdata/storage_number/storage_number.h +++ b/libnetdata/storage_number/storage_number.h @@ -87,4 +87,8 @@ int print_calculated_number(char *str, calculated_number value); #define ACCURACY_LOSS_ACCEPTED_PERCENT 0.0001 #define accuracy_loss(t1, t2) (((t1) == (t2) || (t1) == 0.0 || (t2) == 0.0) ? 0.0 : (100.0 - (((t1) > (t2)) ? ((t2) * 100.0 / (t1) ) : ((t1) * 100.0 / (t2))))) +// Maximum acceptable rate of increase for counters. With a rate of 10% netdata can safely detect overflows with a +// period of at least every other 10 samples. +#define MAX_INCREMENTAL_PERCENT_RATE 10 + #endif /* NETDATA_STORAGE_NUMBER_H */ |