From de189fd7e0f60202714c8d18a3b5e17fd9abe24e Mon Sep 17 00:00:00 2001 From: Celine Mercier Date: Thu, 15 Sep 2016 11:47:02 +0200 Subject: [PATCH] Fixed major bug when cloning an AVL where the bloom filter was not copied properly (because the sutructure copy via assignation does not work for structures with a variable size) --- src/obiavl.c | 90 +++++++++++++++++++++++++++++++++++++++++----------- src/obiavl.h | 8 ++--- 2 files changed, 73 insertions(+), 25 deletions(-) diff --git a/src/obiavl.c b/src/obiavl.c index 8874687..e331a33 100644 --- a/src/obiavl.c +++ b/src/obiavl.c @@ -294,7 +294,7 @@ int remap_an_avl(OBIDMS_avl_p avl); * @warning The previous AVL in the list of the group is unmapped, * if it's not the 1st AVL being added. * - * @param avl A pointer on the AVL tree group structure. + * @param avl_group A pointer on the AVL tree group structure. * * @retval 0 if the operation was successfully completed. * @retval -1 if an error occurred. @@ -305,6 +305,24 @@ int remap_an_avl(OBIDMS_avl_p avl); int add_new_avl_in_group(OBIDMS_avl_group_p avl_group); +/** + * @brief Internal function adding an existing AVL in an AVL group. + * + * The AVL is hard-linked in the AVL group directory, and opened for that group. + * + * @param avl_group_dest A pointer on the destination AVL group to which the AVL should be added. + * @param avl_group_source A pointer on the source AVL group where the AVL already exists. + * @param avl_idx Index of the AVL in the source AVL group. + * + * @retval 0 if the operation was successfully completed. + * @retval -1 if an error occurred. + * + * @since June 2016 + * @author Celine Mercier (celine.mercier@metabarcoding.org) + */ +int add_existing_avl_in_group(OBIDMS_avl_group_p avl_group_dest, OBIDMS_avl_group_p avl_group_source, int avl_idx); + + /** * @brief Internal function testing if a value might already be stored in an AVL tree. * @@ -875,7 +893,6 @@ int grow_avl(OBIDMS_avl_p avl) // TODO Lock when needed { obi_set_errno(OBI_AVL_ERROR); obidebug(1, "\nError enlarging an AVL tree file"); - close(avl_file_descriptor); return -1; } @@ -885,7 +902,6 @@ int grow_avl(OBIDMS_avl_p avl) // TODO Lock when needed { obi_set_errno(OBI_AVL_ERROR); obidebug(1, "\nError munmapping the tree of an AVL tree file before enlarging"); - close(avl_file_descriptor); return -1; } @@ -901,7 +917,6 @@ int grow_avl(OBIDMS_avl_p avl) // TODO Lock when needed { obi_set_errno(OBI_AVL_ERROR); obidebug(1, "\nError re-mmapping the tree of an AVL tree file after enlarging the file"); - close(avl_file_descriptor); return -1; } @@ -936,7 +951,6 @@ int grow_avl_data(OBIDMS_avl_data_p avl_data) // TODO Lock when needed { obi_set_errno(OBI_AVL_ERROR); obidebug(1, "\nError enlarging an AVL tree data file"); - close(avl_data_file_descriptor); return -1; } @@ -946,7 +960,6 @@ int grow_avl_data(OBIDMS_avl_data_p avl_data) // TODO Lock when needed { obi_set_errno(OBI_AVL_ERROR); obidebug(1, "\nError munmapping the data of an AVL tree data file before enlarging"); - close(avl_data_file_descriptor); return -1; } @@ -962,7 +975,6 @@ int grow_avl_data(OBIDMS_avl_data_p avl_data) // TODO Lock when needed { obi_set_errno(OBI_AVL_ERROR); obidebug(1, "\nError re-mmapping the data of an AVL tree data file after enlarging the file"); - close(avl_data_file_descriptor); return -1; } @@ -996,7 +1008,12 @@ int close_avl_data(OBIDMS_avl_data_p avl_data) ret_val = -1; } - close(avl_data->data_fd); + if (close(avl_data->data_fd) < 0) + { + obi_set_errno(OBI_AVL_ERROR); + obidebug(1, "\nError closing an AVL tree data file"); + ret_val = -1; + } free(avl_data); @@ -1084,7 +1101,6 @@ int add_new_avl_in_group(OBIDMS_avl_group_p avl_group) } -// TODO doc int add_existing_avl_in_group(OBIDMS_avl_group_p avl_group_dest, OBIDMS_avl_group_p avl_group_source, int avl_idx) { if (link(get_full_path_of_avl_file_name(avl_group_source->dms, avl_group_source->name, avl_idx), get_full_path_of_avl_file_name(avl_group_dest->dms, avl_group_dest->name, avl_idx)) < 0) @@ -1103,7 +1119,7 @@ int add_existing_avl_in_group(OBIDMS_avl_group_p avl_group_dest, OBIDMS_avl_grou // Increment current AVL index (avl_group_dest->last_avl_idx)++; - // Open AVL for that group TODO ideally not needed, but needed for now + // Open AVL for that group TODO ideally not needed because AVL open twice, but needed for now avl_group_dest->sub_avls[avl_group_dest->last_avl_idx] = obi_open_avl(avl_group_source->dms, avl_group_source->name, avl_idx); if ((avl_group_dest->sub_avls)[avl_group_dest->last_avl_idx] == NULL) { @@ -1114,6 +1130,7 @@ int add_existing_avl_in_group(OBIDMS_avl_group_p avl_group_dest, OBIDMS_avl_grou return 0; } + int maybe_in_avl(OBIDMS_avl_p avl, Obi_blob_p value) { return (bloom_check(&((avl->header)->bloom_filter), value, obi_blob_sizeof(value))); @@ -1361,7 +1378,8 @@ void avl_print_node(OBIDMS_avl_p avl, AVL_node_p node, index_t node_idx, int dep putchar(' '); fprintf(stderr, "Node idx: %lld, Value idx: %lld, Left child: %lld, Right child: %lld, " - "Balance factor: %d, CRC: %llu\n", node_idx, node->value, node->left_child, node->right_child, node->balance_factor, node->crc64); + "Balance factor: %d, CRC: %llu\nValue:", node_idx, node->value, node->left_child, node->right_child, node->balance_factor, node->crc64); + print_bits(((avl->data)->data)+(node->value), obi_blob_sizeof((Obi_blob_p)(((avl->data)->data)+(node->value)))); if (node->right_child != -1) avl_print_node(avl, RIGHT_CHILD(node), node->right_child, depth+2); @@ -1679,8 +1697,6 @@ OBIDMS_avl_p obi_create_avl(OBIDMS_p dms, const char* avl_name, int avl_idx) avl->dms = dms; avl->data = avl_data; - avl->directory = dms->indexer_directory; - avl->dir_fd = avl_dir_fd; avl->avl_fd = avl_file_descriptor; (avl->header)->header_size = header_size; @@ -1696,6 +1712,13 @@ OBIDMS_avl_p obi_create_avl(OBIDMS_p dms, const char* avl_name, int avl_idx) free(complete_avl_name); + if (closedir(directory) < 0) + { + obi_set_errno(OBI_AVL_ERROR); + obidebug(1, "\nError closing an AVL directory"); + return NULL; + } + return avl; } @@ -1937,12 +1960,17 @@ OBIDMS_avl_p obi_open_avl(OBIDMS_p dms, const char* avl_name, int avl_idx) avl->dms = dms; avl->data = avl_data; - avl->directory = dms->indexer_directory; - avl->dir_fd = avl_dir_file_descriptor; avl->avl_fd = avl_file_descriptor; free(complete_avl_name); + if (closedir(directory) < 0) + { + obi_set_errno(OBI_AVL_ERROR); + obidebug(1, "\nError closing an AVL directory"); + return NULL; + } + return avl; } @@ -1996,6 +2024,9 @@ OBIDMS_avl_group_p obi_create_avl_group(OBIDMS_p dms, const char* avl_name) return NULL; } + // Store pointer on directory + avl_group->directory = opendir(avl_dir_name); + // Add in the list of open indexers obi_dms_list_indexer(dms, avl_group); @@ -2005,6 +2036,8 @@ OBIDMS_avl_group_p obi_create_avl_group(OBIDMS_p dms, const char* avl_name) // Set as writable avl_group->writable = true; + free(avl_dir_name); + return avl_group; } @@ -2051,11 +2084,16 @@ OBIDMS_avl_group_p obi_open_avl_group(OBIDMS_p dms, const char* avl_name) if ((avl_group->sub_avls)[i] == NULL) return NULL; } - avl_group->last_avl_idx = avl_count-1; // TODO latest. discuss + + // Store the index of the last AVL (the one to be modified) + avl_group->last_avl_idx = avl_count-1; strcpy(avl_group->name, avl_name); avl_group->dms = dms; + // Store pointer on directory + avl_group->directory = opendir(avl_dir_name); + // Add in the list of open indexers obi_dms_list_indexer(dms, avl_group); @@ -2065,6 +2103,8 @@ OBIDMS_avl_group_p obi_open_avl_group(OBIDMS_p dms, const char* avl_name) // Set as read-only avl_group->writable = false; + free(avl_dir_name); + return avl_group; } @@ -2085,10 +2125,10 @@ int obi_clone_avl(OBIDMS_avl_p avl, OBIDMS_avl_p new_avl) // Clone AVL tree memcpy(new_avl->tree, avl->tree, (avl->header)->avl_size); + memcpy(&((new_avl->header)->bloom_filter), &((avl->header)->bloom_filter), bloom_filter_size(MAX_NODE_COUNT_PER_AVL, BLOOM_FILTER_ERROR_RATE)); (new_avl->header)->avl_size = (avl->header)->avl_size; (new_avl->header)->nb_items = (avl->header)->nb_items; (new_avl->header)->root_idx = (avl->header)->root_idx; - (new_avl->header)->bloom_filter = (avl->header)->bloom_filter; // Clone AVL data memcpy((new_avl->data)->data, (avl->data)->data, ((avl->data)->header)->data_size_used); @@ -2174,7 +2214,12 @@ int obi_close_avl(OBIDMS_avl_p avl) ret_val = -1; } - close(avl->avl_fd); + if (close(avl->avl_fd) < 0) + { + obi_set_errno(OBI_AVL_ERROR); + obidebug(1, "\nError closing an AVL tree file"); + ret_val = -1; + } free(avl); @@ -2209,6 +2254,13 @@ int obi_close_avl_group(OBIDMS_avl_group_p avl_group) ret_val = -1; } + if (closedir(avl_group->directory) < 0) + { + obi_set_errno(OBI_AVL_ERROR); + obidebug(1, "\nError closing an AVL group directory"); + ret_val = -1; + } + free(avl_group); } @@ -2474,8 +2526,8 @@ index_t obi_avl_group_add(OBIDMS_avl_group_p avl_group, Obi_blob_p value) } // Add in the current AVL - bloom_add(&((((avl_group->sub_avls)[avl_group->last_avl_idx])->header)->bloom_filter), value, obi_blob_sizeof(value)); index_in_avl = (int32_t) obi_avl_add((avl_group->sub_avls)[avl_group->last_avl_idx], value); + bloom_add(&((((avl_group->sub_avls)[avl_group->last_avl_idx])->header)->bloom_filter), value, obi_blob_sizeof(value)); // Build the index containing the AVL index index_with_avl = avl_group->last_avl_idx; diff --git a/src/obiavl.h b/src/obiavl.h index 3fa0f29..45793d3 100644 --- a/src/obiavl.h +++ b/src/obiavl.h @@ -143,12 +143,6 @@ typedef struct OBIDMS_avl { OBIDMS_avl_data_p data; /**< A pointer to the structure containing the data * that the AVL tree references. */ - DIR* directory; /**< A directory entry usable to - * refer and scan the AVL tree directory. - */ - int dir_fd; /**< The file descriptor of the directory entry - * usable to refer and scan the AVL tree directory. - */ int avl_fd; /**< The file descriptor of the file containing the AVL tree. */ } OBIDMS_avl_t, *OBIDMS_avl_p; @@ -168,6 +162,8 @@ typedef struct OBIDMS_avl_group { */ bool writable; /**< Indicates whether the AVL group is read-only or not. */ + DIR* directory; /**< A directory entry usable to refer and scan the AVL group directory. + */ size_t counter; /**< Indicates by how many threads/programs (TODO) the AVL group is used. */ } OBIDMS_avl_group_t, *OBIDMS_avl_group_p;