[rrd-developers] [PATCH] make sure "flush" writes before returning

kevin brintnall kbrint at rufus.net
Fri Sep 26 03:42:12 CEST 2008


This patch ensures that the "FLUSH" command will write the updates out to
RRD before returning to the user.  Before, it returned when the update was
"dequeued"; updates were not necessarily on disk.

Also, for new nodes, the cache_lock is not held while we are setting up
the new node.  We don't want to be holding the lock if the stat() blocks.

-- 
 kevin brintnall =~ /kbrint at rufus.net/

-----------------------------------------------------------------

diff --git a/doc/rrdcached.pod b/doc/rrdcached.pod
index 3505728..bf289c5 100644
--- a/doc/rrdcached.pod
+++ b/doc/rrdcached.pod
@@ -187,9 +187,8 @@ The downside of caching values is that they won't show up in graphs generated
 from the RRDE<nbsp>files. To get around this, the daemon provides the "flush
 command" to flush specific files. This means that the file is inserted at the
 B<head> of the update queue or moved there if it is already enqueued. The flush
-command will return after the update thread has dequeued the file, so there is
-a good chance that the file has been updated by the time the client receives
-the response from the daemon, but there is no guarantee.
+command will return only after the file's pending updates have been written
+to disk.
 
  +------+   +------+                               +------+
  ! head !   ! root !                               ! tail !
diff --git a/doc/rrdflush.pod b/doc/rrdflush.pod
index a593821..514bd35 100644
--- a/doc/rrdflush.pod
+++ b/doc/rrdflush.pod
@@ -12,9 +12,7 @@ S<[B<--daemon> I<address>]>
 The B<flush> function connects to L<rrdcached>, the RRD caching daemon, and
 issues a "flush" command for the given file. The daemon will put this file to
 the head of the update queue so it is written "soon". The status will be
-returned after the node has been B<dequeued> by the update thread. By the time
-execution of this command ends it is very likely that the update thread has
-just updated the requested file, though this is not guaranteed.
+returned only after the file's pending updates have been written to disk.
 
 =over 8
 
diff --git a/src/rrd_daemon.c b/src/rrd_daemon.c
index c438028..1c02a49 100644
--- a/src/rrd_daemon.c
+++ b/src/rrd_daemon.c
@@ -119,7 +119,7 @@ struct cache_item_s
 #define CI_FLAGS_IN_TREE  (1<<0)
 #define CI_FLAGS_IN_QUEUE (1<<1)
   int flags;
-
+  pthread_cond_t  flushed;
   cache_item_t *next;
 };
 
@@ -165,8 +165,6 @@ static cache_item_t   *cache_queue_tail = NULL;
 static pthread_mutex_t cache_lock = PTHREAD_MUTEX_INITIALIZER;
 static pthread_cond_t  cache_cond = PTHREAD_COND_INITIALIZER;
 
-static pthread_cond_t  flush_cond = PTHREAD_COND_INITIALIZER;
-
 static int config_write_interval = 300;
 static int config_write_jitter   = 0;
 static int config_flush_interval = 3600;
@@ -640,6 +638,7 @@ static void *queue_thread_main (void *args __attribute__((unused))) /* {{{ */
     }
 
     journal_write("wrote", file);
+    pthread_cond_broadcast(&ci->flushed);
 
     for (i = 0; i < values_num; i++)
       free (values[i]);
@@ -656,7 +655,6 @@ static void *queue_thread_main (void *args __attribute__((unused))) /* {{{ */
     }
 
     pthread_mutex_lock (&cache_lock);
-    pthread_cond_broadcast (&flush_cond);
 
     /* We're about to shut down, so lets flush the entire tree. */
     if ((do_shutdown != 0) && (cache_queue_head == NULL))
@@ -751,23 +749,9 @@ static int flush_file (const char *filename) /* {{{ */
   enqueue_cache_item (ci, HEAD);
   pthread_cond_signal (&cache_cond);
 
-  while ((ci->flags & CI_FLAGS_IN_QUEUE) != 0)
-  {
-    ci = NULL;
-
-    pthread_cond_wait (&flush_cond, &cache_lock);
-
-    ci = g_tree_lookup (cache_tree, filename);
-    if (ci == NULL)
-    {
-      RRDD_LOG (LOG_ERR, "flush_file: Tree node went away "
-          "while waiting for flush.");
-      pthread_mutex_unlock (&cache_lock);
-      return (-1);
-    }
-  }
+  pthread_cond_wait(&ci->flushed, &cache_lock);
+  pthread_mutex_unlock(&cache_lock);
 
-  pthread_mutex_unlock (&cache_lock);
   return (0);
 } /* }}} int flush_file */
 
@@ -1043,17 +1027,19 @@ static int handle_request_update (int fd, /* {{{ */
   pthread_mutex_unlock(&stats_lock);
 
   pthread_mutex_lock (&cache_lock);
-
   ci = g_tree_lookup (cache_tree, file);
+
   if (ci == NULL) /* {{{ */
   {
     struct stat statbuf;
 
+    /* don't hold the lock while we setup; stat(2) might block */
+    pthread_mutex_unlock(&cache_lock);
+
     memset (&statbuf, 0, sizeof (statbuf));
     status = stat (file, &statbuf);
     if (status != 0)
     {
-      pthread_mutex_unlock (&cache_lock);
       RRDD_LOG (LOG_NOTICE, "handle_request_update: stat (%s) failed.", file);
 
       status = errno;
@@ -1067,16 +1053,12 @@ static int handle_request_update (int fd, /* {{{ */
     }
     if (!S_ISREG (statbuf.st_mode))
     {
-      pthread_mutex_unlock (&cache_lock);
-
       snprintf (answer, sizeof (answer), "-1 Not a regular file: %s\n", file);
       RRDD_UPDATE_SEND;
       return (0);
     }
     if (access(file, R_OK|W_OK) != 0)
     {
-      pthread_mutex_unlock (&cache_lock);
-
       snprintf (answer, sizeof (answer), "-1 Cannot read/write %s: %s\n",
                 file, rrd_strerror(errno));
       RRDD_UPDATE_SEND;
@@ -1086,7 +1068,6 @@ static int handle_request_update (int fd, /* {{{ */
     ci = (cache_item_t *) malloc (sizeof (cache_item_t));
     if (ci == NULL)
     {
-      pthread_mutex_unlock (&cache_lock);
       RRDD_LOG (LOG_ERR, "handle_request_update: malloc failed.");
 
       strncpy (answer, "-1 malloc failed.\n", sizeof (answer));
@@ -1098,7 +1079,6 @@ static int handle_request_update (int fd, /* {{{ */
     ci->file = strdup (file);
     if (ci->file == NULL)
     {
-      pthread_mutex_unlock (&cache_lock);
       free (ci);
       RRDD_LOG (LOG_ERR, "handle_request_update: strdup failed.");
 
@@ -1110,6 +1090,7 @@ static int handle_request_update (int fd, /* {{{ */
     _wipe_ci_values(ci, now);
     ci->flags = CI_FLAGS_IN_TREE;
 
+    pthread_mutex_lock(&cache_lock);
     g_tree_insert (cache_tree, (void *) ci->file, (void *) ci);
   } /* }}} */
   assert (ci != NULL);



More information about the rrd-developers mailing list