[389-commits] [389-ds-base] branch 389-ds-base-1.3.9 updated: Ticket 49873 - Contention on virtual attribute lookup

This is an automated email from the git hooks/post-receive script.

tbordaz pushed a commit to branch 389-ds-base-1.3.9
in repository 389-ds-base.

The following commit(s) were added to refs/heads/389-ds-base-1.3.9 by this push:
new f6c0c84 Ticket 49873 - Contention on virtual attribute lookup
f6c0c84 is described below

commit f6c0c84e24096ca7e789af8ff03e11e2edc73a7e
Author: Thierry Bordaz <tbordaz@redhat.com>
AuthorDate: Tue Jan 15 11:13:42 2019 +0100

Ticket 49873 - Contention on virtual attribute lookup

Bug Description:
During lookup of the virtual attribute table (filter evaluation and returned attribute)
the lock is acquired many times in read. For example it is acquired for each targetfilter aci and for
each evaluated entry.
Unfortunately RW lock is expensive and appears frequently on pstacks.
The lock exists because the table can be updated but update is very rare (addition of a new service provider).
So it slows down general proceeding for exceptional events.

Fix Description:
The fix is to acquire/release the read lock at the operation level and set a per-cpu flag, so that later lookup
would just check the flag.

SSL initialization does internal searches that access the vattr_global_lock
Call of vattr_global_lock_create needs to be called before slapd_do_all_nss_ssl_init.

Also, 'main' may or may not fork, the initialization fo the thread private variable
is done either on the child or parent depending if main forks or not.

The leak is fixed using a destructor callback of the private variable and so
call PR_SetThreadPrivate only if there is no private variable.

This patch is the merge of the four 49873 patches done in master

https://pagure.io/389-ds-base/issue/49873

Reviewed by: Ludwig Krispenz, William Brown , Simon Pichugi (thanks !!)

Platforms tested: F28

Flag Day: no

Doc impact: no
---
ldap/servers/slapd/detach.c | 9 +++
ldap/servers/slapd/opshared.c | 6 ++
ldap/servers/slapd/proto-slap.h | 5 ++
ldap/servers/slapd/vattr.c | 164 +++++++++++++++++++++++++++++++++++-----
4 files changed, 167 insertions(+), 17 deletions(-)

diff --git a/ldap/servers/slapd/detach.c b/ldap/servers/slapd/detach.c
index 681e6a7..d5c95a0 100644
--- a/ldap/servers/slapd/detach.c
+++ b/ldap/servers/slapd/detach.c
@@ -144,6 +144,10 @@ detach(int slapd_exemode, int importexport_encrypt, int s_port, daemon_ports_t *
}
break;
}
+ /* The thread private counter needs to be allocated after the fork
+ * it is not inherited from parent process
+ */
+ vattr_global_lock_create();

/* call this right after the fork, but before closing stdin */
if (slapd_do_all_nss_ssl_init(slapd_exemode, importexport_encrypt, s_port, ports_info)) {
@@ -174,6 +178,11 @@ detach(int slapd_exemode, int importexport_encrypt, int s_port, daemon_ports_t *

g_set_detached(1);
} else { /* not detaching - call nss/ssl init */
+ /* The thread private counter needs to be allocated after the fork
+ * it is not inherited from parent process
+ */
+ vattr_global_lock_create();
+
if (slapd_do_all_nss_ssl_init(slapd_exemode, importexport_encrypt, s_port, ports_info)) {
return 1;
}
diff --git a/ldap/servers/slapd/opshared.c b/ldap/servers/slapd/opshared.c
index 50b7ae8..cf6cdff 100644
--- a/ldap/servers/slapd/opshared.c
+++ b/ldap/servers/slapd/opshared.c
@@ -244,6 +244,7 @@ op_shared_search(Slapi_PBlock *pb, int send_result)
int pr_idx = -1;
Slapi_DN *orig_sdn = NULL;
int free_sdn = 0;
+ PRBool vattr_lock_acquired = PR_FALSE;

be_list[0] = NULL;
referral_list[0] = NULL;
@@ -511,6 +512,8 @@ op_shared_search(Slapi_PBlock *pb, int send_result)
}

slapi_pblock_set(pb, SLAPI_BACKEND_COUNT, &index);
+ vattr_rdlock();
+ vattr_lock_acquired = PR_TRUE;

if (be) {
slapi_pblock_set(pb, SLAPI_BACKEND, be);
@@ -969,6 +972,9 @@ free_and_return:
} else if (be_single) {
slapi_be_Unlock(be_single);
}
+ if (vattr_lock_acquired) {
+ vattr_rd_unlock();
+ }

free_and_return_nolock:
slapi_pblock_set(pb, SLAPI_PLUGIN_OPRETURN, &rc);
diff --git a/ldap/servers/slapd/proto-slap.h b/ldap/servers/slapd/proto-slap.h
index 7a429b2..79017e6 100644
--- a/ldap/servers/slapd/proto-slap.h
+++ b/ldap/servers/slapd/proto-slap.h
@@ -1409,6 +1409,11 @@ void subentry_create_filter(Slapi_Filter **filter);
* vattr.c
*/
void vattr_init(void);
+void vattr_global_lock_create(void);
+void vattr_rdlock();
+void vattr_rd_unlock();
+void vattr_wrlock();
+void vattr_wr_unlock();
void vattr_cleanup(void);

/*
diff --git a/ldap/servers/slapd/vattr.c b/ldap/servers/slapd/vattr.c
index f7c473a..852a887 100644
--- a/ldap/servers/slapd/vattr.c
+++ b/ldap/servers/slapd/vattr.c
@@ -102,6 +102,16 @@ int vattr_basic_sp_init();

void **statechange_api;

+struct _vattr_map
+{
+ Slapi_RWLock *lock;
+ PLHashTable *hashtable; /* Hash table */
+};
+typedef struct _vattr_map vattr_map;
+
+static vattr_map *the_map = NULL;
+static PRUintn thread_private_global_vattr_lock;
+
/* Housekeeping Functions, called by server startup/shutdown code */

/* Called on server startup, init all structures etc */
@@ -115,7 +125,136 @@ vattr_init()
vattr_basic_sp_init();