From a64d6be1a11eea4129e0c02f07fa79aa9bfa0a38 Mon Sep 17 00:00:00 2001
From: Mark Reynolds <mreynolds@redhat.com>
Date: Jun 23 2020 01:27:01 +0000
Subject: Issue 49256 - log warning when thread number is very different from autotuned 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!)

---

diff --git a/dirsrvtests/tests/suites/config/autotuning_test.py b/dirsrvtests/tests/suites/config/autotuning_test.py
index d1c7514..5407612 100644
--- a/dirsrvtests/tests/suites/config/autotuning_test.py
+++ b/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
diff --git a/ldap/servers/slapd/libglobs.c b/ldap/servers/slapd/libglobs.c
index d540d84..97af7bb 100644
--- a/ldap/servers/slapd/libglobs.c
+++ b/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) {
diff --git a/ldap/servers/slapd/slap.h b/ldap/servers/slapd/slap.h
index cef8c78..7e0fa61 100644
--- a/ldap/servers/slapd/slap.h
+++ b/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. */
 
diff --git a/ldap/servers/slapd/util.c b/ldap/servers/slapd/util.c
index fda4fdf..7d0dc07 100644
--- a/ldap/servers/slapd/util.c
+++ b/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>