From afd42f46e69e4da82a4cec135a117881a544986d Mon Sep 17 00:00:00 2001 From: Shannon Klaus Date: Tue, 23 Jan 2024 09:18:48 -0700 Subject: [PATCH] Add errors to enable and disable metrics --- src/include/aerospike/aerospike_stats.h | 8 ++++---- src/include/aerospike/as_cluster.h | 8 ++++---- src/include/aerospike/as_metrics.h | 7 +++++-- src/main/aerospike/aerospike_stats.c | 20 ++++++++++++++------ src/main/aerospike/as_cluster.c | 20 ++++++++++++++------ src/main/aerospike/as_metrics.c | 25 +++++++++++++++++++------ 6 files changed, 60 insertions(+), 28 deletions(-) diff --git a/src/include/aerospike/aerospike_stats.h b/src/include/aerospike/aerospike_stats.h index 4718fd2f7..725073c8f 100644 --- a/src/include/aerospike/aerospike_stats.h +++ b/src/include/aerospike/aerospike_stats.h @@ -257,14 +257,14 @@ aerospike_stats_to_string(as_cluster_stats* stats); /** * Enable extended periodic cluster and node latency metrics. */ -AS_EXTERN void -aerospike_enable_metrics(aerospike* as, const struct as_policy_metrics_s* policy); +AS_EXTERN as_status +aerospike_enable_metrics(aerospike* as, as_error* err, const struct as_policy_metrics_s* policy); /** * Disable extended periodic cluster and node latency metrics. */ -AS_EXTERN void -aerospike_disable_metrics(aerospike* as); +AS_EXTERN as_status +aerospike_disable_metrics(aerospike* as, as_error* err); #ifdef __cplusplus } // end extern "C" diff --git a/src/include/aerospike/as_cluster.h b/src/include/aerospike/as_cluster.h index 626eaf919..a3137dc6e 100644 --- a/src/include/aerospike/as_cluster.h +++ b/src/include/aerospike/as_cluster.h @@ -536,15 +536,15 @@ as_partition_shm_get_node( * @private * Enable the collection of metrics */ -void -as_cluster_enable_metrics(as_cluster* cluster, as_policy_metrics* policy); +as_status +as_cluster_enable_metrics(as_error* err, as_cluster* cluster, as_policy_metrics* policy); /** * @private * Disable the collection of metrics */ -void -as_cluster_disable_metrics(as_cluster* cluster); +as_status +as_cluster_disable_metrics(as_error* err, as_cluster* cluster); /** * @private diff --git a/src/include/aerospike/as_metrics.h b/src/include/aerospike/as_metrics.h index 7c9df3f00..3ad80e460 100644 --- a/src/include/aerospike/as_metrics.h +++ b/src/include/aerospike/as_metrics.h @@ -20,6 +20,7 @@ #include #include #include +#include #include #if !defined(_MSC_VER) @@ -84,13 +85,13 @@ typedef struct as_policy_metrics_s { struct as_cluster_s; struct as_node_s; -typedef void (*as_metrics_enable_callback)(const struct as_policy_metrics_s* policy); +typedef as_status (*as_metrics_enable_callback)(as_error* err, const struct as_policy_metrics_s* policy); typedef void (*as_metrics_snapshot_callback)(const struct as_cluster_s* cluster, void* udata); typedef void (*as_metrics_node_close_callback)(const struct as_node_s* node, void* udata); -typedef void (*as_metrics_disable_callback)(const struct as_cluster_s* cluster, void* udata); +typedef as_status (*as_metrics_disable_callback)(as_error* err, const struct as_cluster_s* cluster, void* udata); typedef struct as_metrics_listeners_s { as_metrics_enable_callback enable_callback; @@ -117,6 +118,8 @@ typedef struct as_metrics_writer_s { int32_t latency_columns; int32_t latency_shift; + + const char* report_directory; } as_metrics_writer; const char* diff --git a/src/main/aerospike/aerospike_stats.c b/src/main/aerospike/aerospike_stats.c index 331621034..3850d3975 100644 --- a/src/main/aerospike/aerospike_stats.c +++ b/src/main/aerospike/aerospike_stats.c @@ -210,16 +210,24 @@ aerospike_stats_to_string(as_cluster_stats* stats) return sb.data; } -void -aerospike_enable_metrics(aerospike* as, const as_policy_metrics* policy) +as_status +aerospike_enable_metrics(aerospike* as, as_error* err, const struct as_policy_metrics_s* policy) { as_cluster* cluster = as->cluster; - as_cluster_enable_metrics(cluster, policy); + as_status status = as_cluster_enable_metrics(err, cluster, policy); + if (status != AEROSPIKE_OK) + { + return status; + } } -void -aerospike_disable_metrics(aerospike* as) +as_status +aerospike_disable_metrics(aerospike* as, as_error* err) { as_cluster* cluster = as->cluster; - as_cluster_disable_metrics(cluster); + as_status status = as_cluster_disable_metrics(err, cluster); + if (status != AEROSPIKE_OK) + { + return status; + } } diff --git a/src/main/aerospike/as_cluster.c b/src/main/aerospike/as_cluster.c index 45ec18ec4..a53603f2c 100644 --- a/src/main/aerospike/as_cluster.c +++ b/src/main/aerospike/as_cluster.c @@ -554,8 +554,8 @@ as_cluster_remove_nodes_copy(as_cluster* cluster, as_vector* /* */ no as_vector_append(cluster->gc, &item); } -void -as_cluster_enable_metrics(as_cluster* cluster, as_policy_metrics* policy) +as_status +as_cluster_enable_metrics(as_error* err, as_cluster* cluster, as_policy_metrics* policy) { if (cluster->metrics_enabled) { @@ -576,16 +576,24 @@ as_cluster_enable_metrics(as_cluster* cluster, as_policy_metrics* policy) as_node_enable_metrics(node, policy); } - cluster->metrics_listeners->enable_callback(policy, policy->udata); + as_status status = cluster->metrics_listeners->enable_callback(err, policy, policy->udata); + if (status != AEROSPIKE_OK) + { + return status; + } } -void -as_cluster_disable_metrics(as_cluster* cluster) +as_status +as_cluster_disable_metrics(as_error* err, as_cluster* cluster) { if (cluster->metrics_enabled) { cluster->metrics_enabled = false; - cluster->metrics_listeners->disable_callback(cluster->metrics_policy, cluster, cluster->metrics_policy->udata); + as_status status = cluster->metrics_listeners->disable_callback(cluster->metrics_policy, cluster, cluster->metrics_policy->udata); + if (status != AEROSPIKE_OK) + { + return status; + } } } diff --git a/src/main/aerospike/as_metrics.c b/src/main/aerospike/as_metrics.c index 2eb48f7fb..6861d69f5 100644 --- a/src/main/aerospike/as_metrics.c +++ b/src/main/aerospike/as_metrics.c @@ -141,21 +141,29 @@ as_metrics_add_latency(as_node_metrics* node_metrics, as_latency_type latency_ty as_metrics_latency_buckets_add(&node_metrics->latency[latency_type], elapsed); } -void -as_metrics_writer_enable(const struct as_policy_metrics_s* policy) +as_status +as_metrics_writer_enable(as_error* err, const struct as_policy_metrics_s* policy) { if (policy->report_size_limit != 0 && policy->report_size_limit < MIN_FILE_SIZE) { - // error + return as_error_update(err, AEROSPIKE_ERR_CLIENT, + "Metrics policy report_size_limit %d must be at least %d", policy->report_size_limit, MIN_FILE_SIZE); } // create file directory as_metrics_writer* mw = policy->udata; mw->file = fopen(policy->report_directory, "w"); + + if (!mw->file) + { + return as_error_update(err, AEROSPIKE_ERR_CLIENT, + "Failed to open file: %s", policy->report_directory); + } mw->max_size = policy->report_size_limit; mw->latency_columns = policy->latency_columns; mw->latency_shift = policy->latency_shift; mw->size = 0; + mw->report_directory = policy->report_directory; as_string_builder_inita(mw->sb, 25, true); as_string_builder_append(&mw->sb, utc_time_str(time(NULL))); @@ -198,17 +206,22 @@ as_metrics_writer_node_close(const struct as_node_s* node, void* udata) } } -void -as_metrics_writer_disable(const struct as_cluster_s* cluster, void* udata) +as_status +as_metrics_writer_disable(as_error* err, const struct as_cluster_s* cluster, void* udata) { // write cluster into to file, disable as_metrics_writer* mw = udata; if (mw->enable && mw->file != NULL) { as_metrics_write_cluster(mw, cluster); - fclose(mw->file); + uint32_t result = fclose(mw->file); mw->file = NULL; mw->enable = false; + if (result != 0) + { + return as_error_update(err, AEROSPIKE_ERR_CLIENT, + "File stream did not close successfully: %s", mw->report_directory); + } } }