#49495 search with CoS attribute is getting slower after modifying/adding CosTemplate
Closed: wontfix Opened by firstyear.

Ticket was cloned from Red Hat Bugzilla (product Red Hat Enterprise Linux 7): Bug 1523183

Created attachment 1364283
sample ldif to reproduce the issue
Description of problem:
When using Indirect Cos like the following,
dn: cn=cosDefinition,dc=test,dc=com
objectClass: top
objectClass: ldapsubentry
objectClass: cossuperdefinition
objectClass: cosIndirectDefinition
cosAttribute: ou merge-schemes
cosAttribute: description merge-schemes
cosAttribute: postalCode merge-schemes
cn: cosDefinition
cosIndirectSpecifier: seeAlso
search takes longer than before after cosTemplate entries are added/modified.
Version-Release number of selected component (if applicable):
389-ds-base-1.3.6.1-24.el7_4.x86_64
How reproducible:
This can be reproduced with attached sample LDIF.
Steps to Reproduce:
1. create suffix dc=test,dc=com and import test.ldif
   (or adjust suffix accordingly)
2. run the following search
ldapsearch -D "cn=directory manager" -w password -b dc=test,dc=com uid=user1
  => this return the result immediately
3. add new cosTemplcate by the following command
ldapmodify -D "cn=directory manager" -w password  -a -c -f ou10000.ldif
   stop ldapmodify after adding about 500 entries by CTRL+C
4. run the same search
ldapsearch -D "cn=directory manager" -w password -b dc=test,dc=com uid=user1
  => this will take a time than step 2
5. add new cosTemplcate by the following command again
ldapmodify -D "cn=directory manager" -w password  -a -c -f ou10000.ldif
   stop ldapmodify after adding around 500 entries by CTRL+C
6. run the same search
ldapsearch -D "cn=directory manager" -w password -b dc=test,dc=com uid=user1
  => this will take a time than step 2 and 4
7. add additional Cos Identifier attribute
  ldapmodify -D "cn=directory manager" -w password -a -f addCosIdentifier.ldif
8. run the same search
ldapsearch -D "cn=directory manager" -w password -b dc=test,dc=com uid=user1
  => this will take too time than before (Cos works as expected though)
9. restart server and do the same search
  => this returns entry immediately
Actual results:
search takes longer time than before
Expected results:
search returns result soon
Additional info:
This happen after applying 389-ds-base-1.3.6.1-24.el7_4.x86_64.
But this does not happen with 389-ds-base-1.3.6.1-23.el7_4.x86_64

Metadata Update from @firstyear:
- Custom field rhbz adjusted to https://bugzilla.redhat.com/show_bug.cgi?id=1523183

Metadata Update from @firstyear:
- Custom field component adjusted to None
- Custom field origin adjusted to None
- Custom field reviewstatus adjusted to None
- Custom field type adjusted to None
- Custom field version adjusted to None
- Issue assigned to firstyear
- Issue priority set to: critical
- Issue set to the milestone: 1.3.6.0 (was: 0.0 NEEDS_TRIAGE)

Metadata Update from @firstyear:
- Custom field reviewstatus adjusted to review (was: None)

looks good to me, but am not a real expert.

if nobody objects, you have my ack

Thanks. I'll ask @spichugi to review the lib389 parts, and they can be merged separately.

The patch looks good to my as well.
Just a remark in the loop in vattr_map_entry_free, it only free list_entry at the condition rc==0.
And later free the head of the list vae.
If some list_entry are rc<>0 it should leak few elements.

That's the risk of ref counting though ....

Anyway ,the testing showed no leaks with thousands of templates so far, so I'm confident.

I think that the point is there is:

  • an entry map, that contains "a set of service providers" and "one attribute"
  • a service provider can be associated to "many attributes"

So you could have:

map_1 -> sp1
map_2 -> sp1

So that's why the rc is used. Because every sp is in the vattr maps, but the map can point at the same sp's

But if you see the comment in the code, the reason why rc of sp can be > 1 at all, is an architectural issue in how we actually create the new entry maps ....

Else this would be easier :)

0001-Ticket-49495-cos-stress-test-and-improvements.patch

Fix docstrings for @spichugi

Since there are no objects from @tbordaz and @lkrispen, I will merge the C part of this patch now. Thank you!

and here is the official ack :-)

Metadata Update from @lkrispen:
- Custom field reviewstatus adjusted to ack (was: review)

commit c0346d5f6bc7379af19fe3b9f1bf78dbebb3bf67
To ssh://git@pagure.io/389-ds-base.git
b5e840b..c0346d5 master -> master

commit 4b8fc4b9d57535eb02c898e7c157e661395d2fa5
To ssh://git@pagure.io/389-ds-base.git
3fb1c40..4b8fc4b 389-ds-base-1.3.7 -> 389-ds-base-1.3.7

You have my ack too

0001-Ticket-49495-Fix-memory-management-is-vattr.patch

This is the 1.3.6 version of the patch with the backport of incr/decr for atomics.

Lib389 parts, thanks @spichugi

commit ab61eff1dfae54e84419fd4c059a3bae902fdf95
To ssh://git@pagure.io/389-ds-base.git
c0346d5..ab61eff master -> master

commit 572f978710ccd769673fb4e7c9db1a916afe9b5f
To ssh://git@pagure.io/389-ds-base.git
693abeb..572f978 389-ds-base-1.3.6 -> 389-ds-base-1.3.6

Thank you everyone for all your help reviewing this!!!

Metadata Update from @firstyear:
- Issue close_status updated to: fixed
- Issue status updated to: Closed (was: Open)

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/2554

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)

Metadata