#50312 Ticket 50306 - Move connection config inside struct
Closed 3 years ago by spichugi. Opened 5 years ago by mreynolds.
mreynolds/389-ds-base ticket50306  into  master

@@ -6,20 +6,16 @@ 

  import pytest

  import signal

  import threading

- from lib389 import DirSrv

  from lib389.tools import DirSrvTools

  from lib389._constants import *

  from lib389.properties import *

  from lib389.tasks import *

  from lib389.utils import *

  from lib389.idm.directorymanager import DirectoryManager

+ from lib389.idm.user import UserAccounts

+ from lib389.topologies import topology_st

  

- DEBUGGING = os.getenv('DEBUGGING', default=False)

- 

- if DEBUGGING:

-     logging.getLogger(__name__).setLevel(logging.DEBUG)

- else:

-     logging.getLogger(__name__).setLevel(logging.INFO)

+ logging.getLogger(__name__).setLevel(logging.DEBUG)

  log = logging.getLogger(__name__)

  

  MAX_CONNS = 10000000
@@ -27,45 +23,7 @@ 

  STOP = False

  HOSTNAME = DirSrvTools.getLocalhost()

  PORT = 389

- 

- 

- class TopologyStandalone(object):

-     """The DS Topology Class"""

-     def __init__(self, standalone):

-         """Init"""

-         standalone.open()

-         self.standalone = standalone

- 

- 

- @pytest.fixture(scope="module")

- def topology(request):

-     """Create DS Deployment"""

- 

-     # Creating standalone instance ...

-     standalone = DirSrv(verbose=DEBUGGING)

-     args_instance[SER_HOST] = HOST_STANDALONE

-     args_instance[SER_PORT] = PORT_STANDALONE

-     args_instance[SER_SECURE_PORT] = SECUREPORT_STANDALONE

-     args_instance[SER_SERVERID_PROP] = SERVERID_STANDALONE

-     args_instance[SER_CREATION_SUFFIX] = DEFAULT_SUFFIX

-     args_standalone = args_instance.copy()

-     standalone.allocate(args_standalone)

-     instance_standalone = standalone.exists()

-     if instance_standalone:

-         standalone.delete()

-     standalone.create()

-     standalone.open()

- 

-     def fin():

-         """If we are debugging just stop the instances, otherwise remove them

-         """

-         if DEBUGGING:

-             standalone.stop()

-         else:

-             standalone.delete()

-     request.addfinalizer(fin)

- 

-     return TopologyStandalone(standalone)

+ NUNC_STANS = False

  

  

  def signalHandler(signal, frame):
@@ -81,35 +39,15 @@ 

      """Set the idle timeout, and add sample entries

      """

  

-     try:

-         inst.modify_s(DN_CONFIG, [(ldap.MOD_REPLACE,

-                                    'nsslapd-idletimeout',

-                                    '5')])

-     except ldap.LDAPError as e:

-         log.fatal('Failed to set idletimeout: ' + str(e))

-         assert False

- 

-     try:

-         inst.modify_s(DN_CONFIG, [(ldap.MOD_REPLACE,

-                                    'nsslapd-enable-nunc-stans',

-                                    'on')])

-     except ldap.LDAPError as e:

-         log.fatal('Failed to enable nunc-stans: ' + str(e))

-         assert False

+     inst.config.set('nsslapd-idletimeout', '5')

+     if NUNC_STANS:

+         inst.config.set('nsslapd-enable-nunc-stans', 'on')

+         inst.restart()

  

+     users = UserAccounts(inst, DEFAULT_SUFFIX)

      for idx in range(0, 9):

-         user_dn = 'uid=entry%d,%s' % (idx, DEFAULT_SUFFIX)

-         try:

-             inst.add_s(Entry((user_dn,

-                 {'objectclass': ['top', 'extensibleObject'],

-                  'uid': 'entry%d' % idx,

-                  'cn': 'entry%d' % idx,

-                  'userpassword': 'password'})))

-         except ldap.LDAPError as e:

-             log.fatal('Failed to add user entry (%s): %s' % (user_dn, str(e)))

-             assert False

- 

-     inst.restart()

+         user = users.create_test_user(uid=str(idx), gid=str(idx))

+         user.reset_password('password')

  

  

  class BindOnlyConn(threading.Thread):
@@ -146,7 +84,7 @@ 

      """This class opens and closes connections

      """

      def __init__(self, inst):

-         """Initialize the thread class withte server isntance info"""

+         """Initialize the thread class with the server instance info"""

          threading.Thread.__init__(self)

          self.daemon = True

          self.inst = inst
@@ -160,7 +98,7 @@ 

          while idx < (MAX_CONNS / 10) and not STOP:

              try:

                  conn = self.inst.clone()

-                 conn.simple_bind_s('uid=entry0,dc=example,dc=com', 'password')

+                 conn.simple_bind_s('uid=test_user_0,dc=example,dc=com', 'password')

                  conn.search_s('dc=example,dc=com', ldap.SCOPE_SUBTREE,

                                'uid=*')

                  time.sleep(10)
@@ -217,7 +155,7 @@ 

              idx += 1

  

  

- def test_connection_load(topology):

+ def test_connection_load(topology_st):

      """Send the server a variety of connections using many threads:

          - Open, Bind, Close

          - Open, Bind, Search, wait to trigger idletimeout, Search, Close
@@ -229,7 +167,7 @@ 

  

      # Set the config and add sample entries

      log.info('Initializing setup...')

-     init(topology.standalone)

+     init(topology_st.standalone)

  

      #

      # Bind/Unbind Conn Threads
@@ -238,7 +176,7 @@ 

      threads = []

      idx = 0

      while idx < MAX_THREADS:

-         threads.append(BindOnlyConn(topology.standalone))

+         threads.append(BindOnlyConn(topology_st.standalone))

          idx += 1

      for thread in threads:

          thread.start()
@@ -251,7 +189,7 @@ 

      idx = 0

      idle_threads = []

      while idx < MAX_THREADS:

-         idle_threads.append(IdleConn(topology.standalone))

+         idle_threads.append(IdleConn(topology_st.standalone))

          idx += 1

      for thread in idle_threads:

          thread.start()
@@ -264,7 +202,7 @@ 

      idx = 0

      long_threads = []

      while idx < MAX_THREADS:

-         long_threads.append(LongConn(topology.standalone))

+         long_threads.append(LongConn(topology_st.standalone))

          idx += 1

      for thread in long_threads:

          thread.start()
@@ -285,4 +223,3 @@ 

      # -s for DEBUG mode

      CURRENT_FILE = os.path.realpath(__file__)

      pytest.main("-s %s" % CURRENT_FILE)

- 

@@ -15,7 +15,7 @@ 

  import threading

  

  import ldap

- from lib389 import DirSrv, Entry

+ from lib389 import Entry

  from lib389._constants import *

  from lib389.properties import *

  from lib389.plugins import ReferentialIntegrityPlugin, MemberOfPlugin

file modified
+12 -14
@@ -518,11 +518,11 @@ 

  static void

  connection_dispatch_operation(Connection *conn, Operation *op, Slapi_PBlock *pb)

  {

-     int minssf = config_get_minssf();

-     int minssf_exclude_rootdse = 0;

+     int32_t minssf = conn->c_minssf;

+     int32_t minssf_exclude_rootdse = conn->c_minssf_exclude_rootdse;

  #ifdef TCP_CORK

-     int enable_nagle = config_get_nagle();

-     int pop_cork = 0;

+     int32_t enable_nagle = conn->c_enable_nagle;

+     int32_t pop_cork = 0;

  #endif

  

      /* Set the connid and op_id to be used by internal op logging */
@@ -552,7 +552,6 @@ 

       * next step and check if the operation is against rootdse or not.

       * Once found it's not on rootdse, return LDAP_UNWILLING_TO_PERFORM there.

       */

-     minssf_exclude_rootdse = config_get_minssf_exclude_rootdse();

      if (!minssf_exclude_rootdse &&

          (conn->c_sasl_ssf < minssf) && (conn->c_ssl_ssf < minssf) &&

          (conn->c_local_ssf < minssf) && (op->o_tag != LDAP_REQ_BIND) &&
@@ -580,10 +579,10 @@ 

       * search. */

      if ((slapi_sdn_get_dn(&(op->o_sdn)) == NULL) &&

          /* anon access off and something other than BIND, EXTOP, UNBIND or ABANDON */

-         (((config_get_anon_access_switch() == SLAPD_ANON_ACCESS_OFF) && (op->o_tag != LDAP_REQ_BIND) &&

+         (((conn->c_anon_access == SLAPD_ANON_ACCESS_OFF) && (op->o_tag != LDAP_REQ_BIND) &&

            (op->o_tag != LDAP_REQ_EXTENDED) && (op->o_tag != LDAP_REQ_UNBIND) && (op->o_tag != LDAP_REQ_ABANDON)) ||

           /* root DSE access only and something other than BIND, EXTOP, UNBIND, ABANDON, or SEARCH */

-          ((config_get_anon_access_switch() == SLAPD_ANON_ACCESS_ROOTDSE) && (op->o_tag != LDAP_REQ_BIND) &&

+          ((conn->c_anon_access == SLAPD_ANON_ACCESS_ROOTDSE) && (op->o_tag != LDAP_REQ_BIND) &&

            (op->o_tag != LDAP_REQ_EXTENDED) && (op->o_tag != LDAP_REQ_UNBIND) &&

            (op->o_tag != LDAP_REQ_ABANDON) && (op->o_tag != LDAP_REQ_SEARCH)))) {

          slapi_log_access(LDAP_DEBUG_STATS,
@@ -1053,7 +1052,7 @@ 

      if ((LBER_OVERFLOW == *tagp || LBER_DEFAULT == *tagp) && 0 == bytes_scanned &&

          !SLAPD_SYSTEM_WOULD_BLOCK_ERROR(errno)) {

          if ((LBER_OVERFLOW == *tagp) || (errno == ERANGE)) {

-             ber_len_t maxbersize = config_get_maxbersize();

+             ber_len_t maxbersize = conn->c_maxbersize;

              ber_len_t tmplen = 0;

              (void)_ber_get_len(ber, &tmplen);

              /* openldap does not differentiate between length == 0
@@ -1176,7 +1175,7 @@ 

      }

      /* If we still haven't seen a complete PDU, read from the network */

      while (*tag == LBER_DEFAULT) {

-         int ioblocktimeout_waits = config_get_ioblocktimeout() / CONN_TURBO_TIMEOUT_INTERVAL;

+         int32_t ioblocktimeout_waits = conn->c_maxbersize / CONN_TURBO_TIMEOUT_INTERVAL;

          /* We should never get here with data remaining in the buffer */

          PR_ASSERT(!new_operation || !conn_buffered_data_avail_nolock(conn, &conn_closed));

          /* We make a non-blocking read call */
@@ -1624,7 +1623,7 @@ 

              g_decr_active_threadcnt();

              return;

          }

-         maxthreads = config_get_maxthreadsperconn();

+         maxthreads = conn->c_max_threads_per_conn;

          more_data = 0;

          ret = connection_read_operation(conn, op, &tag, &more_data);

          if ((ret == CONN_DONE) || (ret == CONN_TIMEDOUT)) {
@@ -2195,9 +2194,8 @@ 

  static int

  is_ber_too_big(const Connection *conn, ber_len_t ber_len)

  {

-     ber_len_t maxbersize = config_get_maxbersize();

-     if (ber_len > maxbersize) {

-         log_ber_too_big_error(conn, ber_len, maxbersize);

+     if (ber_len > conn->c_maxbersize) {

+         log_ber_too_big_error(conn, ber_len, conn->c_maxbersize);

          return 1;

      }

      return 0;
@@ -2213,7 +2211,7 @@ 

  log_ber_too_big_error(const Connection *conn, ber_len_t ber_len, ber_len_t maxbersize)

  {

      if (0 == maxbersize) {

-         maxbersize = config_get_maxbersize();

+         maxbersize = conn->c_maxbersize;

      }

      if (0 == ber_len) {

          slapi_log_err(SLAPI_LOG_ERR, "log_ber_too_big_error",

file modified
+15 -9
@@ -1368,7 +1368,6 @@ 

      static int last_accept_new_connections = -1;

      PRIntn count = 0;

      slapdFrontendConfig_t *slapdFrontendConfig = getFrontendConfig();

-     int max_threads_per_conn = config_get_maxthreadsperconn();

      size_t n_listeners = 0;

  

      accept_new_connections = ((ct->size - g_get_current_conn_count()) > slapdFrontendConfig->reservedescriptors);
@@ -1512,7 +1511,7 @@ 

              } else if (c->c_sd == SLAPD_INVALID_SOCKET) {

                  connection_table_move_connection_out_of_active_list(ct, c);

              } else if (c->c_prfd != NULL) {

-                 if ((!c->c_gettingber) && (c->c_threadnumber < max_threads_per_conn)) {

+                 if ((!c->c_gettingber) && (c->c_threadnumber < c->c_max_threads_per_conn)) {

                      int add_fd = 1;

                      /* check timeout for PAGED RESULTS */

                      if (pagedresults_is_timedout_nolock(c)) {
@@ -1533,7 +1532,7 @@ 

                          count++;

                      }

                  } else {

-                     if (c->c_threadnumber >= max_threads_per_conn) {

+                     if (c->c_threadnumber >= c->c_max_threads_per_conn) {

                          c->c_maxthreadsblocked++;

                      }

                      c->c_fdi = SLAPD_INVALID_SOCKET_INDEX;
@@ -1566,7 +1565,6 @@ 

  {

      Connection *c;

      time_t curtime = slapi_current_utc_time();

-     int maxthreads = config_get_maxthreadsperconn();

  

  #if LDAP_ERROR_LOGGING

      if (slapd_ldap_debug & LDAP_DEBUG_CONNS) {
@@ -1615,7 +1613,7 @@ 

  

                      /* This is where the work happens ! */

                      /* MAB: 25 jan 01, error handling added */

-                     if ((connection_activity(c, maxthreads)) == -1) {

+                     if ((connection_activity(c, c->c_max_threads_per_conn)) == -1) {

                          /* This might happen as a result of

                           * trying to acquire a closing connection

                           */
@@ -1855,7 +1853,6 @@ 

  void

  ns_handle_pr_read_ready(struct ns_job_t *job)

  {

-     int maxthreads = config_get_maxthreadsperconn();

      Connection *c = (Connection *)ns_job_get_data(job);

  

      PR_EnterMonitor(c->c_mutex);
@@ -1906,7 +1903,7 @@ 

              slapi_log_err(SLAPI_LOG_WARNING, "ns_handle_pr_read_ready", "Received idletime out with c->c_idletimeout as 0. Ignoring.\n");

          }

          ns_handle_closure_nomutex(c);

-     } else if ((connection_activity(c, maxthreads)) == -1) {

+     } else if ((connection_activity(c, c->c_max_threads_per_conn)) == -1) {

          /* This might happen as a result of

           * trying to acquire a closing connection

           */
@@ -2384,8 +2381,8 @@ 

      /*    struct sockaddr_in    from;*/

      PRNetAddr from = {{0}};

      PRFileDesc *pr_clonefd = NULL;

-     ber_len_t maxbersize;

      slapdFrontendConfig_t *fecfg = getFrontendConfig();

+     int32_t maxbersize;

  

      if (newconn) {

          *newconn = NULL;
@@ -2413,6 +2410,15 @@ 

      conn->c_prfd = pr_clonefd;

      conn->c_flags &= ~CONN_FLAG_CLOSING;

  

+     /* Set per connection static config */

+     conn->c_maxbersize = config_get_maxbersize();

+     conn->c_ioblocktimeout = config_get_ioblocktimeout();

+     conn->c_minssf = config_get_minssf();

+     conn->c_enable_nagle = config_get_nagle();

+     conn->c_minssf_exclude_rootdse = config_get_minssf_exclude_rootdse();

+     conn->c_anon_access = config_get_anon_access_switch();

+     conn->c_max_threads_per_conn = config_get_maxthreadsperconn();

+ 

      /* Store the fact that this new connection is an SSL connection */

      if (secure) {

          conn->c_flags |= CONN_FLAG_SSL;
@@ -2445,7 +2451,7 @@ 

                                 LBER_SOCKBUF_OPT_EXT_IO_FNS, &func_pointers);

      }

  #endif /* !USE_OPENLDAP */

-     maxbersize = config_get_maxbersize();

+     maxbersize = conn->c_maxbersize;

  #if defined(USE_OPENLDAP)

      ber_sockbuf_ctrl(conn->c_sb, LBER_SB_OPT_SET_MAX_INCOMING, &maxbersize);

  #endif

file modified
+14 -6
@@ -1658,12 +1658,20 @@ 

      /* IO layer push/pop */

      Conn_IO_Layer_cb c_push_io_layer_cb; /* callback to push an IO layer on the conn->c_prfd */

      Conn_IO_Layer_cb c_pop_io_layer_cb;  /* callback to pop an IO layer off of the conn->c_prfd */

-     void *c_io_layer_cb_data;            /* callback data */

-     struct connection_table *c_ct;       /* connection table that this connection belongs to */

-     ns_thrpool_t *c_tp;                  /* thread pool for this connection */

-     struct ns_job_t *c_job;              /* If it exists, the current ns_job_t */

-     int c_ns_close_jobs;                 /* number of current close jobs */

-     char *c_ipaddr;                      /* ip address str - used by monitor */

+     void *c_io_layer_cb_data;        /* callback data */

+     struct connection_table *c_ct;   /* connection table that this connection belongs to */

+     ns_thrpool_t *c_tp;              /* thread pool for this connection */

+     struct ns_job_t *c_job;          /* If it exists, the current ns_job_t */

+     int c_ns_close_jobs;             /* number of current close jobs */

+     char *c_ipaddr;                  /* ip address str - used by monitor */

+     /* per conn static config */

+     ber_len_t c_maxbersize;

+     int32_t c_ioblocktimeout;

+     int32_t c_minssf;

+     int32_t c_enable_nagle;

+     int32_t c_minssf_exclude_rootdse;

+     int32_t c_anon_access;

+     int32_t c_max_threads_per_conn;

  } Connection;

  #define CONN_FLAG_SSL 1     /* Is this connection an SSL connection or not ?         \

                             * Used to direct I/O code when SSL is handled differently \

Description:

         We are constantly calling configuration get functions
         during a connection. These calls are expensive, so we
         should just store all these settings in the conn struct
         during handle_new_connection()

CI Tested & ASAN Approved

https://pagure.io/389-ds-base/issue/50306

This looks great! I really like the idea of it. I don't see anything obvious either, so ack from me?

rebased onto c9d6528

4 years ago

Pull-Request has been merged by mreynolds

4 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/3371

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

3 years ago