From: Enrico Scholz Date: Thu, 2 Mar 2023 16:00:25 +0000 (+0100) Subject: Allow to select locking mechanism (#1207) X-Git-Tag: v1.9.0~17 X-Git-Url: http://git.ipfire.org/gitweb/gitweb.cgi?a=commitdiff_plain;h=f71039b4114de3784969de01612f9f0c5a68089f;p=thirdparty%2Frrdtool-1.x.git Allow to select locking mechanism (#1207) * 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 --- diff --git a/CHANGES b/CHANGES index 5cfc048f..f5dbe3ca 100644 --- 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) +* 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 ========================== diff --git a/doc/rrdtool.pod b/doc/rrdtool.pod index c1baf688..df717cef 100644 --- a/doc/rrdtool.pod +++ b/doc/rrdtool.pod @@ -314,6 +314,40 @@ sockets, tools like netcat, or in a quick interactive test by using B 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 + +If this environment variable is set, the B file is locked in the +given mode: + +=over + +=item - + +B fails, when the file is locked by another process. This is the default. + +=item - + +B waits until the lock is released. + +=item - + +B skips locking at all. Caller has to ensure a proper locking +which should be compatible with C. + +=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 diff --git a/doc/rrdupdate.pod b/doc/rrdupdate.pod index 8bc0c2e7..20a193dd 100644 --- a/doc/rrdupdate.pod +++ b/doc/rrdupdate.pod @@ -38,6 +38,14 @@ used with this command. The name of the B you want to update. +=item B<--locking>|B<-L> B|B|B + +Lock the B file in the given mode: B fails, when the file +is locked by another process, B waits until the lock is +released and B skips locking at all. The default is read +from the B<$RRD_LOCKING> environment variable and falls back to +B when such a variable does not exist. + =item B<--template>|B<-t> I[B<:>I]... By default, the B 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 + +If this environment variable is set, the B file is locked in the +given mode: B fails, when the file is locked by another process, +B waits until the lock is released and B skips locking at +all. + +This variable can be overridden by the B<--locking> command line option. + =back =head1 EXAMPLES diff --git a/src/rrd.h b/src/rrd.h index 30ac89c6..184887cc 100644 --- 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, diff --git a/src/rrd_open.c b/src/rrd_open.c index c326568a..c21684d2 100644 --- a/src/rrd_open.c +++ b/src/rrd_open.c @@ -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(); + } +} diff --git a/src/rrd_tool.h b/src/rrd_tool.h index 9fb4e4c5..3eb1fbed 100644 --- a/src/rrd_tool.h +++ b/src/rrd_tool.h @@ -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 diff --git a/src/rrd_update.c b/src/rrd_update.c index 83dd0d99..fbbe2820 100644 --- a/src/rrd_update.c +++ b/src/rrd_update.c @@ -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; } diff --git a/src/rrdupdate.c b/src/rrdupdate.c index 733e34f4..cbbf48cc 100644 --- a/src/rrdupdate.c +++ b/src/rrdupdate.c @@ -51,6 +51,7 @@ int main( } else { printf("Usage: rrdupdate \n" + "\t\t\t[--locking|-L ]\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"