From 009ac7ac9dde1ba7c450b230b9118cff2cedc9f6 Mon Sep 17 00:00:00 2001 From: oetiker Date: Tue, 29 May 2007 21:29:17 +0000 Subject: [PATCH] More updates from Bernhard Fischer - flag rrd_resize's old file with RRD_COPY - cleanup error-handling pathes in rrd_update and fix a few typos in comments - rrd_close(): implement printing mincore results for the rrd if DEBUG=2 was defined - rrd_open(): madvise start addresses need to be page-aligned; implement alternative path to the fine-grained (i.e. exact) madvise by flagging just the first two pages as needed (see TWO_PAGES). Implement alternative path that records the last madvise()ed area to avoid redundant calls to madvise() on identical areas (due to page-alignment constraints) -- see CHECK_MADVISE_OVERLAPS. Implement path for USE_DIRECT_IO. - configure: add check for O_DIRECT flag to open(2). Add option --enable-direct-io. Add _GNU_SOURCE to CFLAGS to silence warnings about chroot which is marked LEGACY since SUSv2 and is a non POSIX extension. Make checks for posix_fadvise() dependant on --disable-mmap, since we do not need fadvise for the mmap case. git-svn-id: svn://svn.oetiker.ch/rrdtool/trunk/program@1088 a5681a0c-68f1-0310-ab6d-d61299d08faa --- configure.ac | 52 ++++++++--- examples/perftest.pl.in | 159 ------------------------------- src/rrd_hw.c | 1 - src/rrd_last.c | 1 + src/rrd_lastupdate.c | 1 + src/rrd_open.c | 196 +++++++++++++++++++++++++++++++------- src/rrd_resize.c | 16 ++-- src/rrd_tool.c | 2 - src/rrd_tune.c | 1 - src/rrd_update.c | 202 ++++++++++++++-------------------------- 10 files changed, 281 insertions(+), 350 deletions(-) diff --git a/configure.ac b/configure.ac index 8fa3c00..dc197e1 100644 --- a/configure.ac +++ b/configure.ac @@ -101,8 +101,7 @@ AH_BOTTOM([ # include #endif -/* enable posix_fadvise on linux */ -#if defined(HAVE_POSIX_FADVISE) && defined(HAVE_FCNTL_H) +#ifdef HAVE_FCNTL_H #include #endif @@ -299,10 +298,12 @@ AC_ARG_ENABLE([mmap], [ --disable-mmap disable mmap in rrd_update, use seek+write instead], [], [enable_mmap=yes]) + +dnl will most likely not work on compressed filesystems, i think.. *shrug* AC_ARG_ENABLE([direct-io], -[ --enable-direct-io enable O_DIRECT], -[], -[enable_direct_io=yes]) +[ --enable-direct-io enable O_DIRECT if available], +[enable_direct_io=yes], +[]) AC_ARG_ENABLE(pthread,[ --disable-pthread disable multithread support], [],[enable_pthread=yes]) @@ -317,6 +318,9 @@ AC_PROG_CC AC_PROG_CPP AC_PROG_LIBTOOL +dnl Try to detect/use GNU features +CFLAGS="$CFLAGS -D_GNU_SOURCE" + dnl which flags does the compiler support? if test "x$GCC" = "xyes"; then for flag in -fno-strict-aliasing -Wall -std=c99 -pedantic -Wundef -Wshadow -Wpointer-arith -Wcast-align -Wmissing-prototypes -Wmissing-declarations -Wnested-externs -Winline -W; do @@ -342,6 +346,26 @@ AC_HEADER_STDC AC_HEADER_DIRENT AC_CHECK_HEADERS(features.h sys/stat.h sys/types.h fcntl.h locale.h fp_class.h malloc.h unistd.h ieeefp.h math.h sys/times.h sys/param.h sys/resource.h signal.h float.h stdio.h stdlib.h errno.h string.h ctype.h) +if test "x$enable_direct_io" = "xyes"; then +AC_CACHE_CHECK([for O_DIRECT flag to open(2)],rrd_cv_HAVE_OPEN_O_DIRECT,[ +AC_TRY_COMPILE([ +#ifdef HAVE_UNISTD_H +#include +#endif +#ifdef HAVE_FCNTL_H +#include +#endif], +[int fd = open("/dev/null", O_DIRECT);], +rrd_cv_HAVE_OPEN_O_DIRECT=yes,rrd_cv_HAVE_OPEN_O_DIRECT=no)]) + if test "x$rrd_cv_HAVE_OPEN_O_DIRECT" = "xyes"; then + AC_DEFINE(USE_DIRECT_IO,1,[Whether the open(2) accepts O_DIRECT]) + else + enable_direct_io="no" + fi +else + enable_direct_io="no" +fi + dnl Checks for typedefs, structures, and compiler characteristics. AC_C_CONST AC_HEADER_TIME @@ -365,12 +389,10 @@ AC_C_BIGENDIAN dnl for each function found we get a definition in config.h dnl of the form HAVE_FUNCTION -AC_CHECK_FUNCS(tzset mbstowcs opendir readdir chdir chroot getuid setlocale strerror strerror_r snprintf vsnprintf fpclass class fp_class isnan memmove strchr mktime getrusage gettimeofday posix_fadvise) +AC_CHECK_FUNCS(tzset mbstowcs opendir readdir chdir chroot getuid setlocale strerror strerror_r snprintf vsnprintf fpclass class fp_class isnan memmove strchr mktime getrusage gettimeofday) dnl Could use these to know if we need to provide a prototype dnl AC_CHECK_DECLS(fdatasync, [], [], [#include ]) -dnl AC_CHECK_DECLS(posix_fadvise, [], [], [#define _XOPEN_SOURCE 600 -dnl #include ]) dnl XXX: dunno about windows.. add AC_CHECK_FUNCS(munmap) there too? if test "x$enable_mmap" = "xyes"; then @@ -403,14 +425,17 @@ if test "x$enable_mmap" = "xyes"; then if test "x$ac_cv_func_mmap" != "xyes"; then AC_MSG_ERROR([--enable-mmap requested but mmap() was not detected]) +dnl enable_mmap="no" fi fi +dnl use FD based I/O ? +if test "x$enable_mmap" = "xno";then + AC_CHECK_DECLS(posix_fadvise, [], [], [#define _XOPEN_SOURCE 600 +#include ]) + AC_CHECK_FUNCS(posix_fadvise) +fi -dnl if test "x$enable_direct_io" = "xyes"; then -dnl check for working O_DIRECT -dnl fi -AC_SUBST([USE_DIRECT_IO]) CONFIGURE_PART(IEEE Math Checks) @@ -826,7 +851,8 @@ echo echo "----------------------------------------------------------------" echo "Config is DONE!" echo -echo " With MMAP IO: $ac_cv_func_mmap_fixed_mapped" +echo " With MMAP IO: $enable_mmap" +echo " Use O_DIRECT: $enable_direct_io" echo " Perl Modules: $COMP_PERL" echo " Perl Binary: $PERL" echo " Perl Version: $PERL_VERSION" diff --git a/examples/perftest.pl.in b/examples/perftest.pl.in index 312ca57..e1e9a04 100755 --- a/examples/perftest.pl.in +++ b/examples/perftest.pl.in @@ -189,163 +189,4 @@ sub main (){ } main; -use strict; -use Time::HiRes qw(time); -use RRDs; -use IO::File; -use Time::HiRes qw( usleep ); - -sub create($$){ - my $file = shift; - my $time = shift; - my $start = time; #since we loaded HiRes - RRDs::create ( $file.".rrd", "-b$time", qw( - -s300 - DS:in:GAUGE:400:U:U - DS:out:GAUGE:400:U:U - RRA:AVERAGE:0.5:1:600 - RRA:AVERAGE:0.5:6:600 - RRA:MAX:0.5:6:600 - RRA:AVERAGE:0.5:24:600 - RRA:MAX:0.5:24:600 - RRA:AVERAGE:0.5:144:600 - RRA:MAX:0.5:144:600 - )); - my $total = time - $start; - my $error = RRDs::error; - die $error if $error; - return $total; -} - -sub update($$){ - my $file = shift; - my $time = shift; - my $in = rand(1000); - my $out = rand(1000); - my $start = time; - my $ret = RRDs::updatev($file.".rrd", $time.":$in:$out"); -# print join("",map {" $_ " . $ret->{$_}."\n" } grep /AVERAGE.\[1\]/, sort keys %$ret)."\n** $time\n\n"; - # sync updates to disk immediately -# usleep(1) if (rand(3) <1 ); - my $total = time - $start; - my $error = RRDs::error; - die $error if $error; - return $total; -} - -sub tune($){ - my $file = shift; - my $start = time; - RRDs::tune ($file.".rrd", "-a","in:U","-a","out:U","-d","in:GAUGE","-d","out:GAUGE"); - my $total = time - $start; - my $error = RRDs::error; - die $error if $error; - return $total; -} - -sub infofetch($){ - my $file = shift; - my $start = time; - my $info = RRDs::info ($file.".rrd"); - my $error = RRDs::error; - die $error if $error; - my $lasttime = $info->{last_update} - $info->{last_update} % $info->{step}; - my $fetch = RRDs::fetch ($file.".rrd",'AVERAGE','-s',$lasttime-1,'-e',$lasttime); - my $total = time - $start; - my $error = RRDs::error; - die $error if $error; - return $total; -} - -sub stddev ($$$){ #http://en.wikipedia.org/wiki/Standard_deviation - my $sum = shift; - my $squaresum = shift; - my $count = shift; - return sqrt( 1 / $count * ( $squaresum - $sum*$sum / $count )) -} - -sub makerrds($$$$){ - my $count = shift; - my $total = shift; - my $list = shift; - my $time = shift; - my @files; - for (1..$count){ - my $id = sprintf ("%07d",$total); - $id =~ s/^(.)(.)(.)(.)(.)//; - push @$list, "$1/$2/$3/$4/$5/$id"; - -d "$1" or mkdir "$1"; - -d "$1/$2" or mkdir "$1/$2"; - -d "$1/$2/$3" or mkdir "$1/$2/$3"; - -d "$1/$2/$3/$4" or mkdir "$1/$2/$3/$4"; - -d "$1/$2/$3/$4/$5" or mkdir "$1/$2/$3/$4/$5"; - push @files, $list->[$total]; - create $list->[$total++],$time-2; - print STDERR "."; - } - for (@files){ - my $fd = new IO::File("$_.rrd","r"); - if (defined $fd) { - $fd->sync; - $fd->close; - } else { - warn "failed to sync $_\n"; - } - } - return $count; -} - - -sub main (){ - mkdir "db-$$" or die $!; - chdir "db-$$"; - - my $step = 200000; # number of rrds to creat for every round - - my @path; - my $time=int(time); - my $tracksize = 0; - my $uppntr = 0; - - - my %squaresum = ( cr => 0, up => 0 ); - my %sum = ( cr => 0, up => 0 ); - my %count =( cr => 0, up => 0 ); - - my $printtime = time; - while (1) { - # enhance the track - $time += 300; - $tracksize += makerrds $step,$tracksize,\@path,$time; - # run benchmark - for (0..10){ - $time += 300; - my $count = 0; - my $sum = 0; - my $squaresum = 0; - for (my $i = 0; $i<$tracksize;$i ++){ - my $elapsed = update($path[$i],$time); - $sum += $elapsed; - $squaresum += $elapsed**2; - $count++; - }; -# for (my $i = 0; $i<$tracksize;$i ++){ -# my $fh = new IO::File "$path[$i].rrd","r"; -# if (defined $fh) { -# $fh->sync; -# $fh->close; -# } else { -# warn "failed to sync $path[$i]\n"; -# } -# } - my $ups = $count/$sum; - my $sdv = stddev($sum,$squaresum,$count); - printf STDERR "%4d %6.0f Up/s (%6.5f sdv)\n",$count,$ups,$sdv; - } - print STDERR "\n"; - exit ; - } -} - -main; diff --git a/src/rrd_hw.c b/src/rrd_hw.c index e849416..d9247c9 100644 --- a/src/rrd_hw.c +++ b/src/rrd_hw.c @@ -818,7 +818,6 @@ void reset_aberrant_coefficients( != (ssize_t) (sizeof(cdp_prep_t) * (rrd->stat_head->rra_cnt) * (rrd->stat_head->ds_cnt))) { rrd_set_error("reset_aberrant_coefficients: cdp_prep write failed"); - return; /*XXX: delme */ } } diff --git a/src/rrd_last.c b/src/rrd_last.c index 99ff125..42a8075 100644 --- a/src/rrd_last.c +++ b/src/rrd_last.c @@ -35,6 +35,7 @@ time_t rrd_last_r( lastup = rrd.live_head->last_up; rrd_free(&rrd); + close(rrd_file->fd); rrd_close(rrd_file); return (lastup); } diff --git a/src/rrd_lastupdate.c b/src/rrd_lastupdate.c index 47ecec7..79274b5 100644 --- a/src/rrd_lastupdate.c +++ b/src/rrd_lastupdate.c @@ -54,6 +54,7 @@ int rrd_lastupdate( } rrd_free(&rrd); + close(rrd_file->fd); rrd_close(rrd_file); return (0); } diff --git a/src/rrd_open.c b/src/rrd_open.c index baeb003..c986f75 100644 --- a/src/rrd_open.c +++ b/src/rrd_open.c @@ -66,6 +66,15 @@ #include "unused.h" #define MEMBLK 8192 +/* DEBUG 2 prints information obtained via mincore(2) */ +// #define DEBUG 2 +/* do not calculate exact madvise hints but assume 2 pages for headers and + * set DONTNEED for the rest, which is assumed to be data */ +//#define TWO_PAGES 1 +/* Avoid calling madvise on areas that were already hinted. May be benefical if + * your syscalls are very slow */ +//#define CHECK_MADVISE_OVERLAPS 1 + #ifdef HAVE_MMAP #define __rrd_read(dst, dst_t, cnt) \ (dst) = (dst_t*) (data + offset); \ @@ -79,6 +88,35 @@ offset += read (rrd_file->fd, dst, sizeof(dst_t)*(cnt)) #endif +/* next page-aligned (i.e. page-align up) */ +#ifndef PAGE_ALIGN +#define PAGE_ALIGN(addr) (((addr)+_page_size-1)&(~(_page_size-1))) +#endif +/* previous page-aligned (i.e. page-align down) */ +#ifndef PAGE_ALIGN_DOWN +#define PAGE_ALIGN_DOWN(addr) (((addr)+_page_size-1)&(~(_page_size-1))) +#endif + +#ifdef HAVE_MMAP +/* vector of last madvise hint */ +typedef struct _madvise_vec_t { + void *start; + ssize_t length; +} _madvise_vec_t; +_madvise_vec_t _madv_vec = { NULL, 0 }; +#endif + +#if defined CHECK_MADVISE_OVERLAPS +#define _madvise(_start, _off, _hint) \ + if ((_start) != _madv_vec.start && (ssize_t)(_off) != _madv_vec.length) { \ + _madv_vec.start = (_start) ; _madv_vec.length = (_off); \ + madvise((_start), (_off), (_hint)); \ + } +#else +#define _madvise(_start, _off, _hint) \ + madvise((_start), (_off), (_hint)) +#endif + /* open a database file, return its header and an open filehandle */ /* positioned to the first cdp in the first rra */ @@ -92,6 +130,7 @@ rrd_file_t *rrd_open( int version; #ifdef HAVE_MMAP + ssize_t _page_size = sysconf(_SC_PAGESIZE); int mm_prot = PROT_READ, mm_flags = 0; char *data; #endif @@ -118,9 +157,8 @@ rrd_file_t *rrd_open( #ifdef HAVE_MMAP mm_flags = MAP_PRIVATE; # ifdef MAP_NORESERVE - mm_flags |= MAP_NORESERVE; + mm_flags |= MAP_NORESERVE; /* readonly, so no swap backing needed */ # endif - mm_flags |= MAP_PRIVATE; #endif } else { if (rdwr & RRD_READWRITE) { @@ -137,19 +175,19 @@ rrd_file_t *rrd_open( } if (rdwr & RRD_READAHEAD) { #ifdef MAP_POPULATE - mm_flags |= MAP_POPULATE; + mm_flags |= MAP_POPULATE; /* populate ptes and data */ #endif -#if defined MAP_NONBLOCK && !defined USE_DIRECT_IO - mm_flags |= MAP_NONBLOCK; /* just populage ptes */ +#if defined MAP_NONBLOCK + mm_flags |= MAP_NONBLOCK; /* just populate ptes */ #endif - } else { #ifdef USE_DIRECT_IO + } else { flags |= O_DIRECT; -#endif -#if 0 //def O_NONBLOCK - flags |= O_NONBLOCK; #endif } +#ifdef O_NONBLOCK + flags |= O_NONBLOCK; +#endif if ((rrd_file->fd = open(file_name, flags, mode)) < 0) { rrd_set_error("opening '%s': %s", file_name, rrd_strerror(errno)); @@ -196,21 +234,32 @@ rrd_file_t *rrd_open( goto out_close; } rrd_file->file_start = data; -#else #endif #ifdef USE_MADVISE - if (rdwr & RRD_COPY) { /*XXX: currently not used! */ + if (rdwr & RRD_COPY) { /* We will read everything in a moment (copying) */ - madvise(data, rrd_file->file_len, MADV_WILLNEED | MADV_SEQUENTIAL); + _madvise(data, rrd_file->file_len, MADV_WILLNEED | MADV_SEQUENTIAL); goto out_done; } /* We do not need to read anything in for the moment */ - madvise(data, rrd_file->file_len, MADV_DONTNEED); +#ifndef TWO_PAGES + _madvise(data, rrd_file->file_len, MADV_DONTNEED); +// _madvise(data, rrd_file->file_len, MADV_RANDOM); +#else +/* alternatively: keep 2 pages worth of data, likely headers, + * don't need the rest. */ + _madvise(data, _page_size * 2, MADV_WILLNEED | MADV_SEQUENTIAL); + _madvise(data + _page_size * 2, (rrd_file->file_len >= _page_size * 2) + ? rrd_file->file_len - _page_size * 2 : 0, MADV_DONTNEED); +#endif #endif -#ifdef USE_MADVISE +#if defined USE_MADVISE && !defined TWO_PAGES /* the stat_head will be needed soonish, so hint accordingly */ - madvise(data + offset, sizeof(stat_head_t), MADV_WILLNEED); +// too finegrained to calc the individual sizes, just keep 2 pages worth of hdr + _madvise(data + PAGE_ALIGN_DOWN(offset), PAGE_ALIGN(sizeof(stat_head_t)), + MADV_WILLNEED); + #endif __rrd_read(rrd->stat_head, stat_head_t, @@ -234,18 +283,20 @@ rrd_file_t *rrd_open( rrd->stat_head->version); goto out_nullify_head; } -#ifdef USE_MADVISE +#if defined USE_MADVISE && !defined TWO_PAGES /* the ds_def will be needed soonish, so hint accordingly */ - madvise(data + offset, sizeof(ds_def_t) * rrd->stat_head->ds_cnt, - MADV_WILLNEED); + _madvise(data + PAGE_ALIGN_DOWN(offset), + PAGE_ALIGN(sizeof(ds_def_t) * rrd->stat_head->ds_cnt), + MADV_WILLNEED); #endif __rrd_read(rrd->ds_def, ds_def_t, rrd->stat_head->ds_cnt); -#ifdef USE_MADVISE +#if defined USE_MADVISE && !defined TWO_PAGES /* the rra_def will be needed soonish, so hint accordingly */ - madvise(data + offset, sizeof(rra_def_t) * rrd->stat_head->rra_cnt, - MADV_WILLNEED); + _madvise(data + PAGE_ALIGN_DOWN(offset), + PAGE_ALIGN(sizeof(rra_def_t) * rrd->stat_head->rra_cnt), + MADV_WILLNEED); #endif __rrd_read(rrd->rra_def, rra_def_t, rrd->stat_head->rra_cnt); @@ -265,9 +316,10 @@ rrd_file_t *rrd_open( #endif rrd->live_head->last_up_usec = 0; } else { -#ifdef USE_MADVISE +#if defined USE_MADVISE && !defined TWO_PAGES /* the live_head will be needed soonish, so hint accordingly */ - madvise(data + offset, sizeof(live_head_t), MADV_WILLNEED); + _madvise(data + PAGE_ALIGN_DOWN(offset), + PAGE_ALIGN(sizeof(live_head_t)), MADV_WILLNEED); #endif __rrd_read(rrd->live_head, live_head_t, 1); @@ -289,7 +341,7 @@ rrd_file_t *rrd_open( #endif rrd_file->header_len = offset; rrd_file->pos = offset; -/* we could close(rrd_file->fd); here, the mapping is still valid anyway */ + return (rrd_file); out_nullify_head: rrd->stat_head = NULL; @@ -298,26 +350,86 @@ rrd_file_t *rrd_open( return NULL; } + /* Close a reference to an rrd_file. */ + int rrd_close( rrd_file_t *rrd_file) { int ret; +#if defined HAVE_MMAP + ssize_t _page_size = sysconf(_SC_PAGESIZE); +#endif +#if defined DEBUG && DEBUG > 1 + /* pretty print blocks in core */ + off_t off; + unsigned char *vec; + + off = + rrd_file->file_len + + ((rrd_file->file_len + sysconf(_SC_PAGESIZE) - + 1) / sysconf(_SC_PAGESIZE)); + vec = malloc(off); + if (vec != NULL) { + memset(vec, 0, off); + if (mincore(rrd_file->file_start, rrd_file->file_len, vec) == 0) { + int prev; + unsigned is_in = 0, was_in = 0; + + for (off = 0, prev = 0; off < rrd_file->file_len; ++off) { + is_in = vec[off] & 1; /* if lsb set then is core resident */ + if (off == 0) + was_in = is_in; + if (was_in != is_in) { + fprintf(stderr, "%sin core: %p len %ld\n", + was_in ? "" : "not ", vec + prev, off - prev); + was_in = is_in; + prev = off; + } + } + fprintf(stderr, + "%sin core: %p len %ld\n", + was_in ? "" : "not ", vec + prev, off - prev); + } else + fprintf(stderr, "mincore: %s", rrd_strerror(errno)); + } +#endif /* DEBUG */ + +#ifdef USE_MADVISE +#ifdef TWO_PAGES +//XXX: ? + /* Keep 2 pages worth of headers around, round up to next page boundary. */ + ret = + PAGE_ALIGN(rrd_file->header_len % _page_size + rrd_file->header_len); + if (rrd_file->file_len > ret) + _madvise(rrd_file->file_start + ret, + rrd_file->file_len - ret, MADV_DONTNEED); +#else + /* ignoring errors from RRDs that are smaller then the file_len+rounding */ + _madvise(rrd_file->file_start + PAGE_ALIGN_DOWN(rrd_file->header_len), + rrd_file->file_len - PAGE_ALIGN(rrd_file->header_len), + MADV_DONTNEED); +#endif +#endif #ifdef HAVE_MMAP ret = munmap(rrd_file->file_start, rrd_file->file_len); if (ret != 0) rrd_set_error("munmap rrd_file: %s", rrd_strerror(errno)); +#else + ret = 0; #endif - ret = close(rrd_file->fd); - if (ret != 0) - rrd_set_error("closing file: %s", rrd_strerror(errno)); +// ret = close(rrd_file->fd); +// if (ret != 0) +// rrd_set_error("closing file: %s", rrd_strerror(errno)); free(rrd_file); rrd_file = NULL; return ret; } + /* Set position of rrd_file. */ + off_t rrd_seek( rrd_file_t *rrd_file, off_t off, @@ -342,24 +454,25 @@ off_t rrd_seek( return ret == -1; //XXX: or just ret to mimic lseek } + /* Get current position in rrd_file. */ -inline off_t rrd_tell( - rrd_file_t *rrd_file) + +inline off_t rrd_tell(rrd_file_t *rrd_file) { return rrd_file->pos; } + /* read count bytes into buffer buf, starting at rrd_file->pos. * Returns the number of bytes read. */ + ssize_t rrd_read( rrd_file_t *rrd_file, void *buf, size_t count) { #ifdef HAVE_MMAP - char *pos = rrd_file->file_start + rrd_file->pos; - - buf = memmove(buf, pos, count); + buf = memmove(buf, rrd_file->file_start + rrd_file->pos, count); rrd_file->pos += count; /* mimmic read() semantics */ return count; #else @@ -372,9 +485,11 @@ ssize_t rrd_read( #endif } + /* write count bytes from buffer buf to the current position * rrd_file->pos of rrd_file->fd. * Returns the number of bytes written. */ + ssize_t rrd_write( rrd_file_t *rrd_file, const void *buf, @@ -388,7 +503,9 @@ ssize_t rrd_write( #endif } + /* flush all data pending to be written to FD. */ + inline void rrd_flush( rrd_file_t *rrd_file) { @@ -398,6 +515,9 @@ inline void rrd_flush( } } + +/* Initialize RRD header. */ + void rrd_init( rrd_t *rrd) { @@ -411,6 +531,9 @@ void rrd_init( rrd->rrd_value = NULL; } + +/* free RRD header data. */ + void rrd_free( rrd_t UNUSED(*rrd)) { @@ -424,19 +547,24 @@ void rrd_free( free(rrd->pdp_prep); free(rrd->cdp_prep); free(rrd->rrd_value); -//XXX: ? rrd_init(rrd); #endif } + /* routine used by external libraries to free memory allocated by * rrd library */ + void rrd_freemem( void *mem) { free(mem); } -int readfile( + +/* XXX: FIXME: missing documentation. */ +/*XXX: FIXME should be renamed to rrd_readfile or _rrd_readfile */ + +int /*_rrd_*/ readfile( const char *file_name, char **buffer, int skipfirst) diff --git a/src/rrd_resize.c b/src/rrd_resize.c index f64615c..11efcda 100644 --- a/src/rrd_resize.c +++ b/src/rrd_resize.c @@ -55,7 +55,7 @@ int rrd_resize( modify = -modify; - rrd_file = rrd_open(infilename, &rrdold, RRD_READWRITE); + rrd_file = rrd_open(infilename, &rrdold, RRD_READWRITE | RRD_COPY); if (rrd_file == NULL) { rrd_set_error("could not open RRD"); return (-1); @@ -111,15 +111,14 @@ int rrd_resize( case 1: rrdold.stat_head->version[3] = '3'; break; - default:{ + default: rrd_set_error("Do not know how to handle RRD version %s", rrdold.stat_head->version); + rrd_close(rrd_file); rrd_free(&rrdold); - close(rrd_file->fd); return (-1); + break; } - } - /* XXX: Error checking? */ rrd_write(rrd_out_file, rrdnew.stat_head, sizeof(stat_head_t) * 1); @@ -231,8 +230,13 @@ int rrd_resize( rrd_write(rrd_out_file, rrdnew.rra_ptr, sizeof(rra_ptr_t) * rrdnew.stat_head->rra_cnt); - close(rrd_out_file->fd); rrd_free(&rrdold); close(rrd_file->fd); + rrd_close(rrd_file); + + rrd_free(&rrdnew); + close(rrd_out_file->fd); + rrd_close(rrd_out_file); + return (0); } diff --git a/src/rrd_tool.c b/src/rrd_tool.c index ba561ce..ee561d6 100644 --- a/src/rrd_tool.c +++ b/src/rrd_tool.c @@ -432,8 +432,6 @@ int main( free(myargv); if (ret == 0) { - - #if HAVE_GETRUSAGE getrusage(RUSAGE_SELF, &myusage); gettimeofday(¤ttime, NULL); diff --git a/src/rrd_tune.c b/src/rrd_tune.c index df22968..680e558 100644 --- a/src/rrd_tune.c +++ b/src/rrd_tune.c @@ -289,7 +289,6 @@ int rrd_tune( } } if (optcnt > 0) { - rrd_seek(rrd_file, 0, SEEK_SET); rrd_write(rrd_file, rrd.stat_head, sizeof(stat_head_t) * 1); rrd_write(rrd_file, rrd.ds_def, diff --git a/src/rrd_update.c b/src/rrd_update.c index eb08f78..6ef5d3f 100644 --- a/src/rrd_update.c +++ b/src/rrd_update.c @@ -56,10 +56,10 @@ static int gettimeofday( #endif /* - * normilize time as returned by gettimeofday. usec part must + * normalize time as returned by gettimeofday. usec part must * be always >= 0 */ -static void normalize_time( +static inline void normalize_time( struct timeval *t) { if (t->tv_usec < 0) { @@ -271,7 +271,7 @@ int _rrd_update( rrd_value_t *pdp_new; /* prepare the incoming data * to be added the the * existing entry */ - rrd_value_t *pdp_temp; /* prepare the pdp values + rrd_value_t *pdp_temp; /* prepare the pdp values * to be added the the * cdp values */ @@ -318,12 +318,12 @@ int _rrd_update( /* need at least 1 arguments: data. */ if (argc < 1) { rrd_set_error("Not enough arguments"); - return -1; + goto err_out; } rrd_file = rrd_open(filename, &rrd, RRD_READWRITE); if (rrd_file == NULL) { - return -1; + goto err_out; } /* initialize time */ @@ -362,36 +362,25 @@ int _rrd_update( */ if (LockRRD(rrd_file->fd) != 0) { rrd_set_error("could not lock RRD"); - rrd_free(&rrd); - close(rrd_file->fd); - return (-1); + goto err_close; } if ((updvals = malloc(sizeof(char *) * (rrd.stat_head->ds_cnt + 1))) == NULL) { rrd_set_error("allocating updvals pointer array"); - rrd_free(&rrd); - close(rrd_file->fd); - return (-1); + goto err_close; } if ((pdp_temp = malloc(sizeof(rrd_value_t) * rrd.stat_head->ds_cnt)) == NULL) { rrd_set_error("allocating pdp_temp ..."); - free(updvals); - rrd_free(&rrd); - close(rrd_file->fd); - return (-1); + goto err_free_updvals; } if ((tmpl_idx = malloc(sizeof(unsigned long) * (rrd.stat_head->ds_cnt + 1))) == NULL) { rrd_set_error("allocating tmpl_idx ..."); - free(pdp_temp); - free(updvals); - rrd_free(&rrd); - close(rrd_file->fd); - return (-1); + goto err_free_pdp_temp; } /* initialize tmplt redirector */ /* default config example (assume DS 1 is a CDEF DS) @@ -421,22 +410,11 @@ int _rrd_update( if (tmpl_cnt > rrd.stat_head->ds_cnt) { rrd_set_error ("tmplt contains more DS definitions than RRD"); - free(updvals); - free(pdp_temp); - free(tmpl_idx); - rrd_free(&rrd); - close(rrd_file->fd); - return (-1); + goto err_free_tmpl_idx; } if ((tmpl_idx[tmpl_cnt++] = ds_match(&rrd, dsname)) == -1) { rrd_set_error("unknown DS name '%s'", dsname); - free(updvals); - free(pdp_temp); - free(tmplt_copy); - free(tmpl_idx); - rrd_free(&rrd); - close(rrd_file->fd); - return (-1); + goto err_free_tmpl_idx; } else { /* the first element is always the time */ tmpl_idx[tmpl_cnt - 1]++; @@ -455,12 +433,7 @@ int _rrd_update( if ((pdp_new = malloc(sizeof(rrd_value_t) * rrd.stat_head->ds_cnt)) == NULL) { rrd_set_error("allocating pdp_new ..."); - free(updvals); - free(pdp_temp); - free(tmpl_idx); - rrd_free(&rrd); - close(rrd_file->fd); - return (-1); + goto err_free_tmpl_idx; } #if 0 //def HAVE_MMAP rrd_mmaped_file = mmap(0, @@ -494,20 +467,11 @@ int _rrd_update( if (stepper == NULL) { rrd_set_error("failed duplication argv entry"); free(step_start); - free(updvals); - free(pdp_temp); - free(tmpl_idx); - rrd_free(&rrd); -#ifdef HAVE_MMAP - rrd_close(rrd_file); -#endif - close(rrd_file->fd); - return (-1); + goto err_free_pdp_new; } /* initialize all ds input to unknown except the first one which has always got to be set */ - for (ii = 1; ii <= rrd.stat_head->ds_cnt; ii++) - updvals[ii] = "U"; + memset(updvals + 1, 'U', rrd.stat_head->ds_cnt); updvals[0] = stepper; /* separate all ds elements; first must be examined separately due to alternate time syntax */ @@ -660,7 +624,7 @@ int _rrd_update( /* NOTE: DST_CDEF should never enter this if block, because * updvals[i+1][0] is initialized to 'U'; unless the caller - * accidently specified a value for the DST_CDEF. To handle + * accidently specified a value for the DST_CDEF. To handle * this case, an extra check is required. */ if ((updvals[i + 1][0] != 'U') && @@ -672,8 +636,6 @@ int _rrd_update( /* pdp_new contains rate * time ... eg the bytes * transferred during the interval. Doing it this way saves * a lot of math operations */ - - switch (dst_idx) { case DST_COUNTER: case DST_DERIVE: @@ -715,7 +677,7 @@ int _rrd_update( errno = 0; pdp_new[i] = strtod(updvals[i + 1], &endptr); if (errno > 0) { - rrd_set_error("converting '%s' to float: %s", + rrd_set_error("converting '%s' to float: %s", updvals[i + 1], rrd_strerror(errno)); break; }; @@ -731,7 +693,7 @@ int _rrd_update( errno = 0; pdp_new[i] = strtod(updvals[i + 1], &endptr) * interval; if (errno > 0) { - rrd_set_error("converting '%s' to float: %s", + rrd_set_error("converting '%s' to float: %s", updvals[i + 1], rrd_strerror(errno)); break; }; @@ -817,21 +779,21 @@ int _rrd_update( } else { /* an pdp_st has occurred. */ - /* in pdp_prep[].scratch[PDP_val].u_val we have collected rate*seconds which - * occurred up to the last run. - pdp_new[] contains rate*seconds from the latest run. - pdp_temp[] will contain the rate for cdp */ + /* in pdp_prep[].scratch[PDP_val].u_val we have collected + rate*seconds which occurred up to the last run. + pdp_new[] contains rate*seconds from the latest run. + pdp_temp[] will contain the rate for cdp */ for (i = 0; i < rrd.stat_head->ds_cnt; i++) { /* update pdp_prep to the current pdp_st. */ double pre_unknown = 0.0; - if (isnan(pdp_new[i])) + if (isnan(pdp_new[i])) { /* a final bit of unkonwn to be added bevore calculation - * we use a tempaorary variable for this so that we - * don't have to turn integer lines before using the value */ + we use a temporary variable for this so that we + don't have to turn integer lines before using the value */ pre_unknown = pre_int; - else { + } else { if (isnan(rrd.pdp_prep[i].scratch[PDP_val].u_val)) { rrd.pdp_prep[i].scratch[PDP_val].u_val = pdp_new[i] / interval * pre_int; @@ -844,9 +806,9 @@ int _rrd_update( /* if too much of the pdp_prep is unknown we dump it */ if ( - /* removed because this does not agree with the definition - a heart beat can be unknown */ - /* (rrd.pdp_prep[i].scratch[PDP_unkn_sec_cnt].u_cnt + /* removed because this does not agree with the + definition that a heartbeat can be unknown */ + /* (rrd.pdp_prep[i].scratch[PDP_unkn_sec_cnt].u_cnt > rrd.ds_def[i].par[DS_mrhb_cnt].u_cnt) || */ /* if the interval is larger thatn mrhb we get NAN */ (interval > rrd.ds_def[i].par[DS_mrhb_cnt].u_cnt) || @@ -944,16 +906,17 @@ int _rrd_update( } if (current_cf == CF_SEASONAL || current_cf == CF_DEVSEASONAL) { - /* If this is a bulk update, we need to skip ahead in the seasonal - * arrays so that they will be correct for the next observed value; - * note that for the bulk update itself, no update will occur to - * DEVSEASONAL or SEASONAL; futhermore, HWPREDICT and DEVPREDICT will - * be set to DNAN. */ + /* If this is a bulk update, we need to skip ahead in + the seasonal arrays so that they will be correct for + the next observed value; + note that for the bulk update itself, no update will + occur to DEVSEASONAL or SEASONAL; futhermore, HWPREDICT + and DEVPREDICT will be set to DNAN. */ if (rra_step_cnt[i] > 2) { /* skip update by resetting rra_step_cnt[i], - * note that this is not data source specific; this is due - * to the bulk update, not a DNAN value for the specific data - * source. */ + note that this is not data source specific; this is + due to the bulk update, not a DNAN value for the + specific data source. */ rra_step_cnt[i] = 0; lookup_seasonal(&rrd, i, rra_start, rrd_file, elapsed_pdp_st, &last_seasonal_coef); @@ -1463,18 +1426,12 @@ int _rrd_update( rrd_set_error("error writing(unmapping) file: %s", filename); } #else - rrd_flush(rrd_file); //XXX: really needed? + //rrd_flush(rrd_file); //XXX: really needed? #endif /* if we got here and if there is an error and if the file has not been * written to, then close things up and return. */ if (rrd_test_error()) { - free(updvals); - free(tmpl_idx); - rrd_free(&rrd); - free(pdp_temp); - free(pdp_new); - close(rrd_file->fd); - return (-1); + goto err_free_pdp_new; } /* aargh ... that was tough ... so many loops ... anyway, its done. @@ -1485,38 +1442,20 @@ int _rrd_update( + sizeof(rra_def_t) * rrd.stat_head->rra_cnt), SEEK_SET) != 0) { rrd_set_error("seek rrd for live header writeback"); - free(updvals); - free(tmpl_idx); - rrd_free(&rrd); - free(pdp_temp); - free(pdp_new); - close(rrd_file->fd); - return (-1); + goto err_free_pdp_new; } if (version >= 3) { if (rrd_write(rrd_file, rrd.live_head, sizeof(live_head_t) * 1) != sizeof(live_head_t) * 1) { rrd_set_error("rrd_write live_head to rrd"); - free(updvals); - rrd_free(&rrd); - free(tmpl_idx); - free(pdp_temp); - free(pdp_new); - close(rrd_file->fd); - return (-1); + goto err_free_pdp_new; } } else { if (rrd_write(rrd_file, &rrd.live_head->last_up, sizeof(time_t) * 1) != sizeof(time_t) * 1) { rrd_set_error("rrd_write live_head to rrd"); - free(updvals); - rrd_free(&rrd); - free(tmpl_idx); - free(pdp_temp); - free(pdp_new); - close(rrd_file->fd); - return (-1); + goto err_free_pdp_new; } } @@ -1525,13 +1464,7 @@ int _rrd_update( sizeof(pdp_prep_t) * rrd.stat_head->ds_cnt) != (ssize_t) (sizeof(pdp_prep_t) * rrd.stat_head->ds_cnt)) { rrd_set_error("rrd_write pdp_prep to rrd"); - free(updvals); - rrd_free(&rrd); - free(tmpl_idx); - free(pdp_temp); - free(pdp_new); - close(rrd_file->fd); - return (-1); + goto err_free_pdp_new; } if (rrd_write(rrd_file, rrd.cdp_prep, @@ -1541,26 +1474,14 @@ int _rrd_update( rrd.stat_head->ds_cnt)) { rrd_set_error("rrd_write cdp_prep to rrd"); - free(updvals); - free(tmpl_idx); - rrd_free(&rrd); - free(pdp_temp); - free(pdp_new); - close(rrd_file->fd); - return (-1); + goto err_free_pdp_new; } if (rrd_write(rrd_file, rrd.rra_ptr, sizeof(rra_ptr_t) * rrd.stat_head->rra_cnt) != (ssize_t) (sizeof(rra_ptr_t) * rrd.stat_head->rra_cnt)) { rrd_set_error("rrd_write rra_ptr to rrd"); - free(updvals); - free(tmpl_idx); - rrd_free(&rrd); - free(pdp_temp); - free(pdp_new); - close(rrd_file->fd); - return (-1); + goto err_free_pdp_new; } #ifdef HAVE_POSIX_FADVISExxx @@ -1572,11 +1493,10 @@ int _rrd_update( if (0 != posix_fadvise(rrd_file->fd, rra_begin, 0, POSIX_FADV_DONTNEED)) { rrd_set_error("setting POSIX_FADV_DONTNEED on '%s': %s", filename, rrd_strerror(errno)); - close(rrd_file->fd); - return (-1); + goto err_free_pdp_new; } #endif - /*XXX: ? */ rrd_flush(rrd_file); + /* rrd_flush(rrd_file); */ /* calling the smoothing code here guarantees at most * one smoothing operation per rrd_update call. Unfortunately, @@ -1584,8 +1504,6 @@ int _rrd_update( * for smoothing to occur off-schedule. This really isn't * critical except during the burning cycles. */ if (schedule_smooth) { -// in_file = fopen(filename,"rb+"); - rra_start = rra_begin; for (i = 0; i < rrd.stat_head->rra_cnt; ++i) { @@ -1607,19 +1525,35 @@ int _rrd_update( posix_fadvise(rrd_file->fd, rra_begin, 0, POSIX_FADV_DONTNEED)) { rrd_set_error("setting POSIX_FADV_DONTNEED on '%s': %s", filename, rrd_strerror(errno)); - close(rrd_file->fd); - return (-1); + goto err_free_pdp_new; } #endif } - rrd_close(rrd_file); rrd_free(&rrd); - free(updvals); - free(tmpl_idx); + close(rrd_file->fd); + rrd_close(rrd_file); + free(pdp_new); + free(tmpl_idx); free(pdp_temp); + free(updvals); return (0); + + err_free_pdp_new: + free(pdp_new); + err_free_tmpl_idx: + free(tmpl_idx); + err_free_pdp_temp: + free(pdp_temp); + err_free_updvals: + free(updvals); + err_close: + rrd_free(&rrd); + close(rrd_file->fd); + rrd_close(rrd_file); + err_out: + return (-1); } /* -- 2.30.2