#616 High contention on computed attribute lock
Closed: wontfix None Opened 8 years ago by tbordaz.

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

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.

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

4 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 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.

Thank you for understanding. We apologize for all inconvenience.

Metadata Update from @spichugi:
- Issue close_status updated to: wontfix (was: Fixed)

11 months ago

Login to comment on this ticket.

Metadata