From 99b687ae0051ab92cf806d1f38bab1fff42f395a Mon Sep 17 00:00:00 2001 From: Timo Sirainen Date: Mon, 2 Sep 2024 10:56:11 +0300 Subject: [PATCH] stats: Immediately allocate dynamic metric settings from persistent pool This avoids having to duplicate the memory later on, which gets more difficult in the following changes. --- src/stats/client-reader.c | 33 +++++++++++++++++++++++---------- src/stats/stats-metrics.c | 33 ++------------------------------- src/stats/stats-metrics.h | 2 +- src/stats/stats-settings.c | 2 +- src/stats/stats-settings.h | 2 ++ 5 files changed, 29 insertions(+), 43 deletions(-) diff --git a/src/stats/client-reader.c b/src/stats/client-reader.c index ca5b5a2110..89370c8120 100644 --- a/src/stats/client-reader.c +++ b/src/stats/client-reader.c @@ -7,6 +7,7 @@ #include "strescape.h" #include "connection.h" #include "ostream.h" +#include "settings.h" #include "master-service.h" #include "stats-metrics.h" #include "stats-settings.h" @@ -163,17 +164,27 @@ reader_client_input_metrics_add(struct reader_client *client, return -1; } - struct stats_metric_settings set = { - .name = args[0], - .description = args[1], - .fields = args[2], - .group_by = args[3], - .filter = args[4], - .exporter = args[5], - .exporter_include = args[6], - }; + pool_t pool = pool_alloconly_create("dynamic stats metrics", 128); + struct stats_metric_settings *set = + p_new(pool, struct stats_metric_settings, 1); + *set = stats_metric_default_settings; + set->pool = pool; + set->name = p_strdup(pool, args[0]); + set->description = p_strdup(pool, args[1]); + set->fields = p_strdup(pool, args[2]); + set->group_by = p_strdup(pool, args[3]); + set->filter = p_strdup(pool, args[4]); + set->exporter = p_strdup(pool, args[5]); + set->exporter_include = p_strdup(pool, args[6]); + + if (!stats_metric_setting_parser_info.check_func(set, pool, &error)) { + e_error(client->conn.event, "METRICS-ADD: %s", error); + pool_unref(&pool); + return -1; + } + o_stream_cork(client->conn.output); - if (stats_metrics_add_dynamic(stats_metrics, &set, &error)) { + if (stats_metrics_add_dynamic(stats_metrics, set, &error)) { client_writer_update_connections(); o_stream_nsend(client->conn.output, "+", 1); } else { @@ -181,6 +192,8 @@ reader_client_input_metrics_add(struct reader_client *client, o_stream_nsend_str(client->conn.output, "METRICS-ADD: "); o_stream_nsend_str(client->conn.output, error); } + settings_free(set); + o_stream_nsend(client->conn.output, "\n", 1); o_stream_uncork(client->conn.output); return 1; diff --git a/src/stats/stats-metrics.c b/src/stats/stats-metrics.c index 58b2446552..046f7982d3 100644 --- a/src/stats/stats-metrics.c +++ b/src/stats/stats-metrics.c @@ -235,24 +235,6 @@ static int stats_metrics_add_filter(struct stats_metrics *metrics, return ret; } -static struct stats_metric_settings * -stats_metric_settings_dup(pool_t pool, const struct stats_metric_settings *src) -{ - struct stats_metric_settings *set = p_new(pool, struct stats_metric_settings, 1); - - set->pool = pool; - pool_ref(pool); - set->name = p_strdup(pool, src->name); - set->description = p_strdup(pool, src->description); - set->fields = p_strdup(pool, src->fields); - set->group_by = p_strdup(pool, src->group_by); - set->filter = p_strdup(pool, src->filter); - set->exporter = p_strdup(pool, src->exporter); - set->exporter_include = p_strdup(pool, src->exporter_include); - - return set; -} - static struct metric * stats_metrics_find(struct stats_metrics *metrics, const char *name, unsigned int *idx_r) @@ -291,7 +273,7 @@ stats_metrics_check_for_exporter(struct stats_metrics *metrics, const char *name } bool stats_metrics_add_dynamic(struct stats_metrics *metrics, - struct stats_metric_settings *set, + const struct stats_metric_settings *set, const char **error_r) { unsigned int existing_idx ATTR_UNUSED; @@ -300,25 +282,14 @@ bool stats_metrics_add_dynamic(struct stats_metrics *metrics, return FALSE; } - struct stats_metric_settings *_set = - stats_metric_settings_dup(metrics->pool, set); - if (!stats_metric_setting_parser_info.check_func(_set, metrics->pool, error_r)) { - settings_free(_set); - return FALSE; - } - if (!stats_metrics_check_for_exporter(metrics, set->exporter)) { *error_r = t_strdup_printf("Exporter '%s' does not exist.", set->exporter); - settings_free(_set); return FALSE; } - if (stats_metrics_add_set(metrics, _set, error_r) < 0) { - settings_free(_set); + if (stats_metrics_add_set(metrics, set, error_r) < 0) return FALSE; - } - settings_free(_set); return TRUE; } diff --git a/src/stats/stats-metrics.h b/src/stats/stats-metrics.h index b211864dad..e2b43acc1c 100644 --- a/src/stats/stats-metrics.h +++ b/src/stats/stats-metrics.h @@ -109,7 +109,7 @@ struct metric { }; bool stats_metrics_add_dynamic(struct stats_metrics *metrics, - struct stats_metric_settings *set, + const struct stats_metric_settings *set, const char **error_r); bool stats_metrics_remove_dynamic(struct stats_metrics *metrics, diff --git a/src/stats/stats-settings.c b/src/stats/stats-settings.c index 59f9b2872b..e236dc2476 100644 --- a/src/stats/stats-settings.c +++ b/src/stats/stats-settings.c @@ -115,7 +115,7 @@ static const struct setting_define stats_metric_setting_defines[] = { SETTING_DEFINE_LIST_END }; -static const struct stats_metric_settings stats_metric_default_settings = { +const struct stats_metric_settings stats_metric_default_settings = { .name = "", .fields = "", .filter = "", diff --git a/src/stats/stats-settings.h b/src/stats/stats-settings.h index 0c075545a4..cb50f9ce8a 100644 --- a/src/stats/stats-settings.h +++ b/src/stats/stats-settings.h @@ -130,4 +130,6 @@ extern const struct setting_parser_info stats_setting_parser_info; extern const struct setting_parser_info stats_metric_setting_parser_info; extern const struct setting_parser_info stats_exporter_setting_parser_info; +extern const struct stats_metric_settings stats_metric_default_settings; + #endif -- 2.47.3