RRDTool patch addressing concerns about r2130

Steve Shipway <[email protected]>
Newsgroups gmane.comp.db.rrdtool.devel
Message-ID <28E447343A85354483BCF7C3E9D5EAA51499AF1E@uxcn10-1.UoA.auckland.ac.nz>
Attached is a diff addressing the issues reported for patch r2130.

Specifically, this deals with:

1.       Remove opt_no_overwrite and the rrd_create_set_no_overwrite function as they are ugly.  Make opt_no_overwrite a local var.

2.       Add 'no_overwrite' param to the rrd_create_r and rename as rrd_create_r2, add rrd_create_r  that calls rrd_create_r2 with a 0 for the no_overwrite function, preserving the API parameter format but allowing an extended arg list that can be used by rrd_create and rrd_handle_request_create

3.       Add -O option to rrdcached command line to force no-overwrite on any creates

4.       Add -O option on create via rrdcached to optionally add no-overwrite (cannot remove it is it was previously set at start)

5.       Fix rrd_handle_request_last to check the queue instead of forcing a flush.  Remove -no-flush option from LAST command since it is now no longer relevant as we're querying the cache chain

6.       Fix ulong/unsigned long issue for compilation under windows

7.       Change rrd_handle_request_create to only accept timestamp parameters, not at-syntax, in order to be threadsafe.  This is no  loss since the rrd_client already pre-parses any at-style syntax

8.       Reinstate missing free() in rrd_info.c and rrd_last.c

This fixes the long-standing issue of the global opt_no_overwrite variable, so a bit of a bonus there.

Steve


________________________________
Steve Shipway
ITS Unix Services Design Lead
University of Auckland, New Zealand
Floor 1, 58 Symonds Street, Auckland
Phone: +64 (0)9 3737599 ext 86487
DDI: +64 (0)9 924 6487
Mobile: +64 (0)21 753 189
Email: [email protected]<mailto:[email protected]>
P Please consider the environment before printing this e-mail

_______________________________________________
rrd-developers mailing list
[email protected]
https://lists.oetiker.ch/cgi-bin/listinfo/rrd-developers
rrd-fixes.diff (application/octet-stream, 5.2 KB)
diff --ignore-all-space --recursive rrdtool-1.4.99910092700/src/librrd.sym.in.in rrdtool-1.4.uoa/src/librrd.sym.in.in
9c9
< rrd_create_set_no_overwrite
---
> rrd_create_r2
diff --ignore-all-space --recursive rrdtool-1.4.99910092700/src/rrd.h rrdtool-1.4.uoa/src/rrd.h
146,147d145
<     void      rrd_create_set_no_overwrite(
<     int);
230a229,236
>     /* int no_overwrite, */
>     int argc,
>     const char **argv);
>     int       rrd_create_r2(
>     const char *filename,
>     unsigned long pdp_step,
>     time_t last_up,
>     int no_overwrite,
diff --ignore-all-space --recursive rrdtool-1.4.99910092700/src/rrd_create.c rrdtool-1.4.uoa/src/rrd_create.c
17d16
< static int opt_no_overwrite = 0;
37,42d35
< void rrd_create_set_no_overwrite( 
<     int opt ) 
< {
< 	opt_no_overwrite = (opt?1:0);
< }
< 
62a56
>     int       opt_no_overwrite = 0;
138,139c132,133
<     rc = rrd_create_r(argv[optind],
<                       pdp_step, last_up,
---
>         rc = rrd_create_r2(argv[optind],
>                       pdp_step, last_up, opt_no_overwrite,
146a141,142
> /* For backwards compatibility with previous API.  Use rrd_create_r2 if you
>    need to have the no_overwrite parameter.                                */
153a150,159
> 	return rrd_create_r2(filename,pdp_step,last_up,0,argc,argv);
> }
> int rrd_create_r2(
>     const char *filename,
>     unsigned long pdp_step,
>     time_t last_up,
>     int no_overwrite,
>     int argc,
>     const char **argv)
> {
587c593
<     return rrd_create_fn(filename, &rrd);
---
>     return rrd_create_fn(filename, &rrd, no_overwrite);
711c717,718
<     rrd_t *rrd)
---
>     rrd_t *rrd,
>     int no_overwrite )
720c727
<     if (opt_no_overwrite) {
---
>     if (no_overwrite) {
diff --ignore-all-space --recursive rrdtool-1.4.99910092700/src/rrd_daemon.c rrdtool-1.4.uoa/src/rrd_daemon.c
1779c1824,1825
< } /* }}} static int handle_request_last  */
---
> } /* }}} static int handle_request_first  */
> 
1785c1831,1834
<   time_t t;
---
>   time_t t, from_file, step;
>   rrd_file_t * rrd_file;
>   cache_item_t * ci;
>   rrd_t rrd; 
1796d1844
<   /* get data */
1798,1799c1846,1848
<   t = rrd_last_r(file);
<   if(t<1) {
---
>   rrd_init(&rrd);
>   rrd_file = rrd_open(file,&rrd,RRD_READONLY);
>   if(!rrd_file) {
1801a1851,1865
>   from_file = rrd.live_head->last_up;
>   step = rrd.stat_head->pdp_step;
>   rrd_close(rrd_file);
>   pthread_mutex_lock(&cache_lock);
>   ci = g_tree_lookup(cache_tree, file);
>   if (ci)
>     t = ci->last_update_stamp;
>   else
>     t = from_file;
>   pthread_mutex_unlock(&cache_lock);
>   t -= t % step;
>   rrd_free(&rrd);
>   if(t<1) {
>     return send_response(sock, RESP_ERR, "Error: rrdcached: Invalid timestamp returned\n");
>   }
1812c1876
<   ulong step = 300;
---
>   unsigned long step = 300;
1814,1815d1877
<   rrd_time_value_t last_up_tv;
<   char     *parsetime_error = NULL;
1830,1831c1892
<   status = buffer_get_field(&buffer, &buffer_size, &tok );
<   for(;(tok && !status);status = buffer_get_field(&buffer, &buffer_size, &tok )) {
---
>   while ((status = buffer_get_field(&buffer, &buffer_size, &tok)) == 0 && tok) {
1835,1842c1896
<       if ((parsetime_error = rrd_parsetime(tok, &last_up_tv))) 
<         return send_response(sock, RESP_ERR, "start time: %s\n", parsetime_error);
<       if (last_up_tv.type == RELATIVE_TO_END_TIME ||
<         last_up_tv.type == RELATIVE_TO_START_TIME) {
<         return send_response(sock, RESP_ERR, "Cannot specify time relative to start or end here.\n");
<       }
<       last_up = mktime(&last_up_tv.tm) +last_up_tv.offset;
< 
---
>       last_up = (time_t) atol(tok);
1866d1919
<   rrd_create_set_no_overwrite(no_overwrite);
1868c1921
<   status = rrd_create_r(file,step,last_up,ac,(const char **)av);
---
>   status = rrd_create_r2(file,step,last_up,no_overwrite,ac,(const char **)av);
2064a2118,2119
>     "The start parameter needs to be in seconds since 1/1/70 (AT-style syntax is\n"
>     "not acceptable) and the step is in seconds (default is 300).\n"
diff --ignore-all-space --recursive rrdtool-1.4.99910092700/src/rrd_info.c rrdtool-1.4.uoa/src/rrd_info.c
154a155
>     if (opt_daemon) free(opt_daemon);
diff --ignore-all-space --recursive rrdtool-1.4.99910092700/src/rrd_last.c rrdtool-1.4.uoa/src/rrd_last.c
17d16
<     int status;
19d17
<     int flushfirst = 1;
29d26
<             {"noflush", no_argument, 0, 'F'},
33c30
<         opt = getopt_long(argc, argv, "d:F", long_options, &option_index);
---
>         opt = getopt_long(argc, argv, "d:", long_options, &option_index);
50,53d46
<         case 'F':
<             flushfirst = 0;
<             break;
< 
55c48
<             rrd_set_error ("Usage: rrdtool %s [--daemon <addr> [--noflush]] <file>",
---
>             rrd_set_error ("Usage: rrdtool %s [--daemon <addr>] <file>",
63c56
<         rrd_set_error ("Usage: rrdtool %s [--daemon <addr> [--noflush]] <file>",
---
>         rrd_set_error ("Usage: rrdtool %s [--daemon <addr>] <file>",
68,72d60
<     if(flushfirst) {
<     status = rrdc_flush_if_daemon(opt_daemon, argv[optind]);
<     if (status) return (-1);
<     }
< 
79a68
>     if (opt_daemon) free(opt_daemon);
diff --ignore-all-space --recursive rrdtool-1.4.99910092700/src/rrd_tool.h rrdtool-1.4.uoa/src/rrd_tool.h
85c85,86
<     rrd_t *rrd);
---
>     rrd_t *rrd,
>     int no_overwrite);
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.