From 209da318fec805ca0430f50337e829133ac444a0 Mon Sep 17 00:00:00 2001

From: Noriko Hosoi <nhosoi@totoro.usersys.redhat.com>

Date: Fri, 8 Feb 2013 12:13:14 -0800

Subject: [PATCH 32/33] Ticket #576 - DNA: use event queue for config update

 only at the start up

https://fedorahosted.org/389/ticket/576

Bug description: DNA config updates were always put into the

event queue and executed in 30 seconds, which increased a chance

to conflict with the ordinary modify operations and cause db

deadlocks.

Fix description: The 30 seconds delay is necessary at the start-

up time when MMR is configured to guarantee the shared config is

logged in the changelog.  This patch leaves the behaviour of the

config update at the start-up as it is; the rest won't be queued

but updated immediately.

Reviewed by Rich (Thank you!!)

(cherry picked from commit 7c11ed17796ba8b17afa5758fcfdf1c937b54ef4)

---

 ldap/servers/plugins/dna/dna.c |   78 +++++++++++++++++++++++++++------------

 1 files changed, 54 insertions(+), 24 deletions(-)

diff --git a/ldap/servers/plugins/dna/dna.c b/ldap/servers/plugins/dna/dna.c

index 36dd936..6babe23 100644

--- a/ldap/servers/plugins/dna/dna.c

+++ b/ldap/servers/plugins/dna/dna.c

@@ -93,6 +93,7 @@

 #define DNA_SHARED_CFG_DN   "dnaSharedCfgDN"

 /* Shared Config */

+#define DNA_SHAREDCONFIG    "dnaSharedConfig"

 #define DNA_REMAINING       "dnaRemainingValues"

 #define DNA_THRESHOLD       "dnaThreshold"

 #define DNA_HOSTNAME        "dnaHostname"

@@ -219,7 +220,7 @@ static int dna_be_txn_preop_init(Slapi_PBlock *pb);

  * Local operation functions

  *

  */

-static int dna_load_plugin_config();

+static int dna_load_plugin_config(int use_eventq);

 static int dna_parse_config_entry(Slapi_Entry * e, int apply);

 static void dna_delete_config();

 static void dna_free_config_entry(struct configEntry ** entry);

@@ -571,7 +572,7 @@ dna_start(Slapi_PBlock * pb)

         slapi_ch_calloc(1, sizeof(struct configEntry));

     PR_INIT_CLIST(dna_global_config);

-    if (dna_load_plugin_config() != DNA_SUCCESS) {

+    if (dna_load_plugin_config(1/* use eventq */) != DNA_SUCCESS) {

         slapi_log_error(SLAPI_LOG_FATAL, DNA_PLUGIN_SUBSYSTEM,

                         "dna_start: unable to load plug-in configuration\n");

         return DNA_FAILURE;

@@ -639,7 +640,7 @@ done:

  * ------ cn=etc etc

  */

 static int

-dna_load_plugin_config()

+dna_load_plugin_config(int use_eventq)

 {

     int status = DNA_SUCCESS;

     int result;

@@ -649,7 +650,8 @@ dna_load_plugin_config()

     Slapi_Entry **entries = NULL;

     slapi_log_error(SLAPI_LOG_TRACE, DNA_PLUGIN_SUBSYSTEM,

-                    "--> dna_load_plugin_config\n");

+                    "--> dna_load_plugin_config %s\n",

+                    use_eventq?"using event queue":"");

     dna_write_lock();

     dna_delete_config();

@@ -681,13 +683,18 @@ dna_load_plugin_config()

         dna_parse_config_entry(entries[i], 1);

     }

-    /* Setup an event to update the shared config 30

-     * seconds from now.  We need to do this since

-     * performing the operation at this point when

-     * starting up  would cause the change to not

-     * get changelogged. */

-    time(&now;;

-    slapi_eq_once(dna_update_config_event, NULL, now + 30);

+    if (use_eventq) {

+        /* Setup an event to update the shared config 30

+         * seconds from now.  We need to do this since

+         * performing the operation at this point when

+         * starting up  would cause the change to not

+         * get changelogged. */

+        time(&now;;

+        slapi_eq_once(dna_update_config_event, NULL, now + 30);

+    } else {

+        int nolock = 1; /* no need to lock since dna write lock is held. */

+        dna_update_config_event(0, &nolock);

+    }

 cleanup:

     slapi_free_search_results_internal(search_pb);

@@ -1240,9 +1247,15 @@ dna_update_config_event(time_t event_time, void *arg)

     Slapi_PBlock *pb = NULL;

     struct configEntry *config_entry = NULL;

     PRCList *list = NULL;

+    int nolock = 0;

-    /* Get read lock to prevent config changes */

-    dna_read_lock();

+    if (arg) {

+        nolock = *(int *)arg;

+    }

+    if (!nolock) {

+        /* Get read lock to prevent config changes */

+        dna_read_lock();

+    }

     /* Bail out if the plug-in close function was just called. */

     if (!g_plugin_started) {

@@ -1287,7 +1300,9 @@ dna_update_config_event(time_t event_time, void *arg)

     }

 bail:

-    dna_unlock();

+    if (!nolock) {

+        dna_unlock();

+    }

     slapi_pblock_destroy(pb);

 }

@@ -2161,7 +2176,7 @@ dna_update_shared_config(struct configEntry * config_entry)

                 slapi_entry_init_ext(e, sdn, NULL); /* sdn is copied into e */

                 slapi_sdn_free(&sdn;;

-                slapi_entry_add_string(e, SLAPI_ATTR_OBJECTCLASS, "dnaSharedConfig");

+                slapi_entry_add_string(e, SLAPI_ATTR_OBJECTCLASS, DNA_SHAREDCONFIG);

                 slapi_entry_add_string(e, DNA_HOSTNAME, hostname);

                 slapi_entry_add_string(e, DNA_PORTNUM, portnum);

                 if (secureportnum) {

@@ -2770,6 +2785,7 @@ _dna_pre_op_add(Slapi_PBlock *pb, Slapi_Entry *e, char **errstr)

     char **generated_types = NULL;

     PRUint64 setval = 0;

     int i;

+    int issharedconfig = 0;

     /* Bail out if the plug-in close function was just called. */

     if (!g_plugin_started) {

@@ -2787,7 +2803,12 @@ _dna_pre_op_add(Slapi_PBlock *pb, Slapi_Entry *e, char **errstr)

      *  We also check if we need to get the next range of values, and grab them.

      *  We do this here so we don't have to do it in the be_txn_preop.

      */

-    dna_read_lock();

+    /* Has write lock for config entries. */

+    issharedconfig = slapi_entry_attr_hasvalue(e, SLAPI_ATTR_OBJECTCLASS,

+                                               DNA_SHAREDCONFIG);

+    if (!issharedconfig) {

+        dna_read_lock();

+    }

     if (!PR_CLIST_IS_EMPTY(dna_global_config)) {

         list = PR_LIST_HEAD(dna_global_config);

@@ -2945,7 +2966,9 @@ next:

         }

     }

-    dna_unlock();

+    if (!issharedconfig) {

+        dna_unlock();

+    }

     slapi_ch_array_free(generated_types);

 bail:

@@ -3314,7 +3337,7 @@ dna_pre_op(Slapi_PBlock * pb, int modtype)

             ret = _dna_pre_op_add(pb, test_e, &errstr);

         } else {

             if((ret = _dna_pre_op_modify(pb, test_e, smods, &errstr))){

-            	slapi_mods_free(&smods);

+                slapi_mods_free(&smods);

             }

         }

         if (ret) {

@@ -3404,6 +3427,7 @@ static int dna_be_txn_pre_op(Slapi_PBlock *pb, int modtype)

     char *type = NULL;

     int numvals, e_numvals = 0;

     int i, len, ret = 0;

+    int issharedconfig = 0;

     slapi_log_error(SLAPI_LOG_TRACE, DNA_PLUGIN_SUBSYSTEM,

                     "--> dna_be_txn_pre_op\n");

@@ -3439,7 +3463,12 @@ static int dna_be_txn_pre_op(Slapi_PBlock *pb, int modtype)

         slapi_mods_init_passin(smods, mods);

     }

-    dna_read_lock();

+    /* Has write lock for config entries. */

+    issharedconfig = slapi_entry_attr_hasvalue(e, SLAPI_ATTR_OBJECTCLASS,

+                                               DNA_SHAREDCONFIG);

+    if (!issharedconfig) {

+        dna_read_lock();

+    }

     if (!PR_CLIST_IS_EMPTY(dna_global_config)) {

         list = PR_LIST_HEAD(dna_global_config);

@@ -3449,7 +3478,7 @@ static int dna_be_txn_pre_op(Slapi_PBlock *pb, int modtype)

             /* Did we already service all of these configured types? */

             if (dna_list_contains_types(generated_types, config_entry->types)) {

-                    goto next;

+                goto next;

             }

             /* is the entry in scope? */

@@ -3626,12 +3655,13 @@ static int dna_be_txn_pre_op(Slapi_PBlock *pb, int modtype)

             } else if (types_to_generate) {

                 slapi_ch_free((void **)&types_to_generate);

             }

-          next:

+next:

             list = PR_NEXT_LINK(list);

         }

     }

-    dna_unlock();

-

+    if (!issharedconfig) {

+        dna_unlock();

+    }

 bail:

     if (LDAP_CHANGETYPE_MODIFY == modtype) {

@@ -3669,7 +3699,7 @@ static int dna_config_check_post_op(Slapi_PBlock * pb)

     if (!slapi_op_internal(pb)) { /* If internal, no need to check. */

         if ((dn = dna_get_dn(pb))) {

             if (dna_dn_is_config(dn)) {

-                dna_load_plugin_config();

+                dna_load_plugin_config(0);

             }

         }

     }

-- 

1.7.7.6