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:
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 :)
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
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.
subscribe
Thank you for understanding. We apologize for all inconvenience.
Metadata Update from @spichugi: - Issue close_status updated to: wontfix (was: fixed)