BUG/MINOR: threads: properly set the number of tgroups when non using policy

When nbthread is set, the CPU policies are not used and do not set
nbthread nor nbtgroups. When back into thread_detect_count(), these
are set respectively to thr_max and 1. The problem which becomes very
visible with max-threads-per-group, is that setting this one in
combination with nbthreads results in only one group with the calculated
number of threads per group. And there's not even a warning. So basically
a configuration having:

   global
       nbthread 64
       max-threads-per-group 8

would only start 8 threads.

In this case, grp_min remains valid and should be used, so let's just
change the assignment so that the number of groups is always correct.

A few ifdefs had to move because the calculations were only made for
the USE_CPU_AFFINITY case. Now these parts have been refined so that
all the logic continues to apply even without USE_CPU_AFFINITY.

One visible side effect is that setting nbthread above 64 will
automatically create the associated number of groups even when
USE_CPU_AFFINITY is not set. Previously it was silently changed
to match the per-group limit.

Ideally this should be backported to 3.2 where the issue was
introduced, though it may change the behavior of configs that were
silently being ignored (e.g. "nbthread 128"), so the backport should
be considered with care. At least 3.3 should have it because it uses
cpu-policy by default so it's only for failing cases that it would be
involved.

diff --git a/src/thread.c b/src/thread.c
index d44be167481941c6a0556e83983f291ae169f1f0..3aa4777821d6f122db483d9c9f2033819d6d6e3d 100644 (file)
--- a/src/thread.c
+++ b/src/thread.c
@@ -1691,12 +1691,16 @@ void thread_detect_count(void)
        if (global.thread_limit && thr_max > global.thread_limit)
                thr_max = global.thread_limit;
 
+       /* In case we can't count CPUs, just use the declared number of threads */
+       cpus_avail = thr_max;
+
 #if defined(USE_THREAD) && defined(USE_CPU_AFFINITY)
        /* consider the number of online CPUs as an upper limit if set */
        cpus_avail = 0;
        for (cpu = 0; cpu <= cpu_topo_lastcpu; cpu++)
                if (!(ha_cpu_topo[cpu].st & HA_CPU_F_OFFLINE))
                        cpus_avail++;
+#endif
 
        if (thr_min <= cpus_avail && cpus_avail < thr_max)
                thr_max = cpus_avail;
@@ -1716,6 +1720,7 @@ void thread_detect_count(void)
        if (grp_max > thr_max && grp_min <= thr_max)
                grp_max = thr_max;
 
+#if defined(USE_THREAD) && defined(USE_CPU_AFFINITY)
        if (grp_min < grp_max && cpu_map_configured()) {
                /* if a cpu-map directive is set, we cannot reliably infer what
                 * CPUs will be used anymore, so we'll use the smallest permitted
@@ -1769,11 +1774,20 @@ void thread_detect_count(void)
        }
 #endif // USE_THREAD && USE_CPU_AFFINITY
 
+       /* Here, we have seveal possibilities:
+        *   - USE_CPU_AFFINITY is set and worked, global.nbthread &
+        *     global.nbtgroups are properly set.
+        *   - USE_CPU_AFFINITY is set and failed (e.g. nbthread forced),
+        *     global.nbthread & global.nbtgroups are filled with pre-computed
+        *     optimal values
+        *   - USE_CPU_AFFINITY is not set: nbthread & nbtgroups are filled
+        *     with pre-computed optimal values
+        */
        if (!global.nbthread)
                global.nbthread = thr_max;
 
        if (!global.nbtgroups)
-               global.nbtgroups = 1;
+               global.nbtgroups = grp_min;
 
        if (global.nbthread > global.maxthrpertgroup * global.nbtgroups) {
                ha_diag_warning("nbthread too large or not set, found %d CPUs, limiting to %d threads (maximum is %d per thread group and %d groups). Please set nbthreads and/or increase thread-groups in the global section to silence this warning.\n",