From fa2097e720c56ac5073b4c32b1f069d3c293b304 Mon Sep 17 00:00:00 2001 From: chenzizhan Date: Mon, 22 Jul 2024 14:32:59 +0800 Subject: better error codes --- include/fieldstat/fieldstat.h | 14 ++++++++-- src/cube.c | 60 ++++++++++++++++++++++------------------ src/cube.h | 1 - src/fieldstat.c | 11 ++++---- test/test_fuzz_test.cpp | 2 +- test/test_merge.cpp | 2 +- test/test_register_and_reset.cpp | 38 ++++++++++++++++--------- 7 files changed, 77 insertions(+), 51 deletions(-) diff --git a/include/fieldstat/fieldstat.h b/include/fieldstat/fieldstat.h index 9bbdb1f..162a235 100644 --- a/include/fieldstat/fieldstat.h +++ b/include/fieldstat/fieldstat.h @@ -13,9 +13,15 @@ extern "C" #define FS_ERR_NULL_HANDLER -2 #define FS_ERR_INVALID_CUBE_ID -3 #define FS_ERR_INVALID_METRIC_ID -4 -#define FS_ERR_INVALID_TAG -5 +#define FS_ERR_INVALID_DIMENSION -5 #define FS_ERR_INVALID_PARAM -6 -#define FS_ERR_INVALID_KEY -7 +#define FS_ERR_INVALID_METRIC_TYPE -8 +#define FS_ERR_MAX_N_CELL_LESS_THAN_ZERO -9 +#define FS_ERR_DIMENSION_ALREADY_EXISTS -10 +#define FS_ERR_METRIC_NAME_ALREADY_EXISTS -11 +#define FS_ERR_CUBE_SAMPLING_NOT_INITIALIZED -12 +#define FS_ERR_OPERATION_NOT_SUPPORTED_FOR_PRIMARY_METRIC -13 +#define FS_ERR_DIFFERENT_CONFIGURATION_FOR_SAME_CUBE -14 enum metric_type { @@ -58,7 +64,7 @@ struct fieldstat *fieldstat_fork(const struct fieldstat *instance); * the configurations will be kept as much as possible, like cells in the same cube will be kept, but the cells in different cubes will be deleted. * @ return FS_OK or FS_ERR_NULL_HANDLER */ -int fieldstat_calibrate(const struct fieldstat *master, struct fieldstat *replica); +void fieldstat_calibrate(const struct fieldstat *master, struct fieldstat *replica); /* * @brief add an cube to this instance. Cube represents an template with a user-defined set of cells and metrics. * @param cube_dimensions: tags that are shared by all cells in this cube. This is the key of the cube. Can be NULL. Must be unique. Shared_tags are ordered, which means that {"TAG_KEY": "123", "TAG_KEY2": "456"} and {"TAG_KEY2": "456", "TAG_KEY": "123"} are map to different cube. @@ -190,8 +196,10 @@ void fieldstat_cube_get_cells(const struct fieldstat *instance, int cube_id, str get the field of fieldstat_cube_create. User free them by calling fieldstat_tag_list_arr_free(struct field_list *, 1) return NULL when ID is invalid. */ +// todo: tags struct field_list *fieldstat_cube_get_tags(const struct fieldstat *instance, int cube_id); + /* return a cube id corresponding to `cube_dimensions`. FS_ERR_INVALID_KEY is returned if the cube is not found. */ diff --git a/src/cube.c b/src/cube.c index abc3a66..c5c21e6 100644 --- a/src/cube.c +++ b/src/cube.c @@ -534,7 +534,7 @@ int cube_set_sampling_mode(struct cube *cube, enum sampling_mode mode, int max_n } if ((mode == SAMPLING_MODE_TOPK && manifest->type != METRIC_TYPE_COUNTER) || (mode == SAMPLING_MODE_TOP_CARDINALITY && manifest->type != METRIC_TYPE_HLL)) { - return FS_ERR_INVALID_PARAM; + return FS_ERR_INVALID_METRIC_TYPE; } if (cube->primary_metric_id != -1) { @@ -632,20 +632,6 @@ void cube_reset(struct cube *cube) { } } -int cube_set_primary_metric(struct cube *cube, int metric_id) { - const struct metric_manifest *manifest = metric_manifest_manager_get_by_id(cube->manifest_manager, metric_id); - if (manifest == NULL) { - return FS_ERR_INVALID_METRIC_ID; - } - if (cube->sampling_mode == SAMPLING_MODE_COMPREHENSIVE || - (cube->sampling_mode == SAMPLING_MODE_TOPK && manifest->type != METRIC_TYPE_COUNTER) || - (cube->sampling_mode == SAMPLING_MODE_TOP_CARDINALITY && manifest->type != METRIC_TYPE_HLL)) { - return FS_ERR_INVALID_PARAM; - } - cube->primary_metric_id = metric_id; - return FS_OK; -} - struct cell *get_cell_in_comprehensive_cube(struct cube *cube, const struct field *dimensions, size_t n_dimension) { char *key; size_t key_len; @@ -761,7 +747,7 @@ int cube_register_counter(struct cube *cube, const char *metric_name) { free(metric->name); free(metric->parameters); free(metric); - return FS_ERR_INVALID_KEY; + return FS_ERR_METRIC_NAME_ALREADY_EXISTS; } metric->id = id; @@ -783,7 +769,7 @@ int cube_register_hll(struct cube *cube,const char *metric_name, unsigned char p free(metric->name); free(metric->parameters); free(metric); - return FS_ERR_INVALID_KEY; + return FS_ERR_METRIC_NAME_ALREADY_EXISTS; } metric->id = id; @@ -813,7 +799,7 @@ int cube_register_hist(struct cube *cube,const char *metric_name, long long lowe free(metric->name); free(metric->parameters); free(metric); - return FS_ERR_INVALID_KEY; + return FS_ERR_METRIC_NAME_ALREADY_EXISTS; } metric->id = id; @@ -821,6 +807,9 @@ int cube_register_hist(struct cube *cube,const char *metric_name, long long lowe } int cube_histogram_record(struct cube *cube, int metric_id, const struct field *dimensions, size_t n_dimensions, long long value) { + if (cube->primary_metric_id == -1) { + return FS_ERR_CUBE_SAMPLING_NOT_INITIALIZED; + } assert(cube->sampling_mode == SAMPLING_MODE_COMPREHENSIVE || (cube->primary_metric_id != metric_id)); const struct metric_manifest *manifest = metric_manifest_manager_get_by_id(cube->manifest_manager, metric_id); @@ -857,6 +846,10 @@ int cube_histogram_record(struct cube *cube, int metric_id, const struct field * } int cube_hll_add(struct cube *cube, int metric_id, const struct field *dimensions, size_t n_dimensions, const char *key, size_t key_len) { + if (cube->primary_metric_id == -1) { + return FS_ERR_CUBE_SAMPLING_NOT_INITIALIZED; + } + assert(cube->sampling_mode != SAMPLING_MODE_TOPK || cube->primary_metric_id != metric_id); const struct metric_manifest *manifest = metric_manifest_manager_get_by_id(cube->manifest_manager, metric_id); @@ -920,6 +913,9 @@ uint64_t field_array_to_hash(const struct field *field, size_t n_dimensions) { int cube_hll_add_field(struct cube *cube, int metric_id, const struct field *dimensions, size_t n_dimensions, const struct field *tags_key, size_t n_tag_key) { + if (cube->primary_metric_id == -1) { + return FS_ERR_CUBE_SAMPLING_NOT_INITIALIZED; + } assert(cube->sampling_mode != SAMPLING_MODE_TOPK || (cube->primary_metric_id != metric_id)); const struct metric_manifest *manifest = metric_manifest_manager_get_by_id(cube->manifest_manager, metric_id); if (manifest == NULL || manifest->type != METRIC_TYPE_HLL) { @@ -970,9 +966,14 @@ int cube_hll_add_field(struct cube *cube, int metric_id, const struct field *dim } int cube_counter_incrby(struct cube *cube, int metric_id, const struct field *dimensions, size_t n_dimensions, long long increment) { - assert(cube->primary_metric_id != -1); + if (cube->primary_metric_id == -1) { + return FS_ERR_CUBE_SAMPLING_NOT_INITIALIZED; + } + if (cube->sampling_mode == SAMPLING_MODE_TOPK && cube->primary_metric_id == metric_id && increment < 0) { + return FS_ERR_OPERATION_NOT_SUPPORTED_FOR_PRIMARY_METRIC; + } assert(cube->sampling_mode == SAMPLING_MODE_COMPREHENSIVE || - (cube->sampling_mode == SAMPLING_MODE_TOPK && (cube->primary_metric_id != metric_id || increment >= 0)) || + (cube->sampling_mode == SAMPLING_MODE_TOPK) || (cube->sampling_mode == SAMPLING_MODE_TOP_CARDINALITY && cube->primary_metric_id != metric_id) ); @@ -1026,7 +1027,12 @@ int cube_counter_incrby(struct cube *cube, int metric_id, const struct field *di } int cube_counter_set(struct cube *cube, int metric_id, const struct field *dimensions, size_t n_dimensions, long long value) { - assert(cube->sampling_mode == SAMPLING_MODE_COMPREHENSIVE || (cube->primary_metric_id != metric_id)); + if (cube->primary_metric_id == -1) { + return FS_ERR_CUBE_SAMPLING_NOT_INITIALIZED; + } + if (cube->sampling_mode == SAMPLING_MODE_TOPK && cube->primary_metric_id == metric_id) { + return FS_ERR_OPERATION_NOT_SUPPORTED_FOR_PRIMARY_METRIC; + } const struct metric_manifest *manifest = metric_manifest_manager_get_by_id(cube->manifest_manager, metric_id); if (manifest == NULL || manifest->type != METRIC_TYPE_COUNTER) { @@ -1088,7 +1094,7 @@ struct cube *cube_copy(const struct cube *cube) int cube_merge(struct cube *dest, const struct cube *src) { if (dest->sampling_mode != src->sampling_mode) { - return FS_ERR_INVALID_PARAM; + return FS_ERR_DIFFERENT_CONFIGURATION_FOR_SAME_CUBE; } size_t n_metric_src = 0; @@ -1098,10 +1104,10 @@ int cube_merge(struct cube *dest, const struct cube *src) int len_min = n_metric_src < n_metric_dst ? n_metric_src : n_metric_dst; for (int i = 0; i < len_min; i++) { if (list_src[i]->type != list_dst[i]->type) { - return FS_ERR_INVALID_PARAM; + return FS_ERR_DIFFERENT_CONFIGURATION_FOR_SAME_CUBE; } if (strcmp(list_src[i]->name, list_dst[i]->name) != 0) { - return FS_ERR_INVALID_PARAM; + return FS_ERR_DIFFERENT_CONFIGURATION_FOR_SAME_CUBE; } } for (int i = n_metric_dst; i < n_metric_src; i++) { @@ -1292,7 +1298,7 @@ const struct metric *get_metric_by_tag_list(const struct cube *cube, const struc const struct cell *data = get_cell_by_tag_list(cube, fields); if (data == NULL) { - *ret = FS_ERR_INVALID_TAG; + *ret = FS_ERR_INVALID_DIMENSION; return NULL; } @@ -1318,7 +1324,7 @@ int cube_counter_get(const struct cube *cube, int metric_id, const struct field_ *value = count; free(dimension_in_string); - return count == 0 ? FS_ERR_INVALID_TAG : FS_OK; + return count == 0 ? FS_ERR_INVALID_DIMENSION : FS_OK; } int ret; @@ -1344,7 +1350,7 @@ int cube_hll_get(const struct cube *cube, int metric_id, const struct field_list double hll_value = spread_sketch_get_cardinality(cube->spread_sketch, dimension_in_string, dimension_string_len); free(dimension_in_string); if (hll_value < 0) { - return FS_ERR_INVALID_TAG; + return FS_ERR_INVALID_DIMENSION; } *value = hll_value; diff --git a/src/cube.h b/src/cube.h index 842f064..c7a5893 100644 --- a/src/cube.h +++ b/src/cube.h @@ -43,7 +43,6 @@ enum sampling_mode cube_get_sampling_mode(const struct cube *cube); void cube_get_cells(const struct cube *cube, struct field_list **tag_list, size_t *n_cell); void cube_get_metrics(const struct cube *cube, int **metric_id_out, size_t *n_metric); void cube_get_metrics_in_cell(const struct cube *cube, const struct field_list *dimensions, int **metric_id_out, size_t *n_metric_out); -int cube_set_primary_metric(struct cube *cube, int metric_id); struct field_list *cube_get_identifier(const struct cube *cube); const char *cube_get_metric_name(const struct cube *cube, int metric_id); enum metric_type cube_get_metric_type(const struct cube *cube, int metric_id); diff --git a/src/fieldstat.c b/src/fieldstat.c index 002d754..5c255ed 100644 --- a/src/fieldstat.c +++ b/src/fieldstat.c @@ -80,9 +80,10 @@ void fieldstat_free_tag_array(struct field *fields, size_t n_tags) int fieldstat_cube_set_sampling(struct fieldstat *instance, int cube_id, enum sampling_mode mode, int max_n_cell, int primary_metric_id) { if (max_n_cell <= 0) { if (mode != SAMPLING_MODE_COMPREHENSIVE) { - return FS_ERR_INVALID_PARAM; + return FS_ERR_MAX_N_CELL_LESS_THAN_ZERO; + } else { + max_n_cell = INT32_MAX; } - max_n_cell = INT32_MAX; } struct cube *cube = cube_manager_get_cube_by_id(instance->cube_manager, cube_id); @@ -108,7 +109,7 @@ int fieldstat_cube_create(struct fieldstat *instance, const struct field *cube_d int ret = cube_manager_add(instance->cube_manager, cube); if (ret < 0) { cube_free(cube); - return FS_ERR_INVALID_KEY; + return FS_ERR_DIMENSION_ALREADY_EXISTS; } return ret; //ret is the cube_id @@ -226,7 +227,7 @@ struct fieldstat *fieldstat_fork(const struct fieldstat *instance) return new_instance; } -int fieldstat_calibrate(const struct fieldstat *master, struct fieldstat *replica) +void fieldstat_calibrate(const struct fieldstat *master, struct fieldstat *replica) { cube_manager_calibrate(replica->cube_manager, master->cube_manager); @@ -376,7 +377,7 @@ int fieldstat_find_cube(const struct fieldstat *instance, const struct field *cu int cube_id = cube_manager_find(instance->cube_manager, cube_dimensions, n_dimensions); if (cube_id == -1) { - return FS_ERR_INVALID_KEY; + return FS_ERR_INVALID_DIMENSION; } return cube_id; diff --git a/test/test_fuzz_test.cpp b/test/test_fuzz_test.cpp index 0fa8241..1a21bcd 100644 --- a/test/test_fuzz_test.cpp +++ b/test/test_fuzz_test.cpp @@ -532,7 +532,7 @@ TEST(Fuzz_test, add_and_reset_with_randomly_generated_flows_and_randomly_chosen_ tag_list_tmp.field = (struct field *)tag_list_wrapper[i]->get_tag(); tag_list_tmp.n_field = tag_list_wrapper[i]->get_tag_count(); int counter_exist = fieldstat_counter_get(instance, cube_id, &tag_list_tmp, using_id, &value); - ASSERT_EQ(counter_exist, FS_ERR_INVALID_TAG); // the field is not added to the cube + ASSERT_EQ(counter_exist, FS_ERR_INVALID_DIMENSION); // the field is not added to the cube fieldstat_tag_list_arr_free(tag_list, n_cell); } diff --git a/test/test_merge.cpp b/test/test_merge.cpp index 5024cff..194002a 100644 --- a/test/test_merge.cpp +++ b/test/test_merge.cpp @@ -502,7 +502,7 @@ TEST(unit_test_merge, primary_metric_id_different) int metric_primary_dst = fieldstat_register_counter(instance_dst, cube_id_dst, "primary"); fieldstat_cube_set_sampling(instance_dst, cube_id_dst, SAMPLING_MODE_TOPK, 2, metric_primary_dst); - EXPECT_EQ(fieldstat_merge(instance_dst, instance), FS_ERR_INVALID_PARAM); + EXPECT_EQ(fieldstat_merge(instance_dst, instance), FS_ERR_DIFFERENT_CONFIGURATION_FOR_SAME_CUBE); fieldstat_free(instance); fieldstat_free(instance_dst); diff --git a/test/test_register_and_reset.cpp b/test/test_register_and_reset.cpp index 380b156..6013b74 100644 --- a/test/test_register_and_reset.cpp +++ b/test/test_register_and_reset.cpp @@ -34,7 +34,7 @@ TEST(test_register, delete_comprehensive_cube_with_cells_and_metrics) struct field_list *tag_list = fieldstat_cube_get_tags(instance, cube_id); EXPECT_EQ(tag_list, nullptr); int cube_id_ret = fieldstat_find_cube(instance, &TEST_SHARED_TAG, 1); - EXPECT_EQ(cube_id_ret, FS_ERR_INVALID_KEY); + EXPECT_EQ(cube_id_ret, FS_ERR_INVALID_DIMENSION); fieldstat_free(instance); } @@ -51,7 +51,7 @@ TEST(test_register, delete_topk_cube_with_cells_and_metrics) struct field_list *tag_list = fieldstat_cube_get_tags(instance, cube_id); EXPECT_EQ(tag_list, nullptr); int cube_id_ret = fieldstat_find_cube(instance, &TEST_SHARED_TAG, 1); - EXPECT_EQ(cube_id_ret, FS_ERR_INVALID_KEY); + EXPECT_EQ(cube_id_ret, FS_ERR_INVALID_DIMENSION); fieldstat_free(instance); } @@ -71,7 +71,7 @@ TEST(test_register, delete_spreadsketch_cube_with_cells_and_metrics) struct field_list *tag_list = fieldstat_cube_get_tags(instance, cube_id); EXPECT_EQ(tag_list, nullptr); int cube_id_ret = fieldstat_find_cube(instance, &TEST_SHARED_TAG, 1); - EXPECT_EQ(cube_id_ret, FS_ERR_INVALID_KEY); + EXPECT_EQ(cube_id_ret, FS_ERR_INVALID_DIMENSION); fieldstat_free(instance); } @@ -94,7 +94,7 @@ TEST(test_register, reset_and_try_to_query_cell_comprehensive) fieldstat_reset(instance); long long value; - EXPECT_EQ(fieldstat_counter_get(instance, cube_id, &TEST_TAG_LIST_INT, metric_id, &value), FS_ERR_INVALID_TAG); + EXPECT_EQ(fieldstat_counter_get(instance, cube_id, &TEST_TAG_LIST_INT, metric_id, &value), FS_ERR_INVALID_DIMENSION); size_t n_cell; struct field_list *tag_list; @@ -115,7 +115,7 @@ TEST(test_register, reset_and_try_to_query_cell_topk) fieldstat_reset(instance); long long value; - EXPECT_EQ(fieldstat_counter_get(instance, cube_id, &TEST_TAG_LIST_INT, metric_id, &value), FS_ERR_INVALID_TAG); + EXPECT_EQ(fieldstat_counter_get(instance, cube_id, &TEST_TAG_LIST_INT, metric_id, &value), FS_ERR_INVALID_DIMENSION); size_t n_cell; struct field_list *tag_list; @@ -135,7 +135,7 @@ TEST(test_register, reset_and_try_to_query_cell_spreadsketch) fieldstat_reset(instance); double value; - EXPECT_EQ(fieldstat_hll_get(instance, cube_id, &TEST_TAG_LIST_INT, metric_id, &value), FS_ERR_INVALID_TAG); + EXPECT_EQ(fieldstat_hll_get(instance, cube_id, &TEST_TAG_LIST_INT, metric_id, &value), FS_ERR_INVALID_DIMENSION); size_t n_cell; struct field_list *tag_list; @@ -496,7 +496,7 @@ TEST(test_register, register_cube_twice) { struct fieldstat *instance = fieldstat_new(); test_fieldstat_cube_create(instance, &TEST_SHARED_TAG, 1, SAMPLING_MODE_COMPREHENSIVE, 10); int cube_id2 = test_fieldstat_cube_create(instance, &TEST_SHARED_TAG, 1, SAMPLING_MODE_COMPREHENSIVE, 10); - EXPECT_EQ(cube_id2, FS_ERR_INVALID_KEY); + EXPECT_EQ(cube_id2, FS_ERR_DIMENSION_ALREADY_EXISTS); fieldstat_free(instance); } @@ -507,7 +507,19 @@ TEST(test_register, find_cube) { int find_cube_id = fieldstat_find_cube(instance, &TEST_SHARED_TAG, 1); EXPECT_EQ(cube_id, find_cube_id); int find_cube_id2 = fieldstat_find_cube(instance, &TEST_TAG_DOUBLE, 1); - EXPECT_EQ(find_cube_id2, FS_ERR_INVALID_KEY); + EXPECT_EQ(find_cube_id2, FS_ERR_INVALID_DIMENSION); + + fieldstat_free(instance); +} + +TEST(test_register, set_sampling_error) { + struct fieldstat *instance = fieldstat_new(); + int cube_id = fieldstat_cube_create(instance, &TEST_SHARED_TAG, 1); + int ret = fieldstat_cube_set_sampling(instance, cube_id, SAMPLING_MODE_TOPK, 10, 0); + EXPECT_EQ(ret, FS_ERR_INVALID_METRIC_ID); + int metric_id = fieldstat_register_hll(instance, cube_id, "hll", 5); + ret = fieldstat_cube_set_sampling(instance, cube_id, SAMPLING_MODE_TOPK, 10, metric_id); + EXPECT_EQ(ret, FS_ERR_INVALID_METRIC_TYPE); fieldstat_free(instance); } @@ -518,7 +530,7 @@ TEST(test_register, register_metric_twice) { fieldstat_register_counter(instance, cube_id, "counter"); int metric_id2 = fieldstat_register_counter(instance, cube_id, "counter"); - EXPECT_EQ(metric_id2, FS_ERR_INVALID_KEY); + EXPECT_EQ(metric_id2, FS_ERR_METRIC_NAME_ALREADY_EXISTS); fieldstat_free(instance); } @@ -527,7 +539,7 @@ TEST(test_register, add_on_uninitialized_cube) { int cube_id = fieldstat_cube_create(instance, &TEST_SHARED_TAG, 1); int metric_id = fieldstat_register_counter(instance, cube_id, "counter"); // fieldstat_counter_incrby(instance, cube_id, metric_id, &TEST_TAG_INT, 1, 1); - EXPECT_DEATH(fieldstat_counter_incrby(instance, cube_id, metric_id, &TEST_TAG_INT, 1, 1), ".*"); + EXPECT_EQ(fieldstat_counter_incrby(instance, cube_id, metric_id, &TEST_TAG_INT, 1, 1), FS_ERR_CUBE_SAMPLING_NOT_INITIALIZED); fieldstat_free(instance); } @@ -621,7 +633,7 @@ TEST(calibrate, target_more_cube) EXPECT_STREQ(tag_list->field[0].key, TEST_SHARED_TAG.key); EXPECT_EQ(fieldstat_find_cube(target, &TEST_SHARED_TAG, 1), 0); - EXPECT_EQ(fieldstat_find_cube(target, &TEST_TAG_STRING, 1), FS_ERR_INVALID_KEY); + EXPECT_EQ(fieldstat_find_cube(target, &TEST_TAG_STRING, 1), FS_ERR_INVALID_DIMENSION); fieldstat_free(master); fieldstat_free(target); @@ -748,7 +760,7 @@ TEST(calibrate, issue_calibrate_wrong) int n_cubes = 0; fieldstat_get_cubes(target, &cubes_id_ret, &n_cubes); EXPECT_EQ(n_cubes, 1); - EXPECT_EQ(fieldstat_find_cube(target, tag_A, 1), FS_ERR_INVALID_KEY); + EXPECT_EQ(fieldstat_find_cube(target, tag_A, 1), FS_ERR_INVALID_DIMENSION); EXPECT_EQ(fieldstat_find_cube(target, tag_B, 1), cubes_id_ret[0]); struct field_list *tag_list = fieldstat_cube_get_tags(target, cubes_id_ret[0]); @@ -780,7 +792,7 @@ TEST(calibrate, delete_first_cube) EXPECT_EQ(cubes_id_ret[0], cube_idB); free(cubes_id_ret); - EXPECT_EQ(fieldstat_find_cube(target, tag_A, 1), FS_ERR_INVALID_KEY); + EXPECT_EQ(fieldstat_find_cube(target, tag_A, 1), FS_ERR_INVALID_DIMENSION); EXPECT_EQ(fieldstat_find_cube(target, tag_B, 1), 1); struct field_list *tag_list = fieldstat_cube_get_tags(target, 1); -- cgit v1.2.3