#51173 Issue 49256 - log warning when thread number is very different from autotuned value
Closed 2 years ago by spichugi. Opened 2 years ago by mreynolds.
mreynolds/389-ds-base issue49256  into  master

@@ -43,6 +43,34 @@ 

      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

file modified
+33 -1
@@ -4374,6 +4374,7 @@ 

  {

      int retVal = LDAP_SUCCESS;

      int32_t threadnum = 0;

+     int32_t hw_threadnum = 0;

      char *endp = NULL;

  

      slapdFrontendConfig_t *slapdFrontendConfig = getFrontendConfig();
@@ -4386,8 +4387,39 @@ 

      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) {

@@ -403,6 +403,9 @@ 

  #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. */

  

@@ -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>

  

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

I agreed, good to warn about this. Ack

rebased onto e7e69bd8c3cc8434e1957b0e871551a26bab9b6e

2 years ago

rebased onto a64d6be

2 years ago

Pull-Request has been merged by mreynolds

2 years ago

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This pull request has been cloned to Github as issue and is available here:
- https://github.com/389ds/389-ds-base/issues/4226

If you want to continue to work on the PR, please navigate to the github issue,
download the patch from the attachments and file a new pull request.

Thank you for understanding. We apologize for all inconvenience.

Pull-Request has been closed by spichugi

2 years ago