Issue 6374 - nsslapd-mdb-max-dbs autotuning doesn't work properly (#6400

)

* Issue 6374 - nsslapd-mdb-max-dbs autotuning doesn't work properly

Several issues:

After restarting the server nsslapd-mdb-max-dbs may not be high enough to add a new backend
because the value computation is wrong.
dbscan fails to open the database if nsslapd-mdb-max-dbs has been increased.
dbscan crashes when closing the database (typically when using -S)
When starting the instance the nsslapd-mdb-max-dbs parameter is increased to ensure that a new backend may be added.
When dse.ldif path is not specified, the db environment is now open using the INFO.mdb data instead of using the default values.
synchronization between thread closure and database context destruction is hardened
Issue: #6374

Reviewed by: @tbordaz , @vashirov (Thanks!)

dirsrvtests/tests/suites/config/config_test.py

@@ -17,6 +17,7 @@
 from lib389.utils import *
 from lib389._constants import DN_CONFIG, DEFAULT_SUFFIX, DEFAULT_BENAME
 from lib389._mapped_object import DSLdapObjects
+from lib389.agreement import Agreements
 from lib389.cli_base import FakeArgs
 from lib389.cli_conf.backend import db_config_set
 from lib389.idm.user import UserAccounts, TEST_USER_PROPERTIES
@@ -27,6 +28,8 @@
 from lib389.backend import Backends, DatabaseConfig
 from lib389.monitor import MonitorLDBM, Monitor
 from lib389.plugins import ReferentialIntegrityPlugin
+from lib389.replica import BootstrapReplicationManager, Replicas
+from lib389.passwd import password_generate
 
 pytestmark = pytest.mark.tier0
 
@@ -36,6 +39,8 @@
 logging.getLogger(__name__).setLevel(logging.INFO)
 log = logging.getLogger(__name__)
 
+DEBUGGING = os.getenv("DEBUGGING", default=False)
+
 @pytest.fixture(scope="module")
 def big_file():
     TEMP_BIG_FILE = ''
@@ -813,6 +818,87 @@ def test_numlisteners_limit(topo):
     assert numlisteners[0] == '4'
 
 
+def bootstrap_replication(inst_from, inst_to, creds):
+    manager = BootstrapReplicationManager(inst_to)
+    rdn_val = 'replication manager'
+    if  manager.exists():
+        manager.delete()
+    manager.create(properties={
+        'cn': rdn_val,
+        'uid': rdn_val,
+        'userPassword': creds
+    })
+    for replica in Replicas(inst_to).list():
+        replica.remove_all('nsDS5ReplicaBindDNGroup')
+        replica.replace('nsDS5ReplicaBindDN', manager.dn)
+    for agmt in Agreements(inst_from).list():
+        agmt.replace('nsDS5ReplicaBindDN', manager.dn)
+        agmt.replace('nsDS5ReplicaCredentials', creds)
+
+
+@pytest.mark.skipif(get_default_db_lib() != "mdb", reason="This test requires lmdb")
+def test_lmdb_autotuned_maxdbs(topology_m2, request):
+    """Verify that after restart, nsslapd-mdb-max-dbs is large enough to add a new backend.
+
+    :id: 0272d432-9080-11ef-8f40-482ae39447e5
+    :setup: Two suppliers configuration
+    :steps:
+        1. loop 20 times
+        3. In 1 loop: restart instance
+        3. In 1 loop: add a new backend
+        4. In 1 loop: check that instance is still alive
+    :expectedresults:
+        1. Success
+        2. Success
+        3. Success
+        4. Success
+    """
+
+    s1 = topology_m2.ms["supplier1"]
+    s2 = topology_m2.ms["supplier2"]
+
+    backends = Backends(s1)
+    db_config = DatabaseConfig(s1)
+    # Generate the teardown finalizer
+    belist = []
+    creds=password_generate()
+    bootstrap_replication(s2, s1, creds)
+    bootstrap_replication(s1, s2, creds)
+
+    def fin():
+        s1.start()
+        for be in belist:
+            be.delete()
+
+    if not DEBUGGING:
+        request.addfinalizer(fin)
+
+    # 1. Set autotuning (off-line to be able to decrease the value)
+    s1.stop()
+    dse_ldif = DSEldif(s1)
+    dse_ldif.replace(db_config.dn, 'nsslapd-mdb-max-dbs', '0')
+    os.remove(f'{s1.dbdir}/data.mdb')
+    s1.start()
+
+    # 2. Reinitialize the db:
+    log.info("Bulk import...")
+    agmt = Agreements(s2).list()[0]
+    agmt.begin_reinit()
+    (done, error) = agmt.wait_reinit()
+    log.info(f'Bulk importresult is ({done}, {error})')
+    assert done is True
+    assert error is False
+
+    # 3. loop 20 times
+    for idx in range(20):
+        s1.restart()
+        log.info(f'Adding backend test{idx}')
+        belist.append(backends.create(properties={'cn': f'test{idx}',
+                                     'nsslapd-suffix': f'dc=test{idx}'}))
+        assert s1.status()
+
+
+
 if __name__ == '__main__':
     # Run isolated
     # -s for DEBUG mode

ldap/servers/slapd/back-ldbm/back-ldbm.h

@@ -896,4 +896,6 @@ typedef struct _back_search_result_set
     ((L)->size == (R)->size && !memcmp((L)->data, (R)->data, (L)->size))
 
 typedef int backend_implement_init_fn(struct ldbminfo *li, config_info *config_array);
+
+pthread_mutex_t *get_import_ctx_mutex();
 #endif /* _back_ldbm_h_ */

ldap/servers/slapd/back-ldbm/db-mdb/mdb_config.c

@@ -83,7 +83,7 @@ dbmdb_compute_limits(struct ldbminfo *li)
     uint64_t total_space = 0;
     uint64_t avail_space = 0;
     uint64_t cur_dbsize = 0;
-    int nbchangelogs = 0;
+    int nbvlvs = 0;
     int nbsuffixes = 0;
     int nbindexes = 0;
     int nbagmt = 0;
@@ -99,8 +99,8 @@ dbmdb_compute_limits(struct ldbminfo *li)
      *  But some tunable may be autotuned.
      */
     if (dbmdb_count_config_entries("(objectClass=nsMappingTree)", &nbsuffixes) ||
-        dbmdb_count_config_entries("(objectClass=nsIndex)", &nbsuffixes) ||
-        dbmdb_count_config_entries("(&(objectClass=nsds5Replica)(nsDS5Flags=1))", &nbchangelogs) ||
+        dbmdb_count_config_entries("(objectClass=nsIndex)", &nbindexes) ||
+        dbmdb_count_config_entries("(objectClass=vlvIndex)", &nbvlvs) ||
         dbmdb_count_config_entries("(objectClass=nsds5replicationagreement)", &nbagmt)) {
         /* error message is already logged */
         return 1;
@@ -120,8 +120,15 @@ dbmdb_compute_limits(struct ldbminfo *li)
 
     info->pagesize = sysconf(_SC_PAGE_SIZE);
     limits->min_readers = config_get_threadnumber() + nbagmt + DBMDB_READERS_MARGIN;
-    /* Default indexes are counted in "nbindexes" so we should always have enough resource to add 1 new suffix */
-    limits->min_dbs = nbsuffixes + nbindexes + nbchangelogs + DBMDB_DBS_MARGIN;
+    /*
+     * For each suffix there are 4 databases instances:
+     *  long-entryrdn, replication_changelog, id2entry and ancestorid
+     * then the indexes and the vlv and vlv cache
+     *
+     * Default indexes are counted in "nbindexes" so we should always have enough
+     *  resource to add 1 new suffix
+     */
+    limits->min_dbs = 4*nbsuffixes + nbindexes + 2*nbvlvs + DBMDB_DBS_MARGIN;
 
     total_space = ((uint64_t)(buf.f_blocks)) * ((uint64_t)(buf.f_bsize));
     avail_space = ((uint64_t)(buf.f_bavail)) * ((uint64_t)(buf.f_bsize));

ldap/servers/slapd/back-ldbm/db-mdb/mdb_import_threads.c

@@ -4280,9 +4280,12 @@ dbmdb_import_init_writer(ImportJob *job, ImportRole_t role)
 void
 dbmdb_free_import_ctx(ImportJob *job)
 {
-    if (job->writer_ctx) {
-        ImportCtx_t *ctx = job->writer_ctx;
-        job->writer_ctx = NULL;
+    ImportCtx_t *ctx = NULL;
+    pthread_mutex_lock(get_import_ctx_mutex());
+    ctx = job->writer_ctx;
+    job->writer_ctx = NULL;
+    pthread_mutex_unlock(get_import_ctx_mutex());
+    if (ctx) {
         pthread_mutex_destroy(&ctx->workerq.mutex);
         pthread_cond_destroy(&ctx->workerq.cv);
         slapi_ch_free((void**)&ctx->workerq.slots);

ldap/servers/slapd/back-ldbm/db-mdb/mdb_instance.c

@@ -287,6 +287,13 @@ int add_dbi(dbi_open_ctx_t *octx, backend *be, const char *fname, int flags)
         slapi_ch_free((void**)&treekey.dbname);
         return octx->rc;
     }
+    if (treekey.dbi >= ctx->dsecfg.max_dbs) {
+        octx->rc = MDB_DBS_FULL;
+        slapi_log_err(SLAPI_LOG_ERR, "add_dbi", "Failed to open database instance %s slots: %d/%d. Error is %d: %s.\n",
+                      treekey.dbname, treekey.dbi, ctx->dsecfg.max_dbs, octx->rc, mdb_strerror(octx->rc));
+        slapi_ch_free((void**)&treekey.dbname);
+        return octx->rc;
+    }
     if (octx->ai && octx->ai->ai_key_cmp_fn) {
 		octx->rc = dbmdb_update_dbi_cmp_fn(ctx, &treekey, octx->ai->ai_key_cmp_fn, octx->txn);
         if (octx->rc) {
@@ -694,6 +701,7 @@ int dbmdb_make_env(dbmdb_ctx_t *ctx, int readOnly, mdb_mode_t mode)
         rc = dbmdb_write_infofile(ctx);
     } else {
         /* No Config ==> read it from info file */
+        ctx->dsecfg = ctx->startcfg;
     }
     if (rc) {
         return rc;

ldap/servers/slapd/back-ldbm/dbimpl.c

@@ -505,7 +505,7 @@ int dblayer_show_statistics(const char *dbimpl_name, const char *dbhome, FILE *f
     li->li_plugin = be->be_database;
     li->li_plugin->plg_name = (char*) "back-ldbm-dbimpl";
     li->li_plugin->plg_libpath = (char*) "libback-ldbm";
-    li->li_directory = (char*)dbhome;
+    li->li_directory = get_li_directory(dbhome);
 
     /* Initialize database plugin */
     rc = dbimpl_setup(li, dbimpl_name);

ldap/servers/slapd/back-ldbm/import.c

@@ -27,6 +27,9 @@
 #define NEED_DN_NORM_SP -25
 #define NEED_DN_NORM_BT -26
 
+/* Protect against import context destruction */
+static pthread_mutex_t import_ctx_mutex = PTHREAD_MUTEX_INITIALIZER;
+
 
 /********** routines to manipulate the entry fifo **********/
 
@@ -143,6 +146,14 @@ ldbm_back_wire_import(Slapi_PBlock *pb)
 
 /* Threads management */
 
+/* Return the mutex that protects against import context destruction */
+pthread_mutex_t *
+get_import_ctx_mutex()
+{
+    return &import_ctx_mutex;
+}
+
+
 /* tell all the threads to abort */
 void
 import_abort_all(ImportJob *job, int wait_for_them)
@@ -151,7 +162,7 @@ import_abort_all(ImportJob *job, int wait_for_them)
 
     /* tell all the worker threads to abort */
     job->flags |= FLAG_ABORT;
-
+    pthread_mutex_lock(&import_ctx_mutex);
     for (worker = job->worker_list; worker; worker = worker->next)
         worker->command = ABORT;
 
@@ -167,6 +178,7 @@ import_abort_all(ImportJob *job, int wait_for_them)
             }
         }
     }
+    pthread_mutex_unlock(&import_ctx_mutex);
 }