#51202 Issue 51000 - Separate the BDB backend monitors
Closed 2 years ago by spichugi. Opened 2 years ago by mreynolds.
mreynolds/389-ds-base issue51000  into  master

@@ -2,6 +2,7 @@ 

  import pytest

  import os

  from lib389.monitor import *

+ from lib389.backend import Backends, DatabaseConfig

  from lib389._constants import *

  from lib389.topologies import topology_st as topo

  
@@ -63,6 +64,87 @@ 

      log.info('dtablesize: {0[0]},readwaiters: {0[1]},entriessent: {0[2]},bytessent: {0[3]},currenttime: {0[4]},starttime: {0[5]}'.format(stats))

  

  

+ pytestmark = pytest.mark.tier1

+ def test_monitor_ldbm(topo):

+     """This test is to check if we are getting the correct monitor entry

+ 

+     :id: e62ba369-32f5-4b03-8865-f597a5bb6a70

+     :setup: Single instance

+     :steps:

+         1. Get the backend library (bdb, ldbm, etc)

+         2. Get the database monitor

+         3. Check for expected attributes in output

+         4. Check for expected DB library specific attributes

+     :expectedresults:

+         1. Success

+         2. Success

+         3. Success

+         4. Success

+     """

+ 

+     # Are we using BDB?

+     db_config = DatabaseConfig(topo.standalone)

+     db_lib = db_config.get_db_lib()

+ 

+     # Get the database monitor entry

+     monitor = MonitorLDBM(topo.standalone).get_status()

+ 

+     # Check that known attributes exist (only NDN cache stats)

+     assert 'normalizeddncachehits' in monitor

+ 

+     # Check for library specific attributes

+     if db_lib == 'bdb':

+         assert 'dbcachehits' in monitor

+         assert 'nsslapd-db-configured-locks' in monitor

+     elif db_lib == 'lmdb':

+         pass

+     else:

+         # Unknown - the server would probably fail to start but check it anyway

+         log.fatal(f'Unknown backend library: {db_lib}')

+         assert False

+ 

+ 

+ pytestmark = pytest.mark.tier1

+ def test_monitor_backend(topo):

+     """This test is to check if we are getting the correct backend monitor entry

+ 

+     :id: 27b0534f-a18c-4c95-aa2b-936bc1886a7b

+     :setup: Single instance

+     :steps:

+         1. Get the backend library (bdb, ldbm, etc)

+         2. Get the backend monitor

+         3. Check for expected attributes in output

+         4. Check for expected DB library specific attributes

+     :expectedresults:

+         1. Success

+         2. Success

+         3. Success

+         4. Success

+     """

+ 

+     # Are we using BDB?

+     db_config = DatabaseConfig(topo.standalone)

+     db_lib = db_config.get_db_lib()

+ 

+     # Get the backend monitor

+     be = Backends(topo.standalone).list()[0]

+     monitor = be.get_monitor().get_status()

+ 

+     # Check for expected attributes

+     assert 'entrycachehits' in monitor

+     assert 'dncachehits' in monitor

+ 

+     # Check for library specific attributes

+     if db_lib == 'bdb':

+         assert 'dbfilename-0' in monitor

+     elif db_lib == 'lmdb':

+         pass

+     else:

+         # Unknown - the server would probably fail to start but check it anyway

+         log.fatal(f'Unknown backend library: {db_lib}')

+         assert False

+ 

+ 

  if __name__ == '__main__':

      # Run isolated

      # -s for DEBUG mode

@@ -88,6 +88,7 @@ 

      priv->instance_postdel_config_fn = &bdb_instance_post_delete_instance_entry_callback;

      priv->instance_cleanup_fn = &bdb_instance_cleanup;

      priv->instance_create_fn = &bdb_instance_create;

+     priv->instance_register_monitor_fn = &bdb_instance_register_monitor;

      priv->instance_search_callback_fn = &bdb_instance_search_callback;

      priv->dblayer_auto_tune_fn = &bdb_start_autotune;

      return 0;
@@ -1530,6 +1531,7 @@ 

      }

      return rval;

  }

+ 

  /* Reads in any config information held in the dse for the bdb

   * implementation of the ldbm plugin.

   * Creates dse entries used to configure the ldbm plugin and dblayer
@@ -1640,7 +1642,7 @@ 

       * Now still using ldbm functions 

       */

      slapi_config_register_callback(SLAPI_OPERATION_SEARCH, DSE_FLAG_PREOP, dn,

-                                    LDAP_SCOPE_BASE, "(objectclass=*)", ldbm_back_monitor_search,

+                                    LDAP_SCOPE_BASE, "(objectclass=*)", bdb_monitor_search,

                                     (void *)li);

      slapi_ch_free_string(&dn);

  
@@ -1656,7 +1658,7 @@ 

          goto bail;

      }

      slapi_config_register_callback(SLAPI_OPERATION_SEARCH, DSE_FLAG_PREOP, dn,

-                                    LDAP_SCOPE_BASE, "(objectclass=*)", ldbm_back_dbmonitor_search,

+                                    LDAP_SCOPE_BASE, "(objectclass=*)", bdb_dbmonitor_search,

                                     (void *)li);

  

  bail:
@@ -1664,6 +1666,72 @@ 

      return rval;

  }

  

+ /* general-purpose callback to deny an operation */

+ static int

+ bdb_deny_config(Slapi_PBlock *pb __attribute__((unused)),

+                 Slapi_Entry *e __attribute__((unused)),

+                 Slapi_Entry *entryAfter __attribute__((unused)),

+                 int *returncode,

+                 char *returntext __attribute__((unused)),

+                 void *arg __attribute__((unused)))

+ {

+     *returncode = LDAP_UNWILLING_TO_PERFORM;

+     return SLAPI_DSE_CALLBACK_ERROR;

+ }

+ 

+ int

+ bdb_instance_register_monitor(ldbm_instance *inst) {

+     struct ldbminfo *li = inst->inst_li;

+     char *dn = NULL;

+ 

+     dn = slapi_create_dn_string("cn=monitor,cn=%s,cn=%s,cn=plugins,cn=config",

+                                 inst->inst_name, li->li_plugin->plg_name);

+     if (NULL == dn) {

+         slapi_log_err(SLAPI_LOG_ERR,

+                       "bdb_instance_register_monitor",

+                       "failed create monitor instance dn for plugin %s, "

+                       "instance %s\n",

+                       inst->inst_li->li_plugin->plg_name, inst->inst_name);

+         return 1;

+     }

+     /* make callback on search; deny add/modify/delete */

+     slapi_config_register_callback(SLAPI_OPERATION_SEARCH, DSE_FLAG_PREOP, dn,

+                                    LDAP_SCOPE_BASE, "(objectclass=*)", bdb_monitor_instance_search,

+                                    (void *)inst);

+     slapi_config_register_callback(SLAPI_OPERATION_ADD, DSE_FLAG_PREOP, dn,

+                                    LDAP_SCOPE_SUBTREE, "(objectclass=*)", bdb_deny_config,

+                                    (void *)inst);

+     slapi_config_register_callback(SLAPI_OPERATION_MODIFY, DSE_FLAG_PREOP, dn,

+                                    LDAP_SCOPE_BASE, "(objectclass=*)", bdb_deny_config,

+                                    (void *)inst);

+     slapi_ch_free_string(&dn);

+ 

+     return 0;

+ }

+ 

+ void

+ bdb_instance_unregister_monitor(ldbm_instance *inst) {

+     struct ldbminfo *li = inst->inst_li;

+     char *dn = NULL;

+ 

+     dn = slapi_create_dn_string("cn=monitor,cn=%s,cn=%s,cn=plugins,cn=config",

+                                 inst->inst_name, li->li_plugin->plg_name);

+     if (NULL == dn) {

+         slapi_log_err(SLAPI_LOG_ERR,

+                       "bdb_instance_unregister_monitor",

+                       "Failed create monitor instance dn for plugin %s, "

+                       "instance %s\n",

+                       inst->inst_li->li_plugin->plg_name, inst->inst_name);

+         return;

+     }

+     slapi_config_remove_callback(SLAPI_OPERATION_SEARCH, DSE_FLAG_PREOP, dn,

+                                  LDAP_SCOPE_BASE, "(objectclass=*)", bdb_monitor_instance_search);

+     slapi_config_remove_callback(SLAPI_OPERATION_ADD, DSE_FLAG_PREOP, dn,

+                                  LDAP_SCOPE_SUBTREE, "(objectclass=*)", bdb_deny_config);

+     slapi_config_remove_callback(SLAPI_OPERATION_MODIFY, DSE_FLAG_PREOP, dn,

+                                  LDAP_SCOPE_BASE, "(objectclass=*)", bdb_deny_config);

+     slapi_ch_free_string(&dn);

+ }

  

  /* Utility function used in creating config entries.  Using the

   * config_info, this function gets info and formats in the correct

@@ -197,6 +197,8 @@ 

          if (inst_dirp != inst_dir) {

              slapi_ch_free_string(&inst_dirp);

          }

+         /* unregister the monitor */

+         bdb_instance_unregister_monitor(inst);

      } /* non-null pEnv */

      return SLAPI_DSE_CALLBACK_OK;

  }

@@ -150,7 +150,6 @@ 

  int bdb_back_fetch_incl_excl(Slapi_PBlock *pb, char ***include, char ***exclude);

  PRUint64 bdb_get_id2entry_size(ldbm_instance *inst);

  

- 

  /* bdb version functions */

  int bdb_version_write(struct ldbminfo *li, const char *directory, const char *dataversion, PRUint32 flags);

  int bdb_version_read(struct ldbminfo *li, const char *directory, char **ldbmversion, char **dataversion);
@@ -162,3 +161,10 @@ 

  int bdb_instance_add_instance_entry_callback(struct ldbminfo *li, struct ldbm_instance *inst);

  int bdb_instance_postadd_instance_entry_callback(struct ldbminfo *li, struct ldbm_instance *inst);

  void bdb_config_setup_default(struct ldbminfo *li);

+ 

+ /* monitor functions */

+ int bdb_monitor_instance_search(Slapi_PBlock *pb, Slapi_Entry *e, Slapi_Entry *entryAfter, int *returncode, char *returntext, void *arg);

+ int bdb_monitor_search(Slapi_PBlock *pb, Slapi_Entry *e, Slapi_Entry *entryAfter, int *returncode, char *returntext, void *arg);

+ int bdb_dbmonitor_search(Slapi_PBlock *pb, Slapi_Entry *e, Slapi_Entry *entryAfter, int *returncode, char *returntext, void *arg);

+ int bdb_instance_register_monitor(ldbm_instance *inst);

+ void bdb_instance_unregister_monitor(ldbm_instance *inst);

@@ -3145,7 +3145,7 @@ 

      run_from_cmdline = (task_flags & SLAPI_TASK_RUNNING_FROM_COMMANDLINE);

      slapi_pblock_get(pb, SLAPI_PLUGIN_PRIVATE, &li);

      if (run_from_cmdline) {

-         ldbm_config_load_dse_info(li);

+         bdb_config_load_dse_info(li);

          if (bdb_check_and_set_import_cache(li) < 0) {

              return -1;

          }

@@ -33,12 +33,12 @@ 

  

  /* DSE callback to monitor stats for a particular instance */

  int

- ldbm_back_monitor_instance_search(Slapi_PBlock *pb __attribute__((unused)),

-                                   Slapi_Entry *e,

-                                   Slapi_Entry *entryAfter __attribute__((unused)),

-                                   int *returncode,

-                                   char *returntext __attribute__((unused)),

-                                   void *arg)

+ bdb_monitor_instance_search(Slapi_PBlock *pb __attribute__((unused)),

+                             Slapi_Entry *e,

+                             Slapi_Entry *entryAfter __attribute__((unused)),

+                             int *returncode,

+                             char *returntext __attribute__((unused)),

+                             void *arg)

  {

      ldbm_instance *inst = (ldbm_instance *)arg;

      struct ldbminfo *li = NULL;
@@ -150,8 +150,8 @@ 

              continue;

  

          /* Since the filenames are now relative, we need to construct an absolute version

-      * for the purpose of stat() etc below...

-      */

+          * for the purpose of stat() etc below...

+          */

          slapi_ch_free_string(&absolute_pathname);

          absolute_pathname = slapi_ch_smprintf("%s%c%s", inst->inst_parent_dir_name, get_sep(inst->inst_parent_dir_name), mpfstat[i]->file_name);

  
@@ -159,10 +159,10 @@ 

          if (stat(absolute_pathname, &astat))

              continue;

          /* If the file has been re-created after been deleted

-      * We should show only statistics for the last instance

-      * Since SleepyCat returns the statistic of the last open file first,

-      * we should only display the first statistic record for a given file

-      */

+          * We should show only statistics for the last instance

+          * Since SleepyCat returns the statistic of the last open file first,

+          * we should only display the first statistic record for a given file

+          */

          for (j = 0; j < i; j++)

              if (!strcmp(mpfstat[i]->file_name, mpfstat[j]->file_name))

                  break;
@@ -194,7 +194,7 @@ 

  

  /* monitor global ldbm stats */

  int

- ldbm_back_monitor_search(Slapi_PBlock *pb, Slapi_Entry *e, Slapi_Entry *entryAfter, int *returncode, char *returntext, void *arg)

+ bdb_monitor_search(Slapi_PBlock *pb, Slapi_Entry *e, Slapi_Entry *entryAfter, int *returncode, char *returntext, void *arg)

  {

      struct ldbminfo *li = (struct ldbminfo *)arg;

      struct berval val;
@@ -288,7 +288,7 @@ 

  

  /* monitor global ldbm database stats */

  int

- ldbm_back_dbmonitor_search(Slapi_PBlock *pb __attribute__((unused)),

+ bdb_dbmonitor_search(Slapi_PBlock *pb __attribute__((unused)),

                             Slapi_Entry *e,

                             Slapi_Entry *entryAfter __attribute__((unused)),

                             int *returncode,

@@ -171,7 +171,7 @@ 

      slapi_pblock_get(pb, SLAPI_SEQ_TYPE, &verbose);

      slapi_pblock_get(pb, SLAPI_PLUGIN_PRIVATE, &li);

      slapi_pblock_get(pb, SLAPI_DBVERIFY_DBDIR, &dbdir);

-     ldbm_config_load_dse_info(li);

+     bdb_config_load_dse_info(li);

      bdb_config_internal_set(li, CONFIG_DB_TRANSACTION_LOGGING, "off");

  

      /* no write needed; choose EXPORT MODE */

@@ -273,10 +273,9 @@ 

      }

  

      ldbm_config_load_dse_info(li);

- 

      priv = (dblayer_private *)li->li_dblayer_private;

- 

      rc = priv->dblayer_load_dse_fn(li);

+ 

      return rc;

  }

  

@@ -147,6 +147,7 @@ 

      instance_config_entry_callback_fn_t *instance_postdel_config_fn;

      instance_cleanup_fn_t *instance_cleanup_fn;

      instance_create_fn_t *instance_create_fn;

+     instance_create_fn_t *instance_register_monitor_fn;

      instance_search_callback_fn_t *instance_search_callback_fn;

      dblayer_auto_tune_fn_t *dblayer_auto_tune_fn;

  };

@@ -1113,38 +1113,6 @@ 

                                     (void *)li);

      slapi_ch_free_string(&dn);

  

-     /* setup the dse callback functions for the ldbm backend monitor entry */

-     dn = slapi_create_dn_string("cn=monitor,cn=%s,cn=plugins,cn=config",

-                                 li->li_plugin->plg_name);

-     if (NULL == dn) {

-         slapi_log_err(SLAPI_LOG_ERR,

-                       "ldbm_config_load_dse_info",

-                       "failed to create monitor dn for %s\n",

-                       li->li_plugin->plg_name);

-         rval = 1;

-         goto bail;

-     }

-     slapi_config_register_callback(SLAPI_OPERATION_SEARCH, DSE_FLAG_PREOP, dn,

-                                    LDAP_SCOPE_BASE, "(objectclass=*)", ldbm_back_monitor_search,

-                                    (void *)li);

-     slapi_ch_free_string(&dn);

- 

-     /* And the ldbm backend database monitor entry */

-     dn = slapi_create_dn_string("cn=database,cn=monitor,cn=%s,cn=plugins,cn=config",

-                                 li->li_plugin->plg_name);

-     if (NULL == dn) {

-         slapi_log_err(SLAPI_LOG_ERR,

-                       "ldbm_config_load_dse_info",

-                       "failed create monitor database dn for %s\n",

-                       li->li_plugin->plg_name);

-         rval = 1;

-         goto bail;

-     }

-     slapi_config_register_callback(SLAPI_OPERATION_SEARCH, DSE_FLAG_PREOP, dn,

-                                    LDAP_SCOPE_BASE, "(objectclass=*)", ldbm_back_dbmonitor_search,

-                                    (void *)li);

-     slapi_ch_free_string(&dn);

- 

      /* setup the dse callback functions for the ldbm backend instance

       * entries */

      dn = slapi_create_dn_string("cn=%s,cn=plugins,cn=config",

@@ -675,32 +675,6 @@ 

                                     LDAP_SCOPE_BASE, "(objectclass=*)",

                                     ldbm_instance_deny_config, (void *)inst);

      /* delete is handled by a callback set in ldbm_config.c */

- 

-     slapi_ch_free_string(&dn);

- 

-     /* don't forget the monitor! */

-     dn = slapi_create_dn_string("cn=monitor,cn=%s,cn=%s,cn=plugins,cn=config",

-                                 inst->inst_name, li->li_plugin->plg_name);

-     if (NULL == dn) {

-         slapi_log_err(SLAPI_LOG_ERR,

-                       "ldbm_instance_config_load_dse_info",

-                       "failed create monitor instance dn for plugin %s, "

-                       "instance %s\n",

-                       inst->inst_li->li_plugin->plg_name, inst->inst_name);

-         rval = 1;

-         goto bail;

-     }

-     /* make callback on search; deny add/modify/delete */

-     slapi_config_register_callback(SLAPI_OPERATION_SEARCH, DSE_FLAG_PREOP, dn,

-                                    LDAP_SCOPE_BASE, "(objectclass=*)", ldbm_back_monitor_instance_search,

-                                    (void *)inst);

-     slapi_config_register_callback(SLAPI_OPERATION_ADD, DSE_FLAG_PREOP, dn,

-                                    LDAP_SCOPE_SUBTREE, "(objectclass=*)", ldbm_instance_deny_config,

-                                    (void *)inst);

-     slapi_config_register_callback(SLAPI_OPERATION_MODIFY, DSE_FLAG_PREOP, dn,

-                                    LDAP_SCOPE_BASE, "(objectclass=*)", ldbm_instance_deny_config,

-                                    (void *)inst);

-     /* delete is okay */

      slapi_ch_free_string(&dn);

  

      /* Callbacks to handle indexes */
@@ -908,6 +882,7 @@ 

  ldbm_instance_generate(struct ldbminfo *li, char *instance_name, Slapi_Backend **ret_be)

  {

      Slapi_Backend *new_be = NULL;

+     dblayer_private *priv = (dblayer_private *)li->li_dblayer_private;

      int rc = 0;

  

      /* Create a new instance, process config info for it,
@@ -922,6 +897,8 @@ 

      }

  

      ldbm_instance_config_load_dse_info(new_be->be_instance_info);

+     priv->instance_register_monitor_fn(new_be->be_instance_info);

+ 

      ldbm_instance_create_default_indexes(new_be);

  

      /* if USN plugin is enabled, set slapi_counter */
@@ -984,24 +961,6 @@ 

                                   ldbm_instance_deny_config);

      slapi_ch_free_string(&dn);

  

-     /* now the cn=monitor entry */

-     dn = slapi_create_dn_string("cn=monitor,cn=%s,cn=%s,cn=plugins,cn=config",

-                                 inst->inst_name, li->li_plugin->plg_name);

-     if (NULL == dn) {

-         slapi_log_err(SLAPI_LOG_ERR,

-                       "ldbm_instance_unregister_callbacks",

-                       "Failed create monitor instance dn for plugin %s, "

-                       "instance %s\n",

-                       inst->inst_li->li_plugin->plg_name, inst->inst_name);

-         goto bail;

-     }

-     slapi_config_remove_callback(SLAPI_OPERATION_SEARCH, DSE_FLAG_PREOP, dn,

-                                  LDAP_SCOPE_BASE, "(objectclass=*)", ldbm_back_monitor_instance_search);

-     slapi_config_remove_callback(SLAPI_OPERATION_ADD, DSE_FLAG_PREOP, dn,

-                                  LDAP_SCOPE_SUBTREE, "(objectclass=*)", ldbm_instance_deny_config);

-     slapi_config_remove_callback(SLAPI_OPERATION_MODIFY, DSE_FLAG_PREOP, dn,

-                                  LDAP_SCOPE_BASE, "(objectclass=*)", ldbm_instance_deny_config);

-     slapi_ch_free_string(&dn);

  

      /* now the cn=index entries */

      dn = slapi_create_dn_string("cn=index,cn=%s,cn=%s,cn=plugins,cn=config",

@@ -480,14 +480,6 @@ 

  int ldbm_back_isinitialized(void);

  

  /*

-  * monitor.c

-  */

- 

- int ldbm_back_monitor_search(Slapi_PBlock *pb, Slapi_Entry *e, Slapi_Entry *entryAfter, int *returncode, char *returntext, void *arg);

- int ldbm_back_monitor_instance_search(Slapi_PBlock *pb, Slapi_Entry *e, Slapi_Entry *entryAfter, int *returncode, char *returntext, void *arg);

- int ldbm_back_dbmonitor_search(Slapi_PBlock *pb, Slapi_Entry *e, Slapi_Entry *entryAfter, int *returncode, char *returntext, void *arg);

- 

- /*

   * vlv.c

   */

  struct vlv_request

@@ -1035,6 +1035,10 @@ 

                  vo = vo[0]

              self._instance.log.info(f'{k}: {vo}')

  

+     def get_db_lib(self):

+         """Return the backend library, bdb, lmdb, etc"""

+         return self._db_lib

+ 

      def set(self, value_pairs):

          for attr, val in value_pairs:

              attr = attr.lower()

Bug Description:

While trying to remove duplicate code from the backend and BDB backend code, I found that we were not correctly separating the BDB monitors from the core backend code.

Fix Description:

Move all the monitor registering to the db_layer private structure. This way we have fully isolated the monitors for each backend implementation/library. This also removed some duplicate code from the core backend and BDB code.

relates: https://pagure.io/389-ds-base/issue/51000

The code LGTM!
Also, basic, replication, import and disk_monitoring test suites pass.

rebased onto ed5b13c

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/4255

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