From: Peter Stamfest Date: Thu, 6 Mar 2014 21:54:13 +0000 (+0100) Subject: Further chop up rrd_modify_r in order to integrate step changes into DS/RRA X-Git-Tag: v1.5.0-rc1~120^2~5 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=73099d8dd45aac00edaa6dcccc6917c1783b26e9;p=thirdparty%2Frrdtool-1.x.git Further chop up rrd_modify_r in order to integrate step changes into DS/RRA modifications. --- diff --git a/src/rrd_modify.c b/src/rrd_modify.c index 010d1b95..41806673 100644 --- a/src/rrd_modify.c +++ b/src/rrd_modify.c @@ -772,10 +772,10 @@ static int stretch_rras(rrd_t *out, int stretch) { goto done; } - int rra_index; + unsigned int rra_index; for (rra_index = 0 ; rra_index < out->stat_head->rra_cnt ; rra_index++) { rra_def_t *rra = out->rra_def + rra_index; - cdp_prep_t *cdp_prep = out->cdp_prep + rra_index; +// cdp_prep_t *cdp_prep = out->cdp_prep + rra_index; rra->pdp_cnt *= stretch; } @@ -787,128 +787,96 @@ done: return rc; } -/* copies the RRD named by infilename to a new RRD called outfilename. - - In that process, data sources may be removed or added. - - removeDS points to an array of strings, each naming a DS to be - removed. The list itself is NULL terminated. addDS points to a - similar list holding rrdcreate-style data source definitions to be - added. -*/ +static void rrd_memory_free(rrd_t *rrd) +{ + if (rrd == NULL) return; + if (rrd->live_head) free(rrd->live_head); + if (rrd->stat_head) free(rrd->stat_head); + if (rrd->ds_def) free(rrd->ds_def); + if (rrd->rra_def) free(rrd->rra_def); + if (rrd->rra_ptr) free(rrd->rra_ptr); + if (rrd->pdp_prep) free(rrd->pdp_prep); + if (rrd->cdp_prep) free(rrd->cdp_prep); + if (rrd->rrd_value) free(rrd->rrd_value); +} -static int rrd_modify_r(const char *infilename, - const char *outfilename, - const char **removeDS, - const char **addDS, - rra_mod_op_t *rra_mod_ops, int rra_mod_ops_cnt, - int newstep) +static rrd_t *rrd_modify_structure(const rrd_t *in, + const char **removeDS, + const char **addDS, + rra_mod_op_t *rra_mod_ops, int rra_mod_ops_cnt) { - rrd_t in, out; + rrd_t *out; int rc = -1; unsigned int i, j; - rrd_file_t *rrd_file; - char *old_locale = NULL; char *ds_ops = NULL; unsigned int ds_ops_cnt = 0; int *ds_map = NULL; - - old_locale = setlocale(LC_NUMERIC, NULL); - setlocale(LC_NUMERIC, "C"); - - rrd_clear_error(); // reset error - - if (rrdc_is_any_connected()) { - // is it a good idea to just ignore the error ???? - rrdc_flush(infilename); - rrd_clear_error(); - } - - rrd_init(&in); - rrd_init(&out); - - rrd_file = rrd_open(infilename, &in, RRD_READONLY | RRD_READAHEAD); - if (rrd_file == NULL) { + + out = malloc(sizeof(rrd_t)); + if (out == NULL) { + rrd_set_error("Out of memory"); goto done; } - + rrd_init(out); + /* currently we only allow to modify version 3 RRDs. If other files should be modified, a dump/restore cycle should be done.... */ - - if (! (strcmp(in.stat_head->version, RRD_VERSION3) == 0 || strcmp(in.stat_head->version, RRD_VERSION) == 0) ) { + + if (! (strcmp(in->stat_head->version, RRD_VERSION3) == 0 || strcmp(in->stat_head->version, RRD_VERSION) == 0) ) { rrd_set_error("direct modification is only supported for version 3 or version 4 RRD files. Consider to dump/restore before retrying a modification"); goto done; } - - /* basic check: do we have a new step size: if we do: is it a smaller than - the original and is the old one a whole-number multiple of the new one? */ - int stretch = 0; - - if (newstep > 0) { - if (in.stat_head->pdp_step % newstep == 0 - && in.stat_head->pdp_step / newstep > 1) { - /* we will "stretch" the RRD: existing rows will correspond to the same - time period, but the CF will consolidate 'stretch' times as many PDPs. - */ - - stretch = in.stat_head->pdp_step / newstep; - } else { - rrd_set_error("invalid 'newstep' parameter. The newsize must " - "divide the old step parameter without a remainder."); - goto done; - } - } /* copy over structure to out RRD */ - - out.stat_head = (stat_head_t *) malloc(sizeof(stat_head_t)); - if (out.stat_head == NULL) { - rrd_set_error("rrd_modify_r: malloc failed."); + + out->stat_head = (stat_head_t *) malloc(sizeof(stat_head_t)); + if (out->stat_head == NULL) { + rrd_set_error("rrd_modify_r: malloc failed."); goto done; } - memset(out.stat_head, 0, (sizeof(stat_head_t))); - - strncpy(out.stat_head->cookie, "RRD", sizeof(out.stat_head->cookie)); - strcpy(out.stat_head->version, in.stat_head->version); - out.stat_head->float_cookie = FLOAT_COOKIE; - out.stat_head->pdp_step = in.stat_head->pdp_step; - - out.stat_head->ds_cnt = 0; - out.stat_head->rra_cnt = 0; - - out.live_head = copy_over_realloc(out.live_head, 0, in.live_head, 0, - sizeof(live_head_t)); - - if (out.live_head == NULL) goto done; - + memset(out->stat_head, 0, (sizeof(stat_head_t))); + + strncpy(out->stat_head->cookie, "RRD", sizeof(out->stat_head->cookie)); + strcpy(out->stat_head->version, in->stat_head->version); + out->stat_head->float_cookie = FLOAT_COOKIE; + out->stat_head->pdp_step = in->stat_head->pdp_step; + + out->stat_head->ds_cnt = 0; + out->stat_head->rra_cnt = 0; + + out->live_head = copy_over_realloc(out->live_head, 0, in->live_head, 0, + sizeof(live_head_t)); + + if (out->live_head == NULL) goto done; + /* use the ops array as a scratchpad to remember what we are about - to do to each DS. There is one entry for every DS in the - original RRD and one additional entry for every added DS. + to do to each DS. There is one entry for every DS in the + original RRD and one additional entry for every added DS. - Entries marked as - - 'c' will be copied to the out RRD, - - 'd' will not be copied (= will effectively be deleted) - - 'a' will be added. + Entries marked as + - 'c' will be copied to the out RRD, + - 'd' will not be copied (= will effectively be deleted) + - 'a' will be added. */ - ds_ops_cnt = in.stat_head->ds_cnt; + ds_ops_cnt = in->stat_head->ds_cnt; ds_ops = malloc(ds_ops_cnt); - + if (ds_ops == NULL) { - rrd_set_error("parse_tag_rrd: malloc failed."); + rrd_set_error("parse_tag_rrd: malloc failed."); goto done; } - - memset(ds_ops, 'c', in.stat_head->ds_cnt); + + memset(ds_ops, 'c', in->stat_head->ds_cnt); // record DSs to be deleted in ds_ops if (removeDS != NULL) { - for (unsigned int in_ds = 0 ; in_ds < in.stat_head->ds_cnt ; in_ds++) { + for (unsigned int in_ds = 0 ; in_ds < in->stat_head->ds_cnt ; in_ds++) { const char *c; for (j = 0, c = removeDS[j] ; c ; j++, c = removeDS[j]) { - if (strcmp(in.ds_def[in_ds].ds_nam, c) == 0) { + if (strcmp(in->ds_def[in_ds].ds_nam, c) == 0) { ds_ops[in_ds] = 'd'; break; } @@ -916,13 +884,13 @@ static int rrd_modify_r(const char *infilename, } } - if (copy_or_delete_DSs(&in, &out, ds_ops) != 0) { + if (copy_or_delete_DSs(in, out, ds_ops) != 0) { // error goto done; } /* now add any DS definitions to be added */ - int added_cnt = add_dss(&in, &out, addDS); + int added_cnt = add_dss(in, out, addDS); if (added_cnt < 0) { // error goto done; @@ -937,8 +905,8 @@ static int rrd_modify_r(const char *infilename, /* prepare explicit data source index to map from output index to input index */ - - ds_map = malloc(sizeof(int) * out.stat_head->ds_cnt); + + ds_map = malloc(sizeof(int) * out->stat_head->ds_cnt); j = 0; for (i = 0 ; i < ds_ops_cnt ; i++) { @@ -953,102 +921,205 @@ static int rrd_modify_r(const char *infilename, break; } } - + /* now take care to copy all RRAs, removing and adding columns for every row as needed for the requested DS changes */ - + /* we also reorder all rows, adding/removing rows as needed */ - + /* later on, we'll need to know the total number of rows for both RRDs in order to allocate memory. Luckily, handle_rra_defs will give that to us. */ int total_out_rra_rows = 0, total_in_rra_rows = 0; - - rc = handle_rra_defs(&in, &out, rra_mod_ops, rra_mod_ops_cnt, ds_ops, ds_ops_cnt, &total_in_rra_rows, &total_out_rra_rows); + + rc = handle_rra_defs(in, out, rra_mod_ops, rra_mod_ops_cnt, ds_ops, ds_ops_cnt, &total_in_rra_rows, &total_out_rra_rows); if (rc != 0) goto done; /* read and process all data ... */ - - /* there seem to be two function in the current rrdtool codebase - dealing with writing a new rrd file to disk: write_file and - rrd_create_fn. The latter has the major problem, that it tries - to free data passed to it (WTF?), but write_file assumes - chronologically ordered data in RRAs (that is, in the data - space starting at rrd.rrd_value.... + + /* there seem to be two function in the current rrdtool codebase dealing + with writing a new rrd file to disk: write_file and rrd_create_fn. The + latter has the major problem, that it tries to free data passed to it + (WTF?), but write_file assumes chronologically ordered data in RRAs (that + is, in the data space starting at rrd.rrd_value.... This is the reason why: - - we use write_file and - - why we reset cur_row in RRAs and reorder data to be cronological + - we use write_file and + - why we reset cur_row in RRAs and reorder data to be cronological */ - - /* prepare space to read data into */ - in.rrd_value = realloc(in.rrd_value, - total_in_rra_rows * in.stat_head->ds_cnt - * sizeof(rrd_value_t)); - if (in.rrd_value == NULL) { + /* prepare space for output data */ + out->rrd_value = realloc(out->rrd_value, + total_out_rra_rows * out->stat_head->ds_cnt + * sizeof(rrd_value_t)); + + if (out->rrd_value == NULL) { rrd_set_error("out of memory"); goto done; } + + rc = mod_rras(in, out, ds_map, rra_mod_ops, rra_mod_ops_cnt, ds_ops, ds_ops_cnt); + if (rc != 0) goto done; + + unsigned long hashed_name = 123213213; // FIXME FnvHash(outfilename); + + rc = add_rras(in, out, ds_map, rra_mod_ops, rra_mod_ops_cnt, hashed_name); + if (rc != 0) goto done; + - /* prepare space for output data */ - out.rrd_value = realloc(out.rrd_value, - total_out_rra_rows * out.stat_head->ds_cnt - * sizeof(rrd_value_t)); +done: + if (ds_ops != NULL) free(ds_ops); + if (ds_map != NULL) free(ds_map); + + if (rc != 0 && out != NULL) { + rrd_memory_free(out); + free(out); + out = NULL; + } + return out; +} + +static rrd_t *rrd_modify_r2(const rrd_t *in, + const char **removeDS, + const char **addDS, + rra_mod_op_t *rra_mod_ops, int rra_mod_ops_cnt, + int newstep) +{ + int rc = -1; + /* basic check: do we have a new step size: if we do: is it a smaller than + the original and is the old one a whole-number multiple of the new one? */ - if (out.rrd_value == NULL) { - rrd_set_error("out of memory"); + int stretch = 0; + rrd_t *out = NULL; + + if (newstep > 0) { + if (in->stat_head->pdp_step % newstep == 0 + && in->stat_head->pdp_step / newstep > 1) { + /* we will "stretch" the RRD: existing rows will correspond to the same + time period, but the CF will consolidate 'stretch' times as many PDPs. + */ + + stretch = in->stat_head->pdp_step / newstep; + } else { + rrd_set_error("invalid 'newstep' parameter. The newsize must " + "divide the old step parameter without a remainder."); + goto done; + } + } + + out = rrd_modify_structure(in, removeDS, addDS, rra_mod_ops, rra_mod_ops_cnt); + + if (stretch > 1) { + rc = stretch_rras(out, stretch); + if (rc != 0) goto done; + } + + rc = 0; +done: + if (rc != 0) { + if (out) { + rrd_memory_free(out); + free(out); + } + out = NULL; + } + + return out; +} + +/* copies the RRD named by infilename to a new RRD called outfilename. + + In that process, data sources may be removed or added. + + removeDS points to an array of strings, each naming a DS to be + removed. The list itself is NULL terminated. addDS points to a + similar list holding rrdcreate-style data source definitions to be + added. +*/ + +static int rrd_modify_r(const char *infilename, + const char *outfilename, + const char **removeDS, + const char **addDS, + rra_mod_op_t *rra_mod_ops, int rra_mod_ops_cnt, + int newstep) +{ + rrd_t in; + rrd_t *out = NULL; + + int rc = -1; + rrd_file_t *rrd_file; + char *old_locale = NULL; + + old_locale = setlocale(LC_NUMERIC, NULL); + setlocale(LC_NUMERIC, "C"); + + rrd_clear_error(); // reset error + + if (rrdc_is_any_connected()) { + // is it a good idea to just ignore the error ???? + rrdc_flush(infilename); + rrd_clear_error(); + } + + rrd_init(&in); + + rrd_file = rrd_open(infilename, &in, RRD_READONLY | RRD_READAHEAD); + if (rrd_file == NULL) { + // rrd error has been set goto done; } - /* - Before we do any other operation on RRAs, we read in all - data. This is important, because in the second pass we may - have to populate newly added rows from existing data - and we - might need any data we have from the input RRD. - */ + // read in data - have to count total number of rows for values array + + int total_in_rra_rows = 0; + for (unsigned int i = 0 ; i < in.stat_head->rra_cnt ; i++) { + total_in_rra_rows += in.rra_def[i].row_cnt; + } size_t to_read = total_in_rra_rows * sizeof(rrd_value_t) * in.stat_head->ds_cnt; size_t read_bytes; + /* prepare space to read data into */ + in.rrd_value = realloc(in.rrd_value, to_read); + if (in.rrd_value == NULL) { + rrd_set_error("Out of memory"); + goto done; + } + read_bytes = rrd_read(rrd_file, in.rrd_value, to_read); if (read_bytes != to_read) { - rrd_set_error("short read 2"); + rrd_set_error("short read"); goto done; } - rc = mod_rras(&in, &out, ds_map, rra_mod_ops, rra_mod_ops_cnt, ds_ops, ds_ops_cnt); - if (rc != 0) goto done; - - unsigned long hashed_name = FnvHash(outfilename); + // now we have read the input RRD... - rc = add_rras(&in, &out, ds_map, rra_mod_ops, rra_mod_ops_cnt, hashed_name); - if (rc != 0) goto done; + out = rrd_modify_r2(&in, removeDS, addDS, rra_mod_ops, rra_mod_ops_cnt, newstep); - if (stretch > 1) { - rc = stretch_rras(&out, stretch); - if (rc != 0) goto done; + if (out == NULL) { + goto done; } - - rc = write_rrd(outfilename, &out); + rc = write_rrd(outfilename, out); + done: - /* clean up */ - if (old_locale) - setlocale(LC_NUMERIC, old_locale); - - if (rrd_file != NULL) { + setlocale(LC_NUMERIC, old_locale); + + if (out) { + rrd_memory_free(out); + free(out); + out = NULL; + } + if (rrd_file) { rrd_close(rrd_file); } rrd_free(&in); - rrd_free(&out); - - if (ds_ops != NULL) free(ds_ops); - if (ds_map != NULL) free(ds_map); - + return rc; } + // prepare CDPs + values for new RRA static void prepare_CDPs(const rrd_t *in, rrd_t *out,