Merge pull request #2349 from rpv-tomsk/fix-register-complex-read
authorFlorian Forster <ff@octo.it>
Mon, 24 Jul 2017 07:31:05 +0000 (09:31 +0200)
committerGitHub <noreply@github.com>
Mon, 24 Jul 2017 07:31:05 +0000 (09:31 +0200)
Free `userdata` if plugin_register_complex_read() has failed.

src/collectd.conf.pod
src/intel_pmu.c
src/processes.c
src/rrdtool.c
src/types.db

index 2bf1a55..2566aa1 100644 (file)
@@ -7043,14 +7043,20 @@ one (exclusive).
 
 When the C<rrdtool> plugin uses a cache (by setting B<CacheTimeout>, see below)
 it writes all values for a certain RRD-file if the oldest value is older than
-(or equal to) the number of seconds specified. If some RRD-file is not updated
+(or equal to) the number of seconds specified by B<CacheTimeout>.
+That check happens on new values arriwal. If some RRD-file is not updated
 anymore for some reason (the computer was shut down, the network is broken,
-etc.) some values may still be in the cache. If B<CacheFlush> is set, then the
-entire cache is searched for entries older than B<CacheTimeout> seconds and
-written to disk every I<Seconds> seconds. Since this is kind of expensive and
-does nothing under normal circumstances, this value should not be too small.
-900 seconds might be a good value, though setting this to 7200 seconds doesn't
-normally do much harm either.
+etc.) some values may still be in the cache. If B<CacheFlush> is set, then
+every I<Seconds> seconds the entire cache is searched for entries older than
+B<CacheTimeout> + B<RandomTimeout> seconds. The entries found are written to
+disk. Since scanning the entire cache is kind of expensive and does nothing
+under normal circumstances, this value should not be too small. 900 seconds
+might be a good value, though setting this to 7200 seconds doesn't normally
+do much harm either.
+
+Defaults to 10x B<CacheTimeout>.
+B<CacheFlush> must be larger than or equal to B<CacheTimeout>, otherwise the
+above default is used.
 
 =item B<CacheTimeout> I<Seconds>
 
index ea7c83f..e5f19ce 100644 (file)
@@ -273,9 +273,9 @@ static void pmu_submit_counter(int cpu, char *event, counter_t value) {
 
   sstrncpy(vl.plugin, PMU_PLUGIN, sizeof(vl.plugin));
   if (cpu == -1) {
-    ssnprintf(vl.plugin_instance, sizeof(vl.plugin_instance), "all");
+    snprintf(vl.plugin_instance, sizeof(vl.plugin_instance), "all");
   } else {
-    ssnprintf(vl.plugin_instance, sizeof(vl.plugin_instance), "%d", cpu);
+    snprintf(vl.plugin_instance, sizeof(vl.plugin_instance), "%d", cpu);
   }
   sstrncpy(vl.type, "counter", sizeof(vl.type));
   sstrncpy(vl.type_instance, event, sizeof(vl.type_instance));
index 4fec161..727ec7f 100644 (file)
@@ -185,6 +185,8 @@ typedef struct process_entry_s {
   derive_t io_wchar;
   derive_t io_syscr;
   derive_t io_syscw;
+  derive_t io_diskr;
+  derive_t io_diskw;
   _Bool has_io;
 
   derive_t cswitch_vol;
@@ -209,6 +211,8 @@ typedef struct procstat_entry_s {
   derive_t io_wchar;
   derive_t io_syscr;
   derive_t io_syscw;
+  derive_t io_diskr;
+  derive_t io_diskw;
 
   derive_t cswitch_vol;
   derive_t cswitch_invol;
@@ -242,6 +246,8 @@ typedef struct procstat {
   derive_t io_wchar;
   derive_t io_syscr;
   derive_t io_syscw;
+  derive_t io_diskr;
+  derive_t io_diskw;
 
   derive_t cswitch_vol;
   derive_t cswitch_invol;
@@ -310,6 +316,8 @@ static procstat_t *ps_list_register(const char *name, const char *regexp) {
   new->io_wchar = -1;
   new->io_syscr = -1;
   new->io_syscw = -1;
+  new->io_diskr = -1;
+  new->io_diskw = -1;
   new->cswitch_vol = -1;
   new->cswitch_invol = -1;
 
@@ -480,6 +488,11 @@ static void ps_list_add(const char *name, const char *cmdline,
       ps_update_counter(&ps->io_syscw, &pse->io_syscw, entry->io_syscw);
     }
 
+    if ((entry->io_diskr != -1) && (entry->io_diskw != -1)) {
+      ps_update_counter(&ps->io_diskr, &pse->io_diskr, entry->io_diskr);
+      ps_update_counter(&ps->io_diskw, &pse->io_diskw, entry->io_diskw);
+    }
+
     if ((entry->cswitch_vol != -1) && (entry->cswitch_vol != -1)) {
       ps_update_counter(&ps->cswitch_vol, &pse->cswitch_vol,
                         entry->cswitch_vol);
@@ -725,7 +738,7 @@ static void ps_submit_proc_list(procstat_t *ps) {
   plugin_dispatch_values(&vl);
 
   if ((ps->io_rchar != -1) && (ps->io_wchar != -1)) {
-    sstrncpy(vl.type, "ps_disk_octets", sizeof(vl.type));
+    sstrncpy(vl.type, "io_octets", sizeof(vl.type));
     vl.values[0].derive = ps->io_rchar;
     vl.values[1].derive = ps->io_wchar;
     vl.values_len = 2;
@@ -733,13 +746,21 @@ static void ps_submit_proc_list(procstat_t *ps) {
   }
 
   if ((ps->io_syscr != -1) && (ps->io_syscw != -1)) {
-    sstrncpy(vl.type, "ps_disk_ops", sizeof(vl.type));
+    sstrncpy(vl.type, "io_ops", sizeof(vl.type));
     vl.values[0].derive = ps->io_syscr;
     vl.values[1].derive = ps->io_syscw;
     vl.values_len = 2;
     plugin_dispatch_values(&vl);
   }
 
+  if ((ps->io_diskr != -1) && (ps->io_diskw != -1)) {
+    sstrncpy(vl.type, "disk_octets", sizeof(vl.type));
+    vl.values[0].derive = ps->io_diskr;
+    vl.values[1].derive = ps->io_diskw;
+    vl.values_len = 2;
+    plugin_dispatch_values(&vl);
+  }
+
   if (ps->num_fd > 0) {
     sstrncpy(vl.type, "file_handles", sizeof(vl.type));
     vl.values[0].gauge = ps->num_fd;
@@ -768,12 +789,13 @@ static void ps_submit_proc_list(procstat_t *ps) {
         "cpu_user_counter = %" PRIi64 "; cpu_system_counter = %" PRIi64 "; "
         "io_rchar = %" PRIi64 "; io_wchar = %" PRIi64 "; "
         "io_syscr = %" PRIi64 "; io_syscw = %" PRIi64 "; "
+        "io_diskr = %" PRIi64 "; io_diskw = %" PRIi64 "; "
         "cswitch_vol = %" PRIi64 "; cswitch_invol = %" PRIi64 ";",
         ps->name, ps->num_proc, ps->num_lwp, ps->num_fd, ps->vmem_size,
         ps->vmem_rss, ps->vmem_data, ps->vmem_code, ps->vmem_minflt_counter,
         ps->vmem_majflt_counter, ps->cpu_user_counter, ps->cpu_system_counter,
-        ps->io_rchar, ps->io_wchar, ps->io_syscr, ps->io_syscw, ps->cswitch_vol,
-        ps->cswitch_invol);
+        ps->io_rchar, ps->io_wchar, ps->io_syscr, ps->io_syscw, ps->io_diskr,
+        ps->io_diskw, ps->cswitch_vol, ps->cswitch_invol);
 } /* void ps_submit_proc_list */
 
 #if KERNEL_LINUX || KERNEL_SOLARIS
@@ -954,6 +976,10 @@ static int ps_read_io(process_entry_t *ps) {
       val = &(ps->io_syscr);
     else if (strncasecmp(buffer, "syscw:", 6) == 0)
       val = &(ps->io_syscw);
+    else if (strncasecmp(buffer, "read_bytes:", 11) == 0)
+      val = &(ps->io_diskr);
+    else if (strncasecmp(buffer, "write_bytes:", 12) == 0)
+      val = &(ps->io_diskw);
     else
       continue;
 
@@ -1149,6 +1175,8 @@ static int ps_read_process(long pid, process_entry_t *ps, char *state) {
   ps->io_wchar = -1;
   ps->io_syscr = -1;
   ps->io_syscw = -1;
+  ps->io_diskr = -1;
+  ps->io_diskw = -1;
 
   ps->cswitch_vol = -1;
   ps->cswitch_invol = -1;
@@ -1407,6 +1435,8 @@ static int ps_read_process(long pid, process_entry_t *ps, char *state) {
   ps->io_wchar = myUsage->pr_oublk * chars_per_block;
   ps->io_syscr = myUsage->pr_sysc;
   ps->io_syscw = myUsage->pr_sysc;
+  ps->io_diskr = -1;
+  ps->io_diskw = -1;
 
   /*
    * TODO: context switch counters for Solaris
@@ -1620,6 +1650,8 @@ static int ps_read(void) {
         pse.io_wchar = -1;
         pse.io_syscr = -1;
         pse.io_syscw = -1;
+        pse.io_diskr = -1;
+        pse.io_diskw = -1;
 
         /* File descriptor count not implemented */
         pse.num_fd = 0;
@@ -1923,6 +1955,8 @@ static int ps_read(void) {
       pse.io_wchar = -1;
       pse.io_syscr = -1;
       pse.io_syscw = -1;
+      pse.io_diskr = -1;
+      pse.io_diskw = -1;
 
       /* file descriptor count not implemented */
       pse.num_fd = 0;
@@ -2062,6 +2096,8 @@ static int ps_read(void) {
       pse.io_wchar = -1;
       pse.io_syscr = -1;
       pse.io_syscw = -1;
+      pse.io_diskr = -1;
+      pse.io_diskw = -1;
 
       /* file descriptor count not implemented */
       pse.num_fd = 0;
@@ -2225,6 +2261,8 @@ static int ps_read(void) {
       pse.io_wchar = -1;
       pse.io_syscr = -1;
       pse.io_syscw = -1;
+      pse.io_diskr = -1;
+      pse.io_diskw = -1;
 
       pse.num_fd = 0;
 
index 4128905..2dfa87a 100644 (file)
@@ -87,7 +87,7 @@ static rrdcreate_config_t rrdcreate_config = {
  * ALWAYS lock `cache_lock' first! */
 static cdtime_t cache_timeout = 0;
 static cdtime_t cache_flush_timeout = 0;
-static cdtime_t random_timeout = TIME_T_TO_CDTIME_T_STATIC(1);
+static cdtime_t random_timeout = 0;
 static cdtime_t cache_flush_last;
 static c_avl_tree_t *cache = NULL;
 static pthread_mutex_t cache_lock = PTHREAD_MUTEX_INITIALIZER;
@@ -505,7 +505,6 @@ static void rrd_cache_flush(cdtime_t timeout) {
         CDTIME_T_TO_DOUBLE(timeout));
 
   now = cdtime();
-  timeout = TIME_T_TO_CDTIME_T(timeout);
 
   /* Build a list of entries to be flushed */
   iter = c_avl_get_iterator(cache);
@@ -606,23 +605,10 @@ static int rrd_cache_flush_identifier(cdtime_t timeout,
 } /* int rrd_cache_flush_identifier */
 
 static int64_t rrd_get_random_variation(void) {
-  long min;
-  long max;
-
   if (random_timeout == 0)
     return 0;
 
-  /* Assure that "cache_timeout + random_variation" is never negative. */
-  if (random_timeout > cache_timeout) {
-    INFO("rrdtool plugin: Adjusting \"RandomTimeout\" to %.3f seconds.",
-         CDTIME_T_TO_DOUBLE(cache_timeout));
-    random_timeout = cache_timeout;
-  }
-
-  max = (long)(random_timeout / 2);
-  min = max - ((long)random_timeout);
-
-  return (int64_t)cdrand_range(min, max);
+  return (int64_t)cdrand_range(-random_timeout, random_timeout);
 } /* int64_t rrd_get_random_variation */
 
 static int rrd_cache_insert(const char *filename, const char *value,
@@ -740,7 +726,7 @@ static int rrd_cache_insert(const char *filename, const char *value,
 
   if ((cache_timeout > 0) &&
       ((cdtime() - cache_flush_last) > cache_flush_timeout))
-    rrd_cache_flush(cache_flush_timeout);
+    rrd_cache_flush(cache_timeout + random_timeout);
 
   pthread_mutex_unlock(&cache_lock);
 
@@ -877,7 +863,7 @@ static int rrd_config(const char *key, const char *value) {
     }
     cache_timeout = DOUBLE_TO_CDTIME_T(tmp);
   } else if (strcasecmp("CacheFlush", key) == 0) {
-    int tmp = atoi(value);
+    double tmp = atof(value);
     if (tmp < 0) {
       fprintf(stderr, "rrdtool: `CacheFlush' must "
                       "be greater than 0.\n");
@@ -885,7 +871,7 @@ static int rrd_config(const char *key, const char *value) {
             "be greater than 0.\n");
       return 1;
     }
-    cache_flush_timeout = tmp;
+    cache_flush_timeout = DOUBLE_TO_CDTIME_T(tmp);
   } else if (strcasecmp("DataDir", key) == 0) {
     char *tmp;
     size_t len;
@@ -1065,9 +1051,23 @@ static int rrd_init(void) {
 
   cache_flush_last = cdtime();
   if (cache_timeout == 0) {
+    random_timeout = 0;
     cache_flush_timeout = 0;
-  } else if (cache_flush_timeout < cache_timeout)
+  } else if (cache_flush_timeout < cache_timeout) {
+    INFO("rrdtool plugin: \"CacheFlush %.3f\" is less than \"CacheTimeout %.3f\". "
+         "Ajusting \"CacheFlush\" to %.3f seconds.",
+         CDTIME_T_TO_DOUBLE(cache_flush_timeout),
+         CDTIME_T_TO_DOUBLE(cache_timeout),
+         CDTIME_T_TO_DOUBLE(cache_timeout * 10));
     cache_flush_timeout = 10 * cache_timeout;
+  }
+
+  /* Assure that "cache_timeout + random_variation" is never negative. */
+  if (random_timeout > cache_timeout) {
+    INFO("rrdtool plugin: Adjusting \"RandomTimeout\" to %.3f seconds.",
+         CDTIME_T_TO_DOUBLE(cache_timeout));
+    random_timeout = cache_timeout;
+  }
 
   pthread_mutex_unlock(&cache_lock);
 
index c9838e1..9ba33e8 100644 (file)
@@ -117,6 +117,7 @@ if_tx_octets            value:DERIVE:0:U
 if_tx_packets           value:DERIVE:0:U
 invocations             value:DERIVE:0:U
 io_octets               rx:DERIVE:0:U, tx:DERIVE:0:U
+io_ops                  read:DERIVE:0:U, write:DERIVE:0:U
 io_packets              rx:DERIVE:0:U, tx:DERIVE:0:U
 ipc                     value:GAUGE:0:U
 ipt_bytes               value:DERIVE:0:U