Fix counter reset detection (#7220)
* Removed support for 16-bit and 8-bit counter overflow * Improve behaviour of counter overflow detection versus counter resets. * Added support for signed 32-bit and 64-bit limits for counter overflows. * Fixed signed incremental counter issues and added unit tests.
Markos Fountoulakis committed
Oct 30, 2019 at 15:38 UTC
69e5e128f6c42e9bdfbe18e0725e4ea91f9f6950
3 files changed
+70
-144
daemon/unit_test.c
+40
-132
@@ -588,28 +588,28 @@ struct test test4 = {
588
// test5 - 32 bit overflows
589
590
struct feed_values test5_feed[] = {
591
- { 0, 0x00000000FFFFFFFFULL / 3 * 0 },
592
- { 1000000, 0x00000000FFFFFFFFULL / 3 * 1 },
593
- { 1000000, 0x00000000FFFFFFFFULL / 3 * 2 },
594
- { 1000000, 0x00000000FFFFFFFFULL / 3 * 0 },
595
- { 1000000, 0x00000000FFFFFFFFULL / 3 * 1 },
596
- { 1000000, 0x00000000FFFFFFFFULL / 3 * 2 },
597
- { 1000000, 0x00000000FFFFFFFFULL / 3 * 0 },
598
- { 1000000, 0x00000000FFFFFFFFULL / 3 * 1 },
599
- { 1000000, 0x00000000FFFFFFFFULL / 3 * 2 },
600
- { 1000000, 0x00000000FFFFFFFFULL / 3 * 0 },
591
+ { 0, 0x00000000FFFFFFFFULL / 15 * 0 },
592
+ { 1000000, 0x00000000FFFFFFFFULL / 15 * 7 },
593
+ { 1000000, 0x00000000FFFFFFFFULL / 15 * 14 },
594
+ { 1000000, 0x00000000FFFFFFFFULL / 15 * 0 },
595
+ { 1000000, 0x00000000FFFFFFFFULL / 15 * 7 },
596
+ { 1000000, 0x00000000FFFFFFFFULL / 15 * 14 },
597
+ { 1000000, 0x00000000FFFFFFFFULL / 15 * 0 },
598
+ { 1000000, 0x00000000FFFFFFFFULL / 15 * 7 },
599
+ { 1000000, 0x00000000FFFFFFFFULL / 15 * 14 },
600
+ { 1000000, 0x00000000FFFFFFFFULL / 15 * 0 },
601
};
602
603
calculated_number test5_results[] = {
604
- 0x00000000FFFFFFFFULL / 3,
605
- 0x00000000FFFFFFFFULL / 3,
606
- 0x00000000FFFFFFFFULL / 3,
607
- 0x00000000FFFFFFFFULL / 3,
608
- 0x00000000FFFFFFFFULL / 3,
609
- 0x00000000FFFFFFFFULL / 3,
610
- 0x00000000FFFFFFFFULL / 3,
611
- 0x00000000FFFFFFFFULL / 3,
612
- 0x00000000FFFFFFFFULL / 3,
604
+ 0x00000000FFFFFFFFULL / 15 * 7,
605
+ 0x00000000FFFFFFFFULL / 15 * 7,
606
+ 0x00000000FFFFFFFFULL / 15,
607
+ 0x00000000FFFFFFFFULL / 15 * 7,
608
+ 0x00000000FFFFFFFFULL / 15 * 7,
609
+ 0x00000000FFFFFFFFULL / 15,
610
+ 0x00000000FFFFFFFFULL / 15 * 7,
611
+ 0x00000000FFFFFFFFULL / 15 * 7,
612
+ 0x00000000FFFFFFFFULL / 15,
613
};
614
615
struct test test5 = {
@@ -628,36 +628,36 @@ struct test test5 = {
628
};
629
630
// --------------------------------------------------------------------------------------------------------------------
631
-// test5b - 16 bit overflows
631
+// test5b - 64 bit overflows
632
633
struct feed_values test5b_feed[] = {
634
- { 0, 0x000000000000FFFFULL / 3 * 0 },
635
- { 1000000, 0x000000000000FFFFULL / 3 * 1 },
636
- { 1000000, 0x000000000000FFFFULL / 3 * 2 },
637
- { 1000000, 0x000000000000FFFFULL / 3 * 0 },
638
- { 1000000, 0x000000000000FFFFULL / 3 * 1 },
639
- { 1000000, 0x000000000000FFFFULL / 3 * 2 },
640
- { 1000000, 0x000000000000FFFFULL / 3 * 0 },
641
- { 1000000, 0x000000000000FFFFULL / 3 * 1 },
642
- { 1000000, 0x000000000000FFFFULL / 3 * 2 },
643
- { 1000000, 0x000000000000FFFFULL / 3 * 0 },
634
+ { 0, 0xFFFFFFFFFFFFFFFFULL / 15 * 0 },
635
+ { 1000000, 0xFFFFFFFFFFFFFFFFULL / 15 * 7 },
636
+ { 1000000, 0xFFFFFFFFFFFFFFFFULL / 15 * 14 },
637
+ { 1000000, 0xFFFFFFFFFFFFFFFFULL / 15 * 0 },
638
+ { 1000000, 0xFFFFFFFFFFFFFFFFULL / 15 * 7 },
639
+ { 1000000, 0xFFFFFFFFFFFFFFFFULL / 15 * 14 },
640
+ { 1000000, 0xFFFFFFFFFFFFFFFFULL / 15 * 0 },
641
+ { 1000000, 0xFFFFFFFFFFFFFFFFULL / 15 * 7 },
642
+ { 1000000, 0xFFFFFFFFFFFFFFFFULL / 15 * 14 },
643
+ { 1000000, 0xFFFFFFFFFFFFFFFFULL / 15 * 0 },
644
};
645
646
calculated_number test5b_results[] = {
647
- 0x000000000000FFFFULL / 3,
648
- 0x000000000000FFFFULL / 3,
649
- 0x000000000000FFFFULL / 3,
650
- 0x000000000000FFFFULL / 3,
651
- 0x000000000000FFFFULL / 3,
652
- 0x000000000000FFFFULL / 3,
653
- 0x000000000000FFFFULL / 3,
654
- 0x000000000000FFFFULL / 3,
655
- 0x000000000000FFFFULL / 3,
647
+ 0xFFFFFFFFFFFFFFFFULL / 15 * 7,
648
+ 0xFFFFFFFFFFFFFFFFULL / 15 * 7,
649
+ 0xFFFFFFFFFFFFFFFFULL / 15,
650
+ 0xFFFFFFFFFFFFFFFFULL / 15 * 7,
651
+ 0xFFFFFFFFFFFFFFFFULL / 15 * 7,
652
+ 0xFFFFFFFFFFFFFFFFULL / 15,
653
+ 0xFFFFFFFFFFFFFFFFULL / 15 * 7,
654
+ 0xFFFFFFFFFFFFFFFFULL / 15 * 7,
655
+ 0xFFFFFFFFFFFFFFFFULL / 15,
656
};
657
658
struct test test5b = {
659
"test5b", // name
660
- "test 16-bit incremental values overflow",
660
+ "test 64-bit incremental values overflow",
661
1, // update_every
662
1, // multiplier
663
1, // divisor
@@ -670,92 +670,6 @@ struct test test5b = {
670
NULL // results2
671
};
672
673
-// --------------------------------------------------------------------------------------------------------------------
674
-// test5c - 8 bit overflows
675
-
676
-struct feed_values test5c_feed[] = {
677
- { 0, 0x00000000000000FFULL / 3 * 0 },
678
- { 1000000, 0x00000000000000FFULL / 3 * 1 },
679
- { 1000000, 0x00000000000000FFULL / 3 * 2 },
680
- { 1000000, 0x00000000000000FFULL / 3 * 0 },
681
- { 1000000, 0x00000000000000FFULL / 3 * 1 },
682
- { 1000000, 0x00000000000000FFULL / 3 * 2 },
683
- { 1000000, 0x00000000000000FFULL / 3 * 0 },
684
- { 1000000, 0x00000000000000FFULL / 3 * 1 },
685
- { 1000000, 0x00000000000000FFULL / 3 * 2 },
686
- { 1000000, 0x00000000000000FFULL / 3 * 0 },
687
-};
688
-
689
-calculated_number test5c_results[] = {
690
- 0x00000000000000FFULL / 3,
691
- 0x00000000000000FFULL / 3,
692
- 0x00000000000000FFULL / 3,
693
- 0x00000000000000FFULL / 3,
694
- 0x00000000000000FFULL / 3,
695
- 0x00000000000000FFULL / 3,
696
- 0x00000000000000FFULL / 3,
697
- 0x00000000000000FFULL / 3,
698
- 0x00000000000000FFULL / 3,
699
-};
700
-
701
-struct test test5c = {
702
- "test5c", // name
703
- "test 8-bit incremental values overflow",
704
- 1, // update_every
705
- 1, // multiplier
706
- 1, // divisor
707
- RRD_ALGORITHM_INCREMENTAL, // algorithm
708
- 10, // feed entries
709
- 9, // result entries
710
- test5c_feed, // feed
711
- test5c_results, // results
712
- NULL, // feed2
713
- NULL // results2
714
-};
715
-
716
-// --------------------------------------------------------------------------------------------------------------------
717
-// test5d - 64 bit overflows
718
-
719
-struct feed_values test5d_feed[] = {
720
- { 0, 0xFFFFFFFFFFFFFFFFULL / 3 * 0 },
721
- { 1000000, 0xFFFFFFFFFFFFFFFFULL / 3 * 1 },
722
- { 1000000, 0xFFFFFFFFFFFFFFFFULL / 3 * 2 },
723
- { 1000000, 0xFFFFFFFFFFFFFFFFULL / 3 * 0 },
724
- { 1000000, 0xFFFFFFFFFFFFFFFFULL / 3 * 1 },
725
- { 1000000, 0xFFFFFFFFFFFFFFFFULL / 3 * 2 },
726
- { 1000000, 0xFFFFFFFFFFFFFFFFULL / 3 * 0 },
727
- { 1000000, 0xFFFFFFFFFFFFFFFFULL / 3 * 1 },
728
- { 1000000, 0xFFFFFFFFFFFFFFFFULL / 3 * 2 },
729
- { 1000000, 0xFFFFFFFFFFFFFFFFULL / 3 * 0 },
730
-};
731
-
732
-calculated_number test5d_results[] = {
733
- 0xFFFFFFFFFFFFFFFFULL / 3,
734
- 0xFFFFFFFFFFFFFFFFULL / 3,
735
- 0xFFFFFFFFFFFFFFFFULL / 3,
736
- 0xFFFFFFFFFFFFFFFFULL / 3,
737
- 0xFFFFFFFFFFFFFFFFULL / 3,
738
- 0xFFFFFFFFFFFFFFFFULL / 3,
739
- 0xFFFFFFFFFFFFFFFFULL / 3,
740
- 0xFFFFFFFFFFFFFFFFULL / 3,
741
- 0xFFFFFFFFFFFFFFFFULL / 3,
742
-};
743
-
744
-struct test test5d = {
745
- "test5d", // name
746
- "test 64-bit incremental values overflow",
747
- 1, // update_every
748
- 1, // multiplier
749
- 1, // divisor
750
- RRD_ALGORITHM_INCREMENTAL, // algorithm
751
- 10, // feed entries
752
- 9, // result entries
753
- test5d_feed, // feed
754
- test5d_results, // results
755
- NULL, // feed2
756
- NULL // results2
757
-};
758
-
673
// --------------------------------------------------------------------------------------------------------------------
674
// test6
675
@@ -1417,12 +1331,6 @@ int run_all_mockup_tests(void)
1331
if(run_test(&test5b))
1332
return 1;
1333
1420
- if(run_test(&test5c))
1421
- return 1;
1422
-
1423
- if(run_test(&test5d))
1424
- return 1;
1425
-
1334
if(run_test(&test6))
1335
return 1;
1336
database/rrdset.c
+26
-12
@@ -1447,9 +1447,11 @@ void rrdset_done(RRDSET *st) {
1447
continue;
1448
}
1449
1450
- // if the new is smaller than the old (an overflow, or reset), set the old equal to the new
1451
- // to reset the calculation (it will give zero as the calculation for this second)
1452
- if(unlikely(rd->last_collected_value > rd->collected_value)) {
1450
+ // If the new is smaller than the old (an overflow, or reset), set the old equal to the new
1451
+ // to reset the calculation (it will give zero as the calculation for this second).
1452
+ // It is imperative to set the comparison to uint64_t since type collected_number is signed and
1453
+ // produces wrong results as far as incremental counters are concerned.
1454
+ if(unlikely((uint64_t)rd->last_collected_value > (uint64_t)rd->collected_value)) {
1455
debug(D_RRD_STATS, "%s.%s: RESET or OVERFLOW. Last collected value = " COLLECTED_NUMBER_FORMAT ", current = " COLLECTED_NUMBER_FORMAT
1456
, st->name, rd->name
1457
, rd->last_collected_value
@@ -1463,17 +1465,29 @@ void rrdset_done(RRDSET *st) {
1465
uint64_t max = (uint64_t)rd->collected_value_max;
1466
uint64_t cap = 0;
1467
1466
- if(max > 0x00000000FFFFFFFFULL) cap = 0xFFFFFFFFFFFFFFFFULL;
1467
- else if(max > 0x000000000000FFFFULL) cap = 0x00000000FFFFFFFFULL;
1468
- else if(max > 0x00000000000000FFULL) cap = 0x000000000000FFFFULL;
1469
- else cap = 0x00000000000000FFULL;
1468
+ // Signed values are handled by exploiting two's complement which will produce positive deltas
1469
+ if (max > 0x00000000FFFFFFFFULL)
1470
+ cap = 0xFFFFFFFFFFFFFFFFULL; // handles signed and unsigned 64-bit counters
1471
+ else
1472
+ cap = 0x00000000FFFFFFFFULL; // handles signed and unsigned 32-bit counters
1473
1474
uint64_t delta = cap - last + new;
1472
-
1473
- rd->calculated_value +=
1474
- (calculated_number) delta
1475
- * (calculated_number) rd->multiplier
1476
- / (calculated_number) rd->divisor;
1475
+ uint64_t max_acceptable_rate = (cap / 100) * MAX_INCREMENTAL_PERCENT_RATE;
1476
+
1477
+ // If the delta is less than the maximum acceptable rate and the previous value was near the cap
1478
+ // then this is an overflow. There can be false positives such that a reset is detected as an
1479
+ // overflow.
1480
+ // TODO: remember recent history of rates and compare with current rate to reduce this chance.
1481
+ if (delta < max_acceptable_rate) {
1482
+ rd->calculated_value +=
1483
+ (calculated_number) delta
1484
+ * (calculated_number) rd->multiplier
1485
+ / (calculated_number) rd->divisor;
1486
+ } else {
1487
+ // This is a reset. Any overflow with a rate greater than MAX_INCREMENTAL_PERCENT_RATE will also
1488
+ // be detected as a reset instead.
1489
+ rd->calculated_value += (calculated_number)0;
1490
+ }
1491
}
1492
else {
1493
rd->calculated_value +=
libnetdata/storage_number/storage_number.h
+4
@@ -87,4 +87,8 @@ int print_calculated_number(char *str, calculated_number value);
87
#define ACCURACY_LOSS_ACCEPTED_PERCENT 0.0001
88
#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)))))
89
90
+// Maximum acceptable rate of increase for counters. With a rate of 10% netdata can safely detect overflows with a
91
+// period of at least every other 10 samples.
92
+#define MAX_INCREMENTAL_PERCENT_RATE 10
93
+
94
#endif /* NETDATA_STORAGE_NUMBER_H */