Fixed handling of unknown data at PDP build time. There was a long standing

(even documented) missfeature in rrdtool which caused uknown-data to be
accepted as long as it was less than the mrhb. This was never the intended
behaviour and had interesting side effects
(http://oss.oetiker.ch/rrdtool-trac/ticket/125): If you have a 60 Second
step with 59s unknown data and 1 second of known data the whole become known
data as long as the mrhb was > step. The intended behaviour was that a step
should become unknown as soon as it contains more than 50% of unknown data.
The patch fixes both the documentation and the code.

git-svn-id: svn://svn.oetiker.ch/rrdtool/trunk/program@1340 a5681a0c-68f1-0310-ab6d-d61299d08faa

diff --git a/doc/rrdcreate.pod b/doc/rrdcreate.pod
index 37eacbe..86817bd 100644 (file)
--- a/doc/rrdcreate.pod
+++ b/doc/rrdcreate.pod
@@ -399,28 +399,24 @@ Here is an explanation by Don Baarda on the inner workings of RRDtool.
 It may help you to sort out why all this *UNKNOWN* data is popping
 up in your databases:
 
-RRDtool gets fed samples at arbitrary times. From these it builds Primary
-Data Points (PDPs) at exact times on every "step" interval. The PDPs are
-then accumulated into RRAs.
+RRDtool gets fed samples/updates at arbitrary times. From these it builds Primary
+Data Points (PDPs) on every "step" interval. The PDPs are
+then accumulated into the RRAs.
 
 The "heartbeat" defines the maximum acceptable interval between
-samples. If the interval between samples is less than "heartbeat",
+samples/updates. If the interval between samples is less than "heartbeat",
 then an average rate is calculated and applied for that interval. If
 the interval between samples is longer than "heartbeat", then that
 entire interval is considered "unknown". Note that there are other
 things that can make a sample interval "unknown", such as the rate
-exceeding limits, or even an "unknown" input sample.
+exceeding limits, or a sample that was explicitly marked as unknown.
 
 The known rates during a PDP's "step" interval are used to calculate
-an average rate for that PDP. Also, if the total "unknown" time during
-the "step" interval exceeds the "heartbeat", the entire PDP is marked
+an average rate for that PDP. If the total "unknown" time accounts for
+more than B<half> the "step", the entire PDP is marked
 as "unknown". This means that a mixture of known and "unknown" sample
-times in a single PDP "step" may or may not add up to enough "unknown"
-time to exceed "heartbeat" and hence mark the whole PDP "unknown". So
-"heartbeat" is not only the maximum acceptable interval between
-samples, but also the maximum acceptable amount of "unknown" time per
-PDP (obviously this is only significant if you have "heartbeat" less
-than "step").
+times in a single PDP "step" may or may not add up to enough "known"
+time to warrent for a known PDP.
 
 The "heartbeat" can be short (unusual) or long (typical) relative to
 the "step" interval between PDPs. A short "heartbeat" means you
@@ -452,7 +448,7 @@ same average rate. I<-- Don Baarda E<lt>don.baarda@baesystems.comE<gt>>
        u|15|/     "swt" expired
        u|16|
         |17|----* sample4, restart "hb", create "pdp" for step1 = 
-        |18|   /  = unknown due to 10 "u" labled secs > "hb"
+        |18|   /  = unknown due to 10 "u" labled secs > 0.5 * step
         |19|  /
         |20| /
         |21|----* sample5, restart "hb"

diff --git a/src/rrd_update.c b/src/rrd_update.c
index c842b9b..6bd04b2 100644 (file)
--- a/src/rrd_update.c
+++ b/src/rrd_update.c
@@ -1,3 +1,4 @@
+
 /*****************************************************************************
  * RRDtool 1.2.99907080300  Copyright by Tobi Oetiker, 1997-2007
  *****************************************************************************
@@ -58,6 +59,8 @@ static int gettimeofday(
 
 #endif
 
+#define DEBUG 1
+
 /* FUNCTION PROTOTYPES */
 
 int       rrd_update_r(
@@ -1240,10 +1243,13 @@ static int process_all_pdp_st(
         }
 #ifdef DEBUG
         fprintf(stderr, "PDP UPD ds[%lu]\t"
+                "elapsed_pdp_st %lu\t"
                 "pdp_temp %10.2f\t"
                 "new_prep %10.2f\t"
                 "new_unkn_sec %5lu\n",
-                ds_idx, pdp_temp[ds_idx],
+                ds_idx,
+                elapsed_pdp_st,
+                pdp_temp[ds_idx],
                 rrd->pdp_prep[ds_idx].scratch[PDP_val].u_val,
                 rrd->pdp_prep[ds_idx].scratch[PDP_unkn_sec_cnt].u_cnt);
 #endif
@@ -1264,7 +1270,7 @@ static int process_pdp_st(
     double interval,
     double pre_int,
     double post_int,
-    long diff_pdp_st,
+    long diff_pdp_st, /* number of seconds in full steps passed since last update */
     rrd_value_t *pdp_new,
     rrd_value_t *pdp_temp)
 {
@@ -1281,7 +1287,7 @@ static int process_pdp_st(
 
 
     if (isnan(pdp_new[ds_idx])) {
-        /* a final bit of unknown to be added bevore calculation
+        /* a final bit of unknown to be added before calculation
            we use a temporary variable for this so that we
            don't have to turn integer lines before using the value */
         pre_unknown = pre_int;
@@ -1295,7 +1301,7 @@ static int process_pdp_st(
     /* if too much of the pdp_prep is unknown we dump it */
     /* if the interval is larger thatn mrhb we get NAN */
     if ((interval > mrhb) ||
-        (diff_pdp_st <= (signed) scratch[PDP_unkn_sec_cnt].u_cnt)) {
+        (rrd->stat_head->pdp_step/2.0 < (signed) scratch[PDP_unkn_sec_cnt].u_cnt)) {
         pdp_temp[ds_idx] = DNAN;
     } else {
         pdp_temp[ds_idx] = scratch[PDP_val].u_val /