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.

diff --git a/doc/configuration.txt b/doc/configuration.txt
index 53414dea876ec778bc29868583712f69302c6ccd..8b7f4aa19b127351b3ce0d4f3d650f5aa844c943 100644 (file)
--- 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 <number>
+  Sets the number of events that may be processed at once by an asynchronous
+  task handler (from event_hdl API). <number> 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 edaf560df5a264b3c164f867ebb40f159a10f3a2..7430c6150e49da2d42e3c10a4a3e7f31cf2c6e9f 100644 (file)
--- 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 c9d90cb88e100f029d2b9f45020e83e7cd66dac7..d4998521d7a4472e8af153c6bcbe02a509d68c46 100644 (file)
--- 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 f174bbd5a8d886708c86bd5869472753f65fdec6..5a7ee661350eb473771430ffe53c5df96e0805b8 100644 (file)
--- 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 5d395c2ea49fc31eaf48aa0984afd45017f9cb6f..aeb4d2499538ae85ecadf76039aaa795e40f9a99 100644 (file)
--- a/src/event_hdl.c
+++ b/src/event_hdl.c
@@ -18,6 +18,7 @@
 #include <haproxy/errors.h>
 #include <haproxy/signal.h>
 #include <haproxy/xxhash.h>
+#include <haproxy/cfgparse.h>
 
 /* 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);