#49495 search with CoS attribute is getting slower after modifying/adding CosTemplate
Closed: fixed 2 years ago Opened 2 years ago 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

2 years ago

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)

2 years ago

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

2 years ago

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 :)

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)

2 years ago

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

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

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 ab61eff
To ssh://git@pagure.io/389-ds-base.git
c0346d5..ab61eff master -> master

commit 572f978
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)

2 years ago

Login to comment on this ticket.

Metadata