Issue 49256 - log warning when thread number is very different from a…

…utotuned value

Description:  To help prevent customers from setting incorrect values for
              the thread number it would be useful to warn them that the
              configured value is either way too low or way too high.

relates: https://pagure.io/389-ds-base/issue/49256

Reviewed by: firstyear(Thanks!)

dirsrvtests/tests/suites/config/autotuning_test.py

@@ -43,6 +43,34 @@ def test_threads_basic(topo):
     assert topo.standalone.config.get_attr_val_int("nsslapd-threadnumber") > 0
 
 
+def test_threads_warning(topo):
+    """Check that we log a warning if the thread number is too high or low
+
+    :id: db92412b-2812-49de-84b0-00f452cd254f
+    :setup: Standalone Instance
+    :steps:
+        1. Get autotuned thread number
+        2. Set threads way higher than hw threads, and find a warning in the log
+        3. Set threads way lower than hw threads, and find a warning in the log
+    :expectedresults:
+        1. Success
+        2. Success
+        3. Success
+    """
+    topo.standalone.config.set("nsslapd-threadnumber", "-1")
+    autotuned_value = topo.standalone.config.get_attr_val_utf8("nsslapd-threadnumber")
+
+    topo.standalone.config.set("nsslapd-threadnumber", str(int(autotuned_value) * 4))
+    time.sleep(.5)
+    assert topo.standalone.ds_error_log.match('.*higher.*hurt server performance.*')
+
+    if int(autotuned_value) > 1:
+        # If autotuned is 1, there isn't anything to test here
+        topo.standalone.config.set("nsslapd-threadnumber", "1")
+        time.sleep(.5)
+        assert topo.standalone.ds_error_log.match('.*lower.*hurt server performance.*')
+
+
 @pytest.mark.parametrize("invalid_value", ('-2', '0', 'invalid'))
 def test_threads_invalid_value(topo, invalid_value):
     """Check nsslapd-threadnumber for an invalid values

ldap/servers/slapd/libglobs.c

@@ -4374,6 +4374,7 @@ config_set_threadnumber(const char *attrname, char *value, char *errorbuf, int a
 {
     int retVal = LDAP_SUCCESS;
     int32_t threadnum = 0;
+    int32_t hw_threadnum = 0;
     char *endp = NULL;
 
     slapdFrontendConfig_t *slapdFrontendConfig = getFrontendConfig();
@@ -4386,8 +4387,39 @@ config_set_threadnumber(const char *attrname, char *value, char *errorbuf, int a
     threadnum = strtol(value, &endp, 10);
 
     /* Means we want to re-run the hardware detection. */
+    hw_threadnum = util_get_hardware_threads();
     if (threadnum == -1) {
-        threadnum = util_get_hardware_threads();
+        threadnum = hw_threadnum;
+    } else {
+        /*
+         * Log a message if the user defined thread number is very different
+         * from the hardware threads as this is probably not the optimal
+         * value.
+         */
+        if (threadnum >= hw_threadnum) {
+            if (threadnum > MIN_THREADS && threadnum / hw_threadnum >= 4) {
+                /* We're over the default minimum and way higher than the hw
+                 * threads. */
+                slapi_log_err(SLAPI_LOG_NOTICE, "config_set_threadnumber",
+                        "The configured thread number (%d) is significantly "
+                        "higher than the number of hardware threads (%d).  "
+                        "This can potentially hurt server performance.  If "
+                        "you are unsure how to tune \"nsslapd-threadnumber\" "
+                        "then set it to \"-1\" and the server will tune it "
+                        "according to the system hardware\n",
+                        threadnum, hw_threadnum);
+            }
+        } else if (threadnum < MIN_THREADS) {
+            /* The thread number should never be less than the minimum and
+             * hardware threads. */
+            slapi_log_err(SLAPI_LOG_WARNING, "config_set_threadnumber",
+                    "The configured thread number (%d) is lower than the number "
+                    "of hardware threads (%d).  This will hurt server performance.  "
+                    "If you are unsure how to tune \"nsslapd-threadnumber\" then "
+                    "set it to \"-1\" and the server will tune it according to the "
+                    "system hardware\n",
+                    threadnum, hw_threadnum);
+            }
     }
 
     if (*endp != '\0' || errno == ERANGE || threadnum < 1 || threadnum > 65535) {

ldap/servers/slapd/slap.h

@@ -403,6 +403,9 @@ typedef void (*VFPV)(); /* takes undefined arguments */
 #define SLAPD_DEFAULT_PW_MAX_CLASS_CHARS_ATTRIBUTE 0
 #define SLAPD_DEFAULT_PW_MAX_CLASS_CHARS_ATTRIBUTE_STR "0"
 
+#define MIN_THREADS 16
+#define MAX_THREADS 512
+
 
 /* Default password values. */
 

ldap/servers/slapd/util.c

@@ -41,9 +41,6 @@
 #define FILTER_BUF 128 /* initial buffer size for attr value */
 #define BUF_INCR 16    /* the amount to increase the FILTER_BUF once it fills up */
 
-#define MIN_THREADS 16
-#define MAX_THREADS 512
-
 /* slapi-private contains the pal. */
 #include <slapi-private.h>