[rrd-developers] [PATCH] rrdcached ensures time marches forward

[kevin brintnall]

This patch introduces a feature whereby rrdcached will disallow updates
that do not advance the update time.  This prevents the updates from being
discarded later by rrd_update_r.

This patch attempts to make the most of the protocol's limited ability to
return error text when using a -1 return code.

---
diff --git a/doc/rrdcached.pod b/doc/rrdcached.pod
index fd56523..4d079a9 100644
--- a/doc/rrdcached.pod
+++ b/doc/rrdcached.pod
@@ -409,6 +409,10 @@ Adds more data to a filename. This is B<the> operation the daemon was designed
 for, so describing the mechanism again is unnecessary. Read L<HOW IT WORKS>
 above for a detailed explanation.
 
+Note that rrdcached only accepts absolute timestamps in the update values.
+Updates strings like "N:1:2:3" are automatically converted to absolute
+time by the RRD client library before sending to rrdcached.
+
 =item B<WROTE> I<filename>
 
 This command is written to the journal after a file is successfully
@@ -434,8 +438,8 @@ message itself.  The first user command after B<BATCH> is command number one.
 
     client:  BATCH
     server:  0 Go ahead.  End with dot '.' on its own line.
-    client:  UPDATE x.rrd N:1:2:3            <--- command #1
-    client:  UPDATE y.rrd N:3:4:5            <--- command #2
+    client:  UPDATE x.rrd 1223661439:1:2:3            <--- command #1
+    client:  UPDATE y.rrd 1223661440:3:4:5            <--- command #2
     client:  and so on...
     client:  .
     server:  2 Errors
@@ -469,10 +473,11 @@ daemon was started.
 
 =item B<DataSetsWritten> I<(unsigned 64bit integer)>
 
-Total number of "data sets" written to disk since the daemon was started. A
-data set is one or more values passed to the B<UPDATE> command. For example:
-C<N:123:456> is one data set with two values. The term "data set" is used to
-prevent confusion whether individual values or groups of values are counted.
+Total number of "data sets" written to disk since the daemon was
+started. A data set is one or more values passed to the B<UPDATE>
+command. For example: C<1223661439:123:456> is one data set with two
+values. The term "data set" is used to prevent confusion whether
+individual values or groups of values are counted.
 
 =item B<TreeNodesNumber> I<(unsigned 64bit integer)>
 
diff --git a/src/rrd_daemon.c b/src/rrd_daemon.c
index 5e774cb..3e92530 100644
--- a/src/rrd_daemon.c
+++ b/src/rrd_daemon.c
@@ -138,6 +138,7 @@ struct cache_item_s
   char **values;
   int values_num;
   time_t last_flush_time;
+  time_t last_update_stamp;
 #define CI_FLAGS_IN_TREE  (1<<0)
 #define CI_FLAGS_IN_QUEUE (1<<1)
   int flags;
@@ -1311,6 +1312,7 @@ static int handle_request_update (listen_socket_t *sock, /* {{{ */
 {
   char *file;
   int values_num = 0;
+  int bad_timestamps = 0;
   int status;
   char orig_buf[CMD_MAX];
 
@@ -1401,6 +1403,8 @@ static int handle_request_update (listen_socket_t *sock, /* {{{ */
   {
     char **temp;
     char *value;
+    time_t stamp;
+    char *eostamp;
 
     status = buffer_get_field (&buffer, &buffer_size, &value);
     if (status != 0)
@@ -1409,6 +1413,26 @@ static int handle_request_update (listen_socket_t *sock, /* {{{ */
       break;
     }
 
+    /* make sure update time is always moving forward */
+    stamp = strtol(value, &eostamp, 10);
+    if (eostamp == value || eostamp == NULL || *eostamp != ':')
+    {
+      ++bad_timestamps;
+      add_response_info(sock, "Cannot find timestamp in '%s'!\n", value);
+      continue;
+    }
+    else if (stamp <= ci->last_update_stamp)
+    {
+      ++bad_timestamps;
+      add_response_info(sock,
+                        "illegal attempt to update using time %ld when"
+                        " last update time is %ld (minimum one second step)\n",
+                        stamp, ci->last_update_stamp);
+      continue;
+    }
+    else
+      ci->last_update_stamp = stamp;
+
     temp = (char **) realloc (ci->values,
         sizeof (char *) * (ci->values_num + 1));
     if (temp == NULL)
@@ -1439,9 +1463,21 @@ static int handle_request_update (listen_socket_t *sock, /* {{{ */
   pthread_mutex_unlock (&cache_lock);
 
   if (values_num < 1)
-    return send_response(sock, RESP_ERR, "No values updated.\n");
+  {
+    /* if we had only one update attempt, then return the full
+       error message... try to get the most information out
+       of the limited error space allowed by the protocol
+    */
+    if (bad_timestamps == 1)
+      return send_response(sock, RESP_ERR, "%s", sock->wbuf);
+    else
+      return send_response(sock, RESP_ERR,
+                           "No values updated (%d bad timestamps).\n",
+                           bad_timestamps);
+  }
   else
-    return send_response(sock, RESP_OK, "Enqueued %i value(s).\n", values_num);
+    return send_response(sock, RESP_OK,
+                         "errors, enqueued %i value(s).\n", values_num);
 
   /* NOTREACHED */
   assert(1==0);