From cb3ec978fd182f770ba9e9688143737f26801a36 Mon Sep 17 00:00:00 2001 From: Aurelien DARRAGON Date: Fri, 24 Nov 2023 15:37:40 +0100 Subject: [PATCH] MINOR: event_hdl: add global tunables The local variable "event_hdl_async_max_notif_at_once" which was introduced with the event_hdl API was left as is but with a TODO note telling that we should make it a global tunable. Well, we're doing this now. To prepare for upcoming tunables related to event_hdl API, we add a dedicated struct named event_hdl_tune which is globally exposed through the event_hdl header file so that it may be used from everywhere. The struct is automatically initialized in event_hdl_init() according to defaults.h. "event_hdl_async_max_notif_at_once" now becomes "event_hdl_tune.max_events_at_once" with it's dedicated configuation keyword: "tune.events.max-events-at-once". We're also taking this opportunity to raise the default value from 10 to 100 since it's seems quite reasonnable given existing async event_hdl users. The documentation was updated accordingly. --- doc/configuration.txt | 11 +++++++++ include/haproxy/defaults.h | 7 ++++++ include/haproxy/event_hdl-t.h | 5 +++++ include/haproxy/event_hdl.h | 3 +++ src/event_hdl.c | 42 ++++++++++++++++++++++++++++++----- 5 files changed, 63 insertions(+), 5 deletions(-) diff --git a/doc/configuration.txt b/doc/configuration.txt index 53414dea8..8b7f4aa19 100644 --- a/doc/configuration.txt +++ b/doc/configuration.txt @@ -1176,6 +1176,7 @@ The following keywords are supported in the "global" section : - tune.comp.maxlevel - tune.disable-fast-forward - tune.disable-zero-copy-forwarding + - tune.events.max-events-at-once - tune.fail-alloc - tune.fd.edge-triggered - tune.h2.be.initial-window-size @@ -2918,6 +2919,16 @@ tune.disable-zero-copy-forwarding directive, it is possible to disable this optimization. Note it also disable any kernel tcp splicing. +tune.events.max-events-at-once + Sets the number of events that may be processed at once by an asynchronous + task handler (from event_hdl API). should be included between 1 + and 10000. Large number could cause thread contention as a result of the + task doing heavy work without interruption, and on the other hand, small + number could result in the task being constantly rescheduled because it + cannot consume enough events per run and is not able to catch up with the + event producer. The default value may be forced at build time, otherwise + defaults to 100. + tune.fail-alloc If compiled with DEBUG_FAIL_ALLOC or started with "-dMfail", gives the percentage of chances an allocation attempt fails. Must be between 0 (no diff --git a/include/haproxy/defaults.h b/include/haproxy/defaults.h index edaf560df..7430c6150 100644 --- a/include/haproxy/defaults.h +++ b/include/haproxy/defaults.h @@ -277,6 +277,13 @@ // X-Original-To header default #define DEF_XORIGINALTO_HDR "X-Original-To" +/* Max number of events that may be processed at once by + * an event_hdl API consumer to prevent thread contention. + */ +#ifndef EVENT_HDL_MAX_AT_ONCE +#define EVENT_HDL_MAX_AT_ONCE 100 +#endif + /* Default connections limit. * * A system limit can be enforced at build time in order to avoid using haproxy diff --git a/include/haproxy/event_hdl-t.h b/include/haproxy/event_hdl-t.h index c9d90cb88..d4998521d 100644 --- a/include/haproxy/event_hdl-t.h +++ b/include/haproxy/event_hdl-t.h @@ -45,6 +45,11 @@ struct event_hdl_cb_data_template { } unsafe; }; +/* event_hdl tunables */ +struct event_hdl_tune { + unsigned int max_events_at_once; +}; + /* FIXME: adjust if needed! Should be large enough * to support every struct event_hdl_cb_data_x types * BUG_ON check in publish/async_mode and static assert diff --git a/include/haproxy/event_hdl.h b/include/haproxy/event_hdl.h index f174bbd5a..5a7ee6613 100644 --- a/include/haproxy/event_hdl.h +++ b/include/haproxy/event_hdl.h @@ -506,4 +506,7 @@ void event_hdl_sub_list_init(event_hdl_sub_list *sub_list); */ void event_hdl_sub_list_destroy(event_hdl_sub_list *sub_list); +/* event_hdl tunables */ +extern struct event_hdl_tune event_hdl_tune; + #endif /* _HAPROXY_EVENT_HDL_H */ diff --git a/src/event_hdl.c b/src/event_hdl.c index 5d395c2ea..aeb4d2499 100644 --- a/src/event_hdl.c +++ b/src/event_hdl.c @@ -18,6 +18,7 @@ #include #include #include +#include /* event types changes in event_hdl-t.h file should be reflected in the * map below to allow string to type and type to string conversions @@ -48,10 +49,8 @@ DECLARE_STATIC_POOL(pool_head_sub_event, "ehdl_sub_e", sizeof(struct event_hdl_a DECLARE_STATIC_POOL(pool_head_sub_event_data, "ehdl_sub_ed", sizeof(struct event_hdl_async_event_data)); DECLARE_STATIC_POOL(pool_head_sub_taskctx, "ehdl_sub_tctx", sizeof(struct event_hdl_async_task_default_ctx)); -/* TODO: will become a config tunable - * ie: tune.events.max-async-notif-at-once - */ -static int event_hdl_async_max_notif_at_once = 10; +/* global event_hdl tunables (public variable) */ +struct event_hdl_tune event_hdl_tune; /* global subscription list (implicit where NULL is used as sublist argument) */ static event_hdl_sub_list global_event_hdl_sub_list; @@ -81,6 +80,9 @@ static void event_hdl_init(void) event_hdl_sub_list_init(&global_event_hdl_sub_list); /* register the deinit function, will be called on soft-stop */ signal_register_fct(0, event_hdl_deinit, 0); + + /* set some default values */ + event_hdl_tune.max_events_at_once = EVENT_HDL_MAX_AT_ONCE; } /* general purpose hashing function when you want to compute @@ -242,7 +244,7 @@ static struct task *event_hdl_async_task_default(struct task *task, void *ctx, u * no more events to come (handler is unregistered) * so we must free task_ctx and stop task */ - while (max_notif_at_once_it < event_hdl_async_max_notif_at_once && + while (max_notif_at_once_it < event_hdl_tune.max_events_at_once && (event = event_hdl_async_equeue_pop(&task_ctx->e_queue))) { if (event_hdl_sub_type_equal(event->type, EVENT_HDL_SUB_END)) { @@ -964,4 +966,34 @@ void event_hdl_sub_list_destroy(event_hdl_sub_list *sub_list) _event_hdl_sub_list_destroy(sub_list); } +/* config parser for global "tune.events.max-events-at-once" */ +static int event_hdl_parse_max_events_at_once(char **args, int section_type, struct proxy *curpx, + const struct proxy *defpx, const char *file, int line, + char **err) +{ + int arg = -1; + + if (too_many_args(1, args, err, NULL)) + return -1; + + if (*(args[1]) != 0) + arg = atoi(args[1]); + + if (arg < 1 || arg > 10000) { + memprintf(err, "'%s' expects an integer argument between 1 and 10000.", args[0]); + return -1; + } + + event_hdl_tune.max_events_at_once = arg; + return 0; +} + +/* config keyword parsers */ +static struct cfg_kw_list cfg_kws = {ILH, { + { CFG_GLOBAL, "tune.events.max-events-at-once", event_hdl_parse_max_events_at_once }, + { 0, NULL, NULL } +}}; + +INITCALL1(STG_REGISTER, cfg_register_keywords, &cfg_kws); + INITCALL0(STG_INIT, event_hdl_init);