While working on virtual attribute lock contention (ticket 512). I setup a stap script to monitor that contention. The script also reported a high contention on a computed attribute lock. This is an unexpected side effect of the script that was suppose to only report virtual attribute lock contention.
How to reproduce
- Identify the computed attribute lock address, attach gdb to DS echo "print compute_evaluators_lock" | gdb /usr/bin/ns-slapd `ps -ef |grep ns-slapd | grep -v grep | awk '{print $2}'` | grep gdb - use the attached stap script. Use the test case described in ticket512.
The script will report (/tmp/stap_output) a set of high contention lock. One of them is compute_evaluators_lock (look for its address for example 0x2064690) The data showing the contention (Look into /tmp/stap_output)
lock 0x2064690 contended 277 times, 19492 avg us ... Histogram 0x2064690 value |-------------------------------------------------- count 0 |@@@@@@@@@ 19 1000 |@ 3 2000 |@@@@@ 10 3000 | 1 4000 |@ 3 5000 |@@@ 6 6000 |@@@@@@ 12 7000 |@@@@@@@@@@@ 23 8000 |@@@@ 8 9000 |@@@ 7 10000 |@@ 4 11000 |@@ 4 12000 |@@@ 7 13000 |@@ 4 14000 |@@@ 6 15000 |@@ 4 16000 |@@@@ 9 17000 |@ 3 18000 |@ 3 19000 |@@@ 6 20000 |@@ 4 21000 |@@@ 6 22000 |@@@@ 9 23000 |@@@@@@ 13 24000 |@@@ 7 25000 |@@ 5 26000 |@@@@ 8 27000 |@@@@ 8 28000 |@@@@ 8 29000 |@ 3 30000 |@@@@ 8 >30000 |@@@@@@@@@@@@@@@@@@@@@@@@@@@@ 56
attachment vattr_contention.sh
attachment vattr_contention.stp
attachment 0001-Ticket-616-High-contention-on-computed-attribute-loc.patch
I would prefer something like this instead: {{{ int needs_lock = require_compute_evaluator_lock; if (needs_lock) { slapi_rwlock_rdlock(compute_evaluators_lock); } rc = compute_call_evaluators_nolock(c, outfn, type, e); if (needs_lock) { slapi_rwlock_unlock(compute_evaluators_lock); } }}} Same with the code in slapi_compute_add_evaluator() and slapi_compute_add_search_rewriter() and compute_rewrite_search_filter() with require_compute_rewriters_lock.
There are several places where you use "elevator" instead of "evaluator".
{{{ char temp[10000], *s; if (PR_GetLine(fd, temp, sizeof(temp))) break; }}} use sizeof(temp) rather than hardcoded magic number.
attachment 0002-Ticket-616-High-contention-on-computed-attribute-loc.patch
Attached the new patch with the recommendations of the first review.
'''git merge ticket616'''
Updating ac6d8e5..4b7791f Fast-forward ldap/servers/slapd/computed.c | 149 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--------------- ldap/servers/slapd/main.c | 3 +- ldap/servers/slapd/proto-slap.h | 1 + ldap/servers/slapd/tools/rsearch/nametable.c | 4 +-- ldap/servers/slapd/tools/rsearch/searchthread.c | 2 +- 5 files changed, 126 insertions(+), 33 deletions(-)
'''git push origin master''' Enter passphrase for key '/home/tbordaz/.ssh/id_rsa_fedora': Counting objects: 23, done. Delta compression using up to 4 threads. Compressing objects: 100% (12/12), done. Writing objects: 100% (12/12), 2.74 KiB, done. Total 12 (delta 10), reused 0 (delta 0) To ssh://git.fedorahosted.org/git/389/ds.git ac6d8e5..4b7791f master -> master
commit 4b7791f Author: Thierry bordaz (tbordaz) tbordaz@redhat.com Date: Thu Mar 14 18:35:21 2013 +0100
Metadata Update from @tbordaz: - Issue assigned to tbordaz - Issue set to the milestone: 1.3.1
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 issue has been cloned to Github and is available here: - https://github.com/389ds/389-ds-base/issues/616
If you want to receive further updates on the issue, please navigate to the github issue and click on subscribe button.
subscribe
Thank you for understanding. We apologize for all inconvenience.
Metadata Update from @spichugi: - Issue close_status updated to: wontfix (was: Fixed)
Login to comment on this ticket.