ldap/servers/plugins/memberof/memberof.c | 126 +++++++++++++++++++++++++++++++
1 file changed, 126 insertions(+)
New commits:
commit 2ce0a6ba8ff4f03288ac3d6fc73157ef9cb3c710
Author: Thierry Bordaz <tbordaz(a)redhat.com>
Date: Fri Dec 2 18:38:05 2016 +0100
Ticket 48861: Memberof plugins can update several times the same entry to set the same
values
Bug Description:
When a group is updated, impacted members are updated (fixup) by memberof
plugins.
If there are multiple paths (membership link) between the udpated group and
the impacted members then those impacted members are fixup several times
Fix Description:
Store impacted members in a global hash table (based on impacted members
normalized DN).
If an impacted member has already been fixup, just skip it.
(design is
http://www.port389.org/docs/389ds/design/memberof-scalability.html)
https://fedorahosted.org/389/ticket/48861
Reviewed by: William Brown (Thanks for your patience and continous help !)
Platforms tested: F24
Flag Day: no
Doc impact: no
diff --git a/ldap/servers/plugins/memberof/memberof.c
b/ldap/servers/plugins/memberof/memberof.c
index 3b38559..fc079f8 100644
--- a/ldap/servers/plugins/memberof/memberof.c
+++ b/ldap/servers/plugins/memberof/memberof.c
@@ -37,6 +37,7 @@
#include "slapi-plugin.h"
#include "string.h"
#include "nspr.h"
+#include "plhash.h"
#include "memberof.h"
static Slapi_PluginDesc pdesc = { "memberof", VENDOR,
@@ -50,6 +51,8 @@ static PRMonitor *memberof_operation_lock = 0;
MemberOfConfig *qsortConfig = 0;
static int usetxn = 0;
static int premodfn = 0;
+#define MEMBEROF_HASHTABLE_SIZE 1000
+static PLHashTable *fixup_entry_hashtable = NULL; /* global hash table protected by
memberof_lock (memberof_operation_lock) */
typedef struct _memberofstringll
{
@@ -147,6 +150,8 @@ static int memberof_fix_memberof_callback(Slapi_Entry *e, void
*callback_data);
static int memberof_entry_in_scope(MemberOfConfig *config, Slapi_DN *sdn);
static int memberof_add_objectclass(char *auto_add_oc, const char *dn);
static int memberof_add_memberof_attr(LDAPMod **mods, const char *dn, char *add_oc);
+static PLHashTable *hashtable_new();
+static void fixup_hashtable_empty(char *msg);
/*** implementation ***/
@@ -338,6 +343,8 @@ int memberof_postop_start(Slapi_PBlock *pb)
}
}
+ fixup_entry_hashtable = hashtable_new();
+
/* Set the alternate config area if one is defined. */
slapi_pblock_get(pb, SLAPI_PLUGIN_CONFIG_AREA, &config_area);
if (config_area)
@@ -432,6 +439,11 @@ int memberof_postop_close(Slapi_PBlock *pb)
PR_DestroyMonitor(memberof_operation_lock);
memberof_operation_lock = NULL;
+ if (fixup_entry_hashtable) {
+ fixup_hashtable_empty("memberof_postop_close empty fixup_entry_hastable");
+ PL_HashTableDestroy(fixup_entry_hashtable);
+ }
+
slapi_log_err(SLAPI_LOG_TRACE, MEMBEROF_PLUGIN_SUBSYSTEM,
"<-- memberof_postop_close\n" );
return 0;
@@ -2614,10 +2626,16 @@ void memberof_lock()
if (usetxn) {
PR_EnterMonitor(memberof_operation_lock);
}
+ if (fixup_entry_hashtable) {
+ fixup_hashtable_empty("memberof_lock");
+ }
}
void memberof_unlock()
{
+ if (fixup_entry_hashtable) {
+ fixup_hashtable_empty("memberof_lock");
+ }
if (usetxn) {
PR_ExitMonitor(memberof_operation_lock);
}
@@ -2862,6 +2880,9 @@ int memberof_fix_memberof_callback(Slapi_Entry *e, void
*callback_data)
MemberOfConfig *config = (MemberOfConfig *)callback_data;
memberof_del_dn_data del_data = {0, config->memberof_attr};
Slapi_ValueSet *groups = 0;
+ const char *ndn;
+ char *dn_copy;
+
/*
* If the server is ordered to shutdown, stop the fixup and return an error.
@@ -2870,6 +2891,14 @@ int memberof_fix_memberof_callback(Slapi_Entry *e, void
*callback_data)
rc = -1;
goto bail;
}
+
+ /* Check if the entry has not already been fixed */
+ ndn = slapi_sdn_get_ndn(sdn);
+ if (ndn && fixup_entry_hashtable &&
PL_HashTableLookupConst(fixup_entry_hashtable, (void*) ndn)) {
+ slapi_log_error(SLAPI_LOG_PLUGIN, MEMBEROF_PLUGIN_SUBSYSTEM, "Entry %s already
fixed up\n", ndn);
+ goto bail;
+ }
+
/* get a list of all of the groups this user belongs to */
groups = memberof_get_groups(config, sdn);
@@ -2910,6 +2939,19 @@ int memberof_fix_memberof_callback(Slapi_Entry *e, void
*callback_data)
}
slapi_valueset_free(groups);
+
+ /* records that this entry has been fixed up */
+ if (fixup_entry_hashtable) {
+ dn_copy = slapi_ch_strdup(ndn);
+ if (PL_HashTableAdd(fixup_entry_hashtable, dn_copy, dn_copy) == NULL) {
+ slapi_log_error(SLAPI_LOG_FATAL, MEMBEROF_PLUGIN_SUBSYSTEM,
"memberof_fix_memberof_callback: "
+ "failed to add dn (%s) in the fixup hashtable; NSPR error - %d\n",
+ dn_copy, PR_GetError());
+ slapi_ch_free((void **) &dn_copy);
+ /* let consider this as not a fatal error, it just skip an optimization */
+ }
+ }
+
bail:
return rc;
}
@@ -2998,3 +3040,87 @@ memberof_add_objectclass(char *auto_add_oc, const char *dn)
return rc;
}
+
+static PRIntn memberof_hash_compare_keys(const void *v1, const void *v2)
+{
+ PRIntn rc;
+ if (0 == strcasecmp((const char *) v1, (const char *) v2)) {
+ rc = 1;
+ } else {
+ rc = 0;
+ }
+ return rc;
+}
+
+static PRIntn memberof_hash_compare_values(const void *v1, const void *v2)
+{
+ PRIntn rc;
+ if ((char *) v1 == (char *) v2) {
+ rc = 1;
+ } else {
+ rc = 0;
+ }
+ return rc;
+}
+
+/*
+ * Hashing function using Bernstein's method
+ */
+static PLHashNumber memberof_hash_fn(const void *key)
+{
+ PLHashNumber hash = 5381;
+ unsigned char *x = (unsigned char *)key;
+ int c;
+
+ while ((c = *x++)){
+ hash = ((hash << 5) + hash) ^ c;
+ }
+ return hash;
+}
+
+/* allocates the plugin hashtable
+ * This hash table is used by operation and is protected from
+ * concurrent operations with the memberof_lock (if not usetxn, memberof_lock
+ * is not implemented and the hash table will be not used.
+ *
+ * The hash table contains all the DN of the entries for which the memberof
+ * attribute has been computed/updated during the current operation
+ *
+ * hash table should be empty at the beginning and end of the plugin callback
+ */
+static PLHashTable *hashtable_new()
+{
+ if (!usetxn) {
+ return NULL;
+ }
+
+ return PL_NewHashTable(MEMBEROF_HASHTABLE_SIZE,
+ memberof_hash_fn,
+ memberof_hash_compare_keys,
+ memberof_hash_compare_values, NULL, NULL);
+}
+/* this function called for each hash node during hash destruction */
+static PRIntn fixup_hashtable_remove(PLHashEntry *he, PRIntn index, void *arg)
+{
+ char *dn_copy;
+ char *msg; /* used for debug purpose */
+
+ if (he == NULL) {
+ return HT_ENUMERATE_NEXT;
+ }
+
+
+ dn_copy = (char*) he->value;
+ msg = (char *) arg;
+
+ slapi_ch_free((void **) &dn_copy);
+
+ return HT_ENUMERATE_REMOVE;
+}
+
+static void fixup_hashtable_empty(char *msg)
+{
+ if (fixup_entry_hashtable) {
+ PL_HashTableEnumerateEntries(fixup_entry_hashtable, fixup_hashtable_remove, msg);
+ }
+}