Issue 6064 - bdb2mdb shows errors (#6341)

Fix several issues with dsctl dblib migration tools:
Fix some problem related to dbscan -D bdb --import:

random crash when closing the database - Fixed by fully cleaning the bdb framework if open in read-write mode
random records were missing - Fixed by fully starting the bdb framework when it is open in read-write mode
and by using txn
Fixed error messages displayed when trying to reopen ldap connection on restarted instance by not reopening the connection that are not needed.
Fixed error messages displayed when there is no replication changelog by detecting the presence of replication changelog before trying to export it.
Issue: #6064

Reviewed by: @tbordaz (Thans!)

(cherry picked from commit c1b842e)

dirsrvtests/tests/suites/clu/dsctl_dblib_test.py

@@ -10,22 +10,28 @@
 import pytest
 import ldap
 import os
+import time
 from lib389._constants import DEFAULT_SUFFIX
 from lib389.backend import DatabaseConfig
 from lib389.cli_ctl.dblib import (FakeArgs, dblib_bdb2mdb, dblib_mdb2bdb, dblib_cleanup)
 from lib389.idm.user import UserAccounts
 from lib389.replica import ReplicationManager
-from lib389.topologies import topology_m2 as topo_m2
+from lib389.topologies import topology_m2 as topo_m2, topology_st as topo_st
 
 
 log = logging.getLogger(__name__)
 
 
-@pytest.fixture
-def init_user(topo_m2, request):
+@pytest.fixture(scope='function', params=['topo_st','topo_m2'])
+def init_user(topo_m2, topo_st, request):
     """Initialize a user - Delete and re-add test user
     """
-    s1 = topo_m2.ms["supplier1"]
+    if  request.param == 'topo_st':
+        s1 = topo_st.standalone
+        s2 = None
+    else:
+        s1 = topo_m2.ms["supplier1"]
+        s2 = topo_m2.ms["supplier2"]
     users = UserAccounts(s1, DEFAULT_SUFFIX)
     try:
         user_data = {'uid': 'test entry',
@@ -48,6 +54,7 @@ def fin():
             pass
 
     request.addfinalizer(fin)
+    return (s1, s2)
 
 
 def _check_db(inst, log, impl):
@@ -59,8 +66,10 @@ def _check_db(inst, log, impl):
     db_files = os.listdir(inst.dbdir)
     if inst.ds_paths.db_home_dir is not None and inst.ds_paths.db_home_dir != inst.dbdir:
         db_files.extend(os.listdir(inst.ds_paths.db_home_dir))
+    db_files = [ file for file in db_files if not file.startswith('log.00') ]
+
     mdb_list = ['data.mdb', 'INFO.mdb', 'lock.mdb']
-    bdb_list = ['__db.001', 'DBVERSION', '__db.003', 'userRoot', 'log.0000000001', '__db.002']
+    bdb_list = ['__db.001', 'DBVERSION', '__db.003', 'userRoot', '__db.002']
     mdb_list.sort()
     bdb_list.sort()
     db_files = sorted(set(db_files))
@@ -77,7 +86,7 @@ def _check_db(inst, log, impl):
         assert db_files == mdb_list
 
 
-def test_dblib_migration(topo_m2, init_user):
+def test_dblib_migration(init_user):
     """
     Verify dsctl dblib xxxxxxx sub commands (migration between bdb and lmdb)
 
@@ -92,28 +101,36 @@ def test_dblib_migration(topo_m2, init_user):
         2. Success
         3. Success
     """
-    s1 = topo_m2.ms["supplier1"]
-    s2 = topo_m2.ms["supplier2"]
+    s1, s2 = init_user
     db_lib = s1.get_db_lib()
-    repl = ReplicationManager(DEFAULT_SUFFIX)
+    if s2 is not None:
+        repl = ReplicationManager(DEFAULT_SUFFIX)
     users = UserAccounts(s1, DEFAULT_SUFFIX)
     assert users.get('test entry')
     args = FakeArgs({'tmpdir': None})
     if db_lib == 'bdb':
         dblib_bdb2mdb(s1, log, args)
+        s1.open()
         dblib_cleanup(s1, log, args)
         _check_db(s1, log, 'mdb')
-        repl.test_replication_topology([s1, s2])
+        if s2 is not None:
+            repl.test_replication_topology([s1, s2])
         dblib_mdb2bdb(s1, log, args)
+        s1.open()
         dblib_cleanup(s1, log, args)
         _check_db(s1, log, 'bdb')
-        repl.test_replication_topology([s1, s2])
+        if s2 is not None:
+            repl.test_replication_topology([s1, s2])
     else:
         dblib_mdb2bdb(s1, log, args)
+        s1.open()
         dblib_cleanup(s1, log, args)
         _check_db(s1, log, 'bdb')
-        repl.test_replication_topology([s1, s2])
+        if s2 is not None:
+            repl.test_replication_topology([s1, s2])
         dblib_bdb2mdb(s1, log, args)
+        s1.open()
         dblib_cleanup(s1, log, args)
         _check_db(s1, log, 'mdb')
-        repl.test_replication_topology([s1, s2])
+        if s2 is not None:
+            repl.test_replication_topology([s1, s2])

ldap/servers/slapd/back-ldbm/db-bdb/bdb_layer.c

@@ -6905,39 +6905,51 @@ bdb_public_private_open(backend *be, const char *db_filename, int rw, dbi_env_t
     int flags;
     int rc;
 
-    /* Either filename is an existing regular file
-     *  or the "home" directory where txn logs are
-     */
+    slapi_ch_free_string(&conf->bdb_dbhome_directory);
+    if (li->li_directory == NULL) {
+        /* Either filename is an existing regular file
+         *  or the "home" directory where txn logs are
+         */
 
-    PL_strncpyz(dbhome, db_filename, MAXPATHLEN);
-    if (stat(dbhome, &st) == 0) {
-        if (S_ISDIR(st.st_mode)) {
-            li->li_directory = slapi_ch_strdup(dbhome);
-        } else if (S_ISREG(st.st_mode)) {
+        PL_strncpyz(dbhome, db_filename, MAXPATHLEN);
+        if (stat(dbhome, &st) == 0) {
+            if (S_ISDIR(st.st_mode)) {
+                li->li_directory = slapi_ch_strdup(dbhome);
+            } else if (S_ISREG(st.st_mode)) {
+                getdir(dbhome, NULL);
+                li->li_directory = slapi_ch_strdup(db_filename);
+                getdir(dbhome, NULL);
+            } else {
+                fprintf(stderr, "bdb_public_private_open: Unable to determine dbhome from %s\n", db_filename);
+                return EINVAL;
+            }
+        } else {
             getdir(dbhome, NULL);
-            li->li_directory = slapi_ch_strdup(db_filename);
+            li->li_directory = slapi_ch_strdup(dbhome);
             getdir(dbhome, NULL);
-        } else {
-            fprintf(stderr, "bdb_public_private_open: Unable to determine dbhome from %s\n", db_filename);
-            return EINVAL;
+            if (stat(dbhome, &st) || ((st.st_mode & S_IFMT) != S_IFDIR)) {
+                fprintf(stderr, "bdb_public_private_open: Unable to determine dbhome from %s\n", db_filename);
+                return EINVAL;
+            }
         }
+        conf->bdb_dbhome_directory = slapi_ch_strdup(dbhome);
     } else {
-        getdir(dbhome, NULL);
-        li->li_directory = slapi_ch_strdup(dbhome);
-        getdir(dbhome, NULL);
-        if (stat(dbhome, &st) || ((st.st_mode & S_IFMT) != S_IFDIR)) {
-            fprintf(stderr, "bdb_public_private_open: Unable to determine dbhome from %s\n", db_filename);
-            return EINVAL;
+        conf->bdb_dbhome_directory = slapi_ch_strdup(li->li_directory);
+        if (strcmp(li->li_directory, db_filename)) {
+            getdir(conf->bdb_dbhome_directory, NULL);
         }
     }
+
     li->li_config_mutex = PR_NewLock();
-    conf->bdb_dbhome_directory = slapi_ch_strdup(dbhome);
     if (rw) {
         /* Setup a fully transacted environment */
         priv->dblayer_env = NULL;
-        conf->bdb_enable_transactions = 0;
+        conf->bdb_enable_transactions = 1;
         conf->bdb_tx_max = 50;
         rc = bdb_start(li, DBLAYER_NORMAL_MODE);
+        if (rc == 0) {
+            bdb_env = ((struct bdb_db_env*)(priv->dblayer_env))->bdb_DB_ENV;
+        }
     } else {
         /* Setup minimal environment */
         rc = db_env_create(&bdb_env, 0);
@@ -6968,18 +6980,36 @@ bdb_public_private_open(backend *be, const char *db_filename, int rw, dbi_env_t
 }
 
 int
-bdb_public_private_close(dbi_env_t **env, dbi_db_t **db)
+bdb_public_private_close(struct ldbminfo *li, dbi_env_t **env, dbi_db_t **db)
 {
     DB_ENV *bdb_env = *env;
     DB *bdb_db = *db;
     int rc = 0;
+    int rw = 0;
+    dblayer_private *priv = li->li_dblayer_private;
+    bdb_config *conf = (bdb_config *)li->li_dblayer_config;
 
-    if (bdb_db) {
-        rc = bdb_db->close(bdb_db, 0);
+    if (priv) {
+        /* Detect if db is fully set up in read write mode */
+        bdb_db_env *pEnv = (bdb_db_env *)priv->dblayer_env;
+        if (pEnv && pEnv->bdb_thread_count>0) {
+            rw = 1;
+        }
     }
-    if (bdb_env) {
-        rc = bdb_env->close(bdb_env, 0);
+    if (rw == 0) {
+        if (bdb_db) {
+            rc = bdb_db->close(bdb_db, 0);
+        }
+        if (bdb_env) {
+            rc = bdb_env->close(bdb_env, 0);
+        }
+    } else {
+        rc = bdb_close(li, DBLAYER_NORMAL_MODE);
     }
+    slapi_ch_free_string(&conf->bdb_dbhome_directory);
+    slapi_ch_free_string(&conf->bdb_home_directory);
+    slapi_ch_free_string(&conf->bdb_compactdb_time);
+    slapi_ch_free_string(&conf->bdb_log_directory);
     *db = NULL;
     *env = NULL;
     return bdb_map_error(__FUNCTION__, rc);

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

@@ -2728,7 +2728,7 @@ dbmdb_public_private_open(backend *be, const char *db_filename, int rw, dbi_env_
 
 
 int
-dbmdb_public_private_close(dbi_env_t **env, dbi_db_t **db)
+dbmdb_public_private_close(struct ldbminfo *li, dbi_env_t **env, dbi_db_t **db)
 {
     if (*db)
         dbmdb_public_db_op(*db, NULL, DBI_OP_CLOSE, NULL, NULL);

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

@@ -476,14 +476,12 @@ int dblayer_private_close(Slapi_Backend **be, dbi_env_t **env, dbi_db_t **db)
         dblayer_private *priv = li->li_dblayer_private;
 
         if (priv && priv->dblayer_private_close_fn) {
-            rc = priv->dblayer_private_close_fn(env, db);
+            rc = priv->dblayer_private_close_fn(li, env, db);
         }
+        slapi_ch_free_string(&li->li_directory);
         slapi_ch_free((void**)&li->li_dblayer_private);
         slapi_ch_free((void**)&li->li_dblayer_config);
-        if (dblayer_is_lmdb(*be)) {
-            /* Generate use after free and double free in bdb case */
-            ldbm_config_destroy(li);
-        }
+        ldbm_config_destroy(li);
         slapi_ch_free((void**)&(*be)->be_database);
         slapi_ch_free((void**)&(*be)->be_instance_info);
         slapi_ch_free((void**)be);

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

@@ -113,7 +113,7 @@ typedef int dblayer_dbi_txn_abort_fn_t(dbi_txn_t *txn);
 typedef int dblayer_get_entries_count_fn_t(dbi_db_t *db, dbi_txn_t *txn, int *count);
 typedef int dblayer_cursor_get_count_fn_t(dbi_cursor_t *cursor, dbi_recno_t *count);
 typedef int dblayer_private_open_fn_t(backend *be, const char *db_filename, int rw, dbi_env_t **env, dbi_db_t **db);
-typedef int dblayer_private_close_fn_t(dbi_env_t **env, dbi_db_t **db);
+typedef int dblayer_private_close_fn_t(struct ldbminfo *li, dbi_env_t **env, dbi_db_t **db);
 typedef int ldbm_back_wire_import_fn_t(Slapi_PBlock *pb);
 typedef int dblayer_restore_file_init_fn_t(struct ldbminfo *li);
 typedef void dblayer_restore_file_update_fn_t(struct ldbminfo *li, const char *directory);

ldap/servers/slapd/tools/dbscan.c

@@ -1162,6 +1162,7 @@ importdb(const char *dbimpl_name, const char *filename, const char *dump_name)
     dbi_db_t *db = NULL;
     int keyword = 0;
     int ret = 0;
+    int count = 0;
 
     if (dump_name == NULL) {
         printf("Error: dump_name can not be NULL\n");
@@ -1183,12 +1184,35 @@ importdb(const char *dbimpl_name, const char *filename, const char *dump_name)
         return 1;
     }
 
+    ret = dblayer_txn_begin(be, NULL, &txn);
+    if (ret != 0) {
+        printf("Error: failed to start the database txn. Error %d: %s\n", ret, dblayer_strerror(ret));
+    }
     while (ret == 0 &&
            !_read_line(dump, &keyword, &key) && keyword == 'k' &&
            !_read_line(dump, &keyword, &data) && keyword == 'v') {
         ret = dblayer_db_op(be, db, txn.txn, DBI_OP_PUT, &key, &data);
+        if (ret == 0  && count++ >= 1000) {
+            ret = dblayer_txn_commit(be, &txn);
+            if (ret != 0) {
+                printf("Error: failed to commit the database txn. Error %d: %s\n", ret, dblayer_strerror(ret));
+            } else {
+                count = 0;
+                ret = dblayer_txn_begin(be, NULL, &txn);
+                if (ret != 0) {
+                    printf("Error: failed to start the database txn. Error %d: %s\n", ret, dblayer_strerror(ret));
+                }
+            }
+        }
     }
-    if (ret !=0) {
+    if (ret == 0) {
+        ret = dblayer_txn_commit(be, &txn);
+        if (ret != 0) {
+            printf("Error: failed to commit the database txn. Error %d: %s\n", ret, dblayer_strerror(ret));
+        }
+    }
+    if (ret != 0) {
+        (void) dblayer_txn_abort(be, &txn);
         printf("Error: failed to write record in database. Error %d: %s\n", ret, dblayer_strerror(ret));
         dump_ascii_val("Failing record key", &key);
         dump_ascii_val("Failing record value", &data);
@@ -1331,7 +1355,7 @@ main(int argc, char **argv)
     /* Compute getopt short option string */
     {
         char *pt = optstring;
-        for (struct option *opt = options; opt->name; opt++) {
+        for (const struct option *opt = options; opt->name; opt++) {
             if (opt->val>0 && opt->val<OPT_FIRST) {
                 *pt++ = (char)(opt->val);
                 if (opt->has_arg == required_argument) {