]> git.ipfire.org Git - thirdparty/rrdtool-1.x.git/commitdiff
Allow to select locking mechanism (#1207)
authorEnrico Scholz <github@ensc.de>
Thu, 2 Mar 2023 16:00:25 +0000 (17:00 +0100)
committerGitHub <noreply@github.com>
Thu, 2 Mar 2023 16:00:25 +0000 (17:00 +0100)
* rrd: add _rrd_lock_xxx() helper functions and constants

This adds two sets of constants: these used in 'extra_flags' in various
parts of the extended api (e.g. rrd_updatex_r()), and these used within
rrd_open().

There are implemented some helper functions which help to parse command
line strings and environment variables, and to convert these two sets
of constants.

* rrd_update: pass custom RRD_LOCK_xxx flags to rrd_open()

* rrd_update: add '--locking' cli option

* rrd_open: handle RRD_LOCK_DEFAULT

When rrd_open() was called with RRD_LOCK_DEFAULT, read the locking
setup from $RRD_LOCKING environment.

* rrd_open: implement other locking methods

Allow locking to wait and make it possible to bypass locking
completely.

With this patch it is e.g. expected:

| $ RRD_LOCKING=none strace -e fcntl rrdupdate --locking block x N:1
| fcntl(3, F_SETLKW, {l_type=F_WRLCK, l_whence=SEEK_SET, l_start=0, l_len=0}) = 0

Other values for RRD_LOCKING and --locking give

 | RRD_LOCKING=... | --locking ... | fcntl()  |
 |-----------------|---------------|----------|
 | *               |               | F_SETLK  |
 | *               | block         | F_SETLKW |
 | *               | try           | F_SETLK  |
 | *               | none          |          |
 | block           |               | F_SETLKW |
 | try             |               | F_SETLK  |
 | none            |               |          |

* doc: document --locking + $RRD_LOCKING

Signed-off-by: Enrico Scholz <enrico.scholz@ensc.de>
CHANGES
doc/rrdtool.pod
doc/rrdupdate.pod
src/rrd.h
src/rrd_open.c
src/rrd_tool.h
src/rrd_update.c
src/rrdupdate.c

diff --git a/CHANGES b/CHANGES
index 5cfc048fb5f1a88d512cede406f1031c325384f8..f5dbe3ca5c88a7e78ab2fab6aa9a5bd10026d631 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,6 @@
 
-RRDtool git - 2022-04-28
-==========================
+RRDtool - master ... 
+======================
 
 Bugfixes
 --------
@@ -17,6 +17,27 @@ Features
 --------
 * Remove autogenerated files from git repo (configure, Makefile.in, conftools, rrd_config.h.in) <c72578>
 
+* Reads $RRD_LOCKING environment variable and adds --locking option to some tools. The updatex api has been also
+  updated to support setting locking related bits in its extra_flags parameter. @ensc
+
+  This allows now to choose between three kinds of locking:
+
+    none: no locking is done at all; caller has to do it manually and can implement e.g. a timeout with alarm(2) or so
+    try: fails when lock is hold by another process; users will see "ERROR: could not lock RRD". This is the default and the only possible mode with the old code
+    block: waits until lock is available
+
+  It can be used like
+
+    env RRD_LOCKING=block rrdupdate ...
+
+  or
+
+    rrdupdate --locking none ...
+
+  or
+
+  rrd_updatex_r(filename, tmplt, RRD_FLAGS_LOCKING_MODE_BLOCK, ...);
+
 
 RRDtool 1.8.0 - 2022-03-13
 ==========================
index c1baf6885b83cb3ee4173ec85b1f37c423c5adcb..df717cefec53ef3ccadfcefaf47f280cb4bc25a7 100644 (file)
@@ -314,6 +314,40 @@ sockets, tools like netcat, or in a quick interactive test by using
 B<NOTE:> that there is no authentication with this feature! Do not setup
 such a port unless you are sure what you are doing.
 
+=head1 ENVIRONMENT VARIABLES
+
+The following environment variables may be used to change the behavior
+of most of the utilities.
+
+=over
+
+=item B<RRD_LOCKING>
+
+If this environment variable is set, the B<RRD> file is locked in the
+given mode:
+
+=over
+
+=item -
+
+B<try> fails, when the file is locked by another process.  This is the default.
+
+=item -
+
+B<block> waits until the lock is released.
+
+=item -
+
+B<none> skips locking at all.  Caller has to ensure a proper locking
+which should be compatible with C<fcntl(fd, F_SETLK, ...)>.
+
+=back
+
+Some utilities have command line options (B<--locking>) which override
+this variable.
+
+=back
+
 =head1 RRDCACHED, THE CACHING DAEMON
 
 For very big setups, updating thousands of RRD files often becomes a serious IO
index 8bc0c2e7c2cefc02158392949623e4a58dc4a595..20a193dd15f316c5276ef6346a54d001a6dfdae8 100644 (file)
@@ -38,6 +38,14 @@ used with this command.
 
 The name of the B<RRD> you want to update.
 
+=item B<--locking>|B<-L> B<try>|B<block>|B<none>
+
+Lock the B<RRD> file in the given mode: B<try> fails, when the file
+is locked by another process, B<block> waits until the lock is
+released and B<none> skips locking at all.  The default is read
+from the B<$RRD_LOCKING> environment variable and falls back to
+B<try> when such a variable does not exist.
+
 =item B<--template>|B<-t> I<ds-name>[B<:>I<ds-name>]...
 
 By default, the B<update> function expects its data input in the order
@@ -132,6 +140,15 @@ filename prior to sending the filename to rrdcached.
 This is mostly intended to allow rrdcached to work with xymon and cacti tools
 without having to modify those tools.
 
+=item B<RRD_LOCKING>
+
+If this environment variable is set, the B<RRD> file is locked in the
+given mode: B<try> fails, when the file is locked by another process,
+B<block> waits until the lock is released and B<none> skips locking at
+all.
+
+This variable can be overridden by the B<--locking> command line option.
+
 =back
 
 =head1 EXAMPLES
index 30ac89c6350d26201e83becccf5303a238663f9b..184887ccc42d01f825141328e0a5e9be245bbe3d 100644 (file)
--- a/src/rrd.h
+++ b/src/rrd.h
@@ -281,6 +281,20 @@ extern    "C" {
 /* extra flags */
 #define RRD_SKIP_PAST_UPDATES 0x01
 
+/* Locking mode which can be set in 'extra_flags':
+ *
+ * DEFAULT ... read $RRD_LOCKING environment or fall back to TRY
+ * NONE  ... no locking; caller is responsible to ensure that the file is not
+ *           used else
+ * BLOCK ... wait until lock is available
+ * TRY   ... try to lock but fail when file is used elsewhere (default)
+ */
+#define RRD_FLAGS_LOCKING_MODE_NONE    (1 << 7)
+#define RRD_FLAGS_LOCKING_MODE_DEFAULT (0 << 7)
+#define RRD_FLAGS_LOCKING_MODE_BLOCK   (2 << 7)
+#define RRD_FLAGS_LOCKING_MODE_TRY     (3 << 7)
+#define RRD_FLAGS_LOCKING_MODE_MASK    (3 << 7)
+
     int       rrd_updatex_r(
     const char *filename,
     const char *_template,
index c326568aa85b68b2c2a87a22c2fb8cd0416ad6bb..c21684d2a72520589a8eb98de601245d77997140 100644 (file)
@@ -140,7 +140,8 @@ DWORD     dwCreationDisposition = 0;
 
 static int rrd_rwlock(
     rrd_file_t *rrd_file,
-    int writelock);
+    int writelock,
+    int lock_mode);
 static int close_and_unlock(
     int fd);
 
@@ -172,6 +173,11 @@ rrd_file_t *rrd_open(
     rrd_simple_file_t *rrd_simple_file = NULL;
     size_t    newfile_size = 0;
 
+    if ((rdwr & RRD_LOCK_MASK) == RRD_LOCK_DEFAULT) {
+        rdwr &= ~RRD_LOCK_MASK;
+        rdwr |= _rrd_lock_flags(_rrd_lock_default());
+    }
+
     /* Are we creating a new file? */
     if (rdwr & RRD_CREAT) {
         size_t    header_len, value_cnt, data_len;
@@ -341,11 +347,9 @@ rrd_file_t *rrd_open(
 #endif
 #endif
 
-    if (rdwr & RRD_LOCK) {
-        if (rrd_rwlock(rrd_file, rdwr & RRD_READWRITE) != 0) {
-            rrd_set_error("could not lock RRD");
-            goto out_close;
-        }
+    if (rrd_rwlock(rrd_file, rdwr & RRD_READWRITE, rdwr & RRD_LOCK_MASK) != 0) {
+        rrd_set_error("could not lock RRD");
+        goto out_close;
     }
 
     /* Better try to avoid seeks as much as possible. stat may be heavy but
@@ -663,7 +667,7 @@ void mincore_print(
 int rrd_lock(
     rrd_file_t *rrd_file)
 {
-    return rrd_rwlock(rrd_file, 1);
+    return rrd_rwlock(rrd_file, 1, RRD_LOCK_DEFAULT);
 }
 
 #if defined(_WIN32) && !defined(__CYGWIN__) && !defined(__CYGWIN32__)
@@ -761,8 +765,12 @@ int close_and_unlock(
 static
 int rrd_rwlock(
     rrd_file_t *rrd_file,
-    int writelock)
+    int writelock,
+    int lock_mode)
 {
+    if (lock_mode == RRD_LOCK_NONE)
+        return 0;
+
 #ifdef DISABLE_FLOCK
     (void) rrd_file;
     return 0;
@@ -795,6 +803,7 @@ int rrd_rwlock(
 #else
     {
         struct flock lock;
+        int op = lock_mode == RRD_LOCK_TRY ? F_SETLK : F_SETLKW;
 
         lock.l_type = writelock ? F_WRLCK : /* exclusive write lock or */
             F_RDLCK;    /* shared read lock */
@@ -802,7 +811,7 @@ int rrd_rwlock(
         lock.l_start = 0;   /* start of file */
         lock.l_whence = SEEK_SET;   /* end of file */
 
-        rcstat = fcntl(rrd_simple_file->fd, F_SETLK, &lock);
+        rcstat = fcntl(rrd_simple_file->fd, op, &lock);
     }
 #endif
 
@@ -1187,3 +1196,103 @@ unsigned long rrd_select_initial_row(
 {
     return rrd_random() % rra->row_cnt;
 }
+
+/*
+ * Translates a string in a RRD_FLAGS_LOCKING_xxx constant.
+ *
+ * Empty or non-existing strings are valid and will be mapped to a default
+ * value.
+ *
+ * Functions returns -1 on unsupported values but does not emit diagnostics.
+ */
+static int _rrd_lock_parse(const char *opt)
+{
+    /* non-existing and empty values */
+    if (!opt || !opt[0])
+        /* the default locking mode */
+        return RRD_FLAGS_LOCKING_MODE_TRY;
+    else if (strcmp(opt, "try") == 0)
+        return RRD_FLAGS_LOCKING_MODE_TRY;
+    else if (strcmp(opt, "block") == 0)
+        return RRD_FLAGS_LOCKING_MODE_BLOCK;
+    else if (strcmp(opt, "none") == 0)
+        return RRD_FLAGS_LOCKING_MODE_NONE;
+    else
+        return -1;
+}
+
+/*
+ * Returns the default locking method.
+ *
+ * It reads the $RRD_LOCKING environment.
+ *
+ * Function always succeeds; unsupported values will emit a
+ * diagnostic and function returns a default value in this case.
+ */
+int _rrd_lock_default(void)
+{
+    const char *opt = getenv("RRD_LOCKING");
+    int flags = _rrd_lock_parse(opt);
+
+    if (flags < 0) {
+        fprintf(stderr,
+                "unsupported locking mode '%s' in $RRD_LOCKING; assuming 'try'\n",
+                opt);
+        return RRD_FLAGS_LOCKING_MODE_TRY;
+    }
+
+    return flags;
+}
+
+/*
+ * Translates a string to a RRD_FLAGS_LOCKING_xxx constant and updates flags.
+ *
+ * Function will fail on unsupported values and return -1.  It sets rrd_set_error()
+ * in this case.
+ *
+ * Else, the RRD_FLAGS_LOCKING_xxx related bits in 'out_flags' will be cleared
+ * and updated.  Function returns 0 then.
+ */
+int _rrd_lock_from_opt(int *out_flags, const char *opt)
+{
+    int flags = _rrd_lock_parse(opt);
+
+    if (flags < 0) {
+        rrd_set_error("unsupported locking mode '%s'\n", opt);
+        return flags;
+    }
+
+    *out_flags &= ~RRD_FLAGS_LOCKING_MODE_MASK;
+    *out_flags |= flags;
+
+    return 0;
+}
+
+/*
+ * Translates RRD_FLAGS_LOCKING_MODE_xxx to RRD_LOCK_xxx
+ *
+ * Function removes unrelated bits from 'extra_flags' and maps it to the
+ * RRD_LOCK_xxx constants.
+ */
+int _rrd_lock_flags(int extra_flags)
+{
+    /* Due to legacy reasons, we have to map this manually.
+     *
+     * E.g. RRD_LOCK_DEFAULT (which might be used by deprecated direct calls
+     * to rrd_open()) must be non-zero.  But RRD_FLAGS_LOCKING_MODE_DEFAULT
+     * must be 0 because not all users of the updatex api might have been
+     * updated yet.
+     */
+    switch (extra_flags & RRD_FLAGS_LOCKING_MODE_MASK) {
+    case RRD_FLAGS_LOCKING_MODE_NONE:
+        return RRD_LOCK_NONE;
+    case RRD_FLAGS_LOCKING_MODE_TRY:
+        return RRD_LOCK_TRY;
+    case RRD_FLAGS_LOCKING_MODE_BLOCK:
+        return RRD_LOCK_BLOCK;
+    case RRD_FLAGS_LOCKING_MODE_DEFAULT:
+        return RRD_LOCK_DEFAULT;
+    default:
+        abort();
+    }
+}
index 9fb4e4c5b89253215b040a89bde72c3f75c07b0c..3eb1fbeda8921dc7a9a0a953824044fda45e0055 100644 (file)
@@ -133,7 +133,12 @@ typedef int (*rrd_fetch_cb_t)(
 #define RRD_COPY        (1<<4)
 #define RRD_EXCL        (1<<5)
 #define RRD_READVALUES  (1<<6)
-#define RRD_LOCK        (1<<7)
+#define RRD_LOCK        RRD_LOCK_DEFAULT
+#define RRD_LOCK_NONE    (0<<7)
+#define RRD_LOCK_DEFAULT (1<<7)
+#define RRD_LOCK_BLOCK   (2<<7)
+#define RRD_LOCK_TRY     (3<<7)
+#define RRD_LOCK_MASK    (3<<7)
 
     enum cf_en rrd_cf_conv(
     const char *string);
@@ -150,6 +155,10 @@ typedef int (*rrd_fetch_cb_t)(
 
     const char *cf_to_string (enum cf_en cf);
 
+    int _rrd_lock_default(void);
+    int _rrd_lock_from_opt(int *out_flags, const char *opt);
+    int _rrd_lock_flags(int extra_flags);
+
 #ifdef  __cplusplus
 }
 #endif
index 83dd0d9924d443d557b697c032bbabd75e65cdd5..fbbe2820a712a90f1ab81440dcd4738d95cafb82 100644 (file)
@@ -304,12 +304,13 @@ rrd_info_t *rrd_update_v(
     struct optparse_long longopts[] = {
         {"template",          't', OPTPARSE_REQUIRED},
         {"skip-past-updates", 's', OPTPARSE_NONE},
+        {"locking",           'L', OPTPARSE_REQUIRED},
         {0},
     };
     struct optparse options;
     int opt;
     const char *tmplt = NULL;
-    int      extra_flags = 0;
+    int      extra_flags = _rrd_lock_default();
     rrd_info_t *result = NULL;
     rrd_infoval_t rc;
     char *opt_daemon = NULL;
@@ -327,6 +328,11 @@ rrd_info_t *rrd_update_v(
             extra_flags |= RRD_SKIP_PAST_UPDATES;
             break;
 
+        case 'L':
+            if (_rrd_lock_from_opt(&extra_flags, options.optarg) < 0)
+                goto end_tag;
+            break;
+
         case '?':
             rrd_set_error("%s", options.errmsg);
             goto end_tag;
@@ -674,12 +680,13 @@ int rrd_update(
         {"template",          't', OPTPARSE_REQUIRED},
         {"daemon",            'd', OPTPARSE_REQUIRED},
         {"skip-past-updates", 's', OPTPARSE_NONE},
+        {"locking",           'L', OPTPARSE_REQUIRED},
         {0},
     };
     struct optparse options;
     int       opt;
     char     *tmplt = NULL;
-    int       extra_flags = 0;
+    int       extra_flags = _rrd_lock_default();
     int       rc = -1;
     char     *opt_daemon = NULL;
 
@@ -710,6 +717,11 @@ int rrd_update(
             }
             break;
 
+        case 'L':
+            if (_rrd_lock_from_opt(&extra_flags, options.optarg) < 0)
+                goto out;
+            break;
+
         case '?':
             rrd_set_error("%s", options.errmsg);
             goto out;
@@ -739,7 +751,7 @@ int rrd_update(
     {
        rrd_clear_error();
        if (tmplt) {
-               if (extra_flags != 0) {
+               if ((extra_flags & RRD_SKIP_PAST_UPDATES) != 0) {
                    rrd_set_error("The caching daemon cannot be used together with "
                                  "templates and skip-past-updates yet.");
                    goto out;
@@ -859,7 +871,8 @@ static int _rrd_updatex(
     }
 
     rrd_init(&rrd);
-    rrd_file = rrd_open(filename, &rrd, RRD_READWRITE | RRD_LOCK);
+    rrd_file = rrd_open(filename, &rrd, RRD_READWRITE |
+                        _rrd_lock_flags(extra_flags));
     if (rrd_file == NULL) {
         goto err_free;
     }
index 733e34f4410e63b678341abdcecad204ab122f11..cbbf48cc1dffb4dbe094192a5f2303d0c81759fd 100644 (file)
@@ -51,6 +51,7 @@ int main(
        }
        else {
             printf("Usage: rrdupdate <filename>\n"
+                  "\t\t\t[--locking|-L <try|block|none>]\n"
                    "\t\t\t[--template|-t ds-name[:ds-name]...]\n"
                    "\t\t\t[--skip-past-updates]\n"
                    "\t\t\ttime|N:value[:value...]\n\n"