Bug 628300
| Summary: | DN is not normalized in dn/entry cache when an entry is added, entrydn is not present in search results | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Retired] 389 | Reporter: | Andrey Ivanov <andrey.ivanov> | ||||||||
| Component: | Database - Indexes/Searches | Assignee: | Noriko Hosoi <nhosoi> | ||||||||
| Status: | CLOSED CURRENTRELEASE | QA Contact: | Viktor Ashirov <vashirov> | ||||||||
| Severity: | medium | Docs Contact: | |||||||||
| Priority: | low | ||||||||||
| Version: | 1.2.6 | CC: | amsharma, rmeggins | ||||||||
| Target Milestone: | --- | ||||||||||
| Target Release: | --- | ||||||||||
| Hardware: | x86_64 | ||||||||||
| OS: | Linux | ||||||||||
| Whiteboard: | |||||||||||
| Fixed In Version: | Doc Type: | Bug Fix | |||||||||
| Doc Text: | Story Points: | --- | |||||||||
| Clone Of: | Environment: | ||||||||||
| Last Closed: | 2015-12-07 16:31:31 UTC | Type: | --- | ||||||||
| Regression: | --- | Mount Type: | --- | ||||||||
| Documentation: | --- | CRM: | |||||||||
| Verified Versions: | Category: | --- | |||||||||
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||||||
| Cloudforms Team: | --- | Target Upstream Version: | |||||||||
| Embargoed: | |||||||||||
| Bug Depends On: | |||||||||||
| Bug Blocks: | 543590, 639035 | ||||||||||
| Attachments: |
|
||||||||||
|
Description
Andrey Ivanov
2010-08-29 10:11:48 UTC
Thanks for the bug report, Andrey! Actually, it's a bug in the entrydn support. (Bug 578296 - Attribute type entrydn needs to be added when subtree rename switch is on. ) This normalized entry also did not show the entrydn. *shame shame* :( dn: uid=bug2-normalizer,ou=payroll,dc=eXAmple,dc=COM changetype: add objectClass: top objectClass: person objectClass: organizationalPerson objectClass: inetOrgPerson cn: Bug2 Normalizer Test sn: Bug2 Normalizer uid: bug2-normalizer The fix is very simple. I'm attaching it next. Created attachment 441989 [details] git patch file (master) Description: Code for supporting entrydn (added for Bug 578296) contained a bug. If an entry was found in the entry cache, id2entry_ext returned it without adding the entrydn attribute value. This patch fixes the problem. Hi, Noriko, thanks for your patch! Yes, i've seen the Bug 578296, but i've made a new one since it's only a part of the problem. Your patch does what is necessary to make appear the entrydn attribute, however the second part of the bug is still here - the returned DN in ldapsearch is still non-normalized and becomes normalized only after server restart : ldapsearch -x -b "dc=example,dc=com" entrydn dn: uid=bug-normalizer,ou=paYROll,dc=eXAmple,dc=COM entrydn: uid=bug-normalizer,ou=payroll,dc=example,dc=com And after the server restart the returned dn is normalised (service dirsrv restart): dn: uid=bug-normalizer,ou=Payroll,dc=example,dc=com entrydn: uid=bug-normalizer,ou=payroll,dc=example,dc=com What i mean is that the server response should not depend on whether it is restarted or not, it should be invariant. Actually, it's a bit tricky issue... DN normalizer (slapi_dn_normalize_ext) does not change the case. For instance, if you add this test entry, dn: uid=bug3-normalizer, ou=p\61yroll, dc=eXAmple, dc=COM [..] cn: Bug3 Normalizer Test sn: Bug3 Normalizer uid: bug3-normalizer You get this search result (before restarting the server). # bug3-normalizer, payroll, eXAmple.COM dn: uid=bug3-normalizer,ou=payroll,dc=eXAmple,dc=COM entrydn: uid=bug3-normalizer,ou=payroll,dc=example,dc=com Once the server is restarted, the result becomes like this: # bug3-normalizer, Payroll, example.com dn: uid=bug3-normalizer,ou=Payroll,dc=example,dc=com entrydn: uid=bug3-normalizer,ou=payroll,dc=example,dc=com The lower case payroll and mixed case eXAmple.COM are from the test entry's p\61yroll and dc=eXAmple, dc=COM, respectively. Both are "normalized". '\61' is converted to a and a space between eXAmple, and dc= is removed. Once the server is restarted, the parent entries' RDNs are respected. The dn of payroll happens to be "ou=Payroll" and dc=example's "dc=example,dc=com"... Probably, we should return all lowered (and normalized) DN as the search result? I don't think everything should lowered - the values of the naming attributes should be conserved as is. And the server gives the DN absolutely correctly after restart. So we need just to re-calculate the returned DN (when the entry is added to cache) in the same way the server does it during restart. This sort of normalization is done, for example, by Active Directory(i've just tested). I think openldap does the same thing. Another interesting notice is that moving the entry to another subtree works correctly, even if you put the name of the new subtree in a way like "ou=paYROll,dc=eXAmple,dc=COM" - the final returned DN will be correct, that is the new DN is recalculated during the entry subtree move. the DN calculation, of course, should be done only once, when the added entry goes into the dn or entry cache.. (In reply to comment #6) > I don't think everything should lowered - the values of the naming attributes > should be conserved as is. And the server gives the DN absolutely correctly > after restart. So we need just to re-calculate the returned DN (when the entry > is added to cache) in the same way the server does it during restart. Well, that's going to be a big change... :) Currently, I'm trying to reduce the entryrdn index access as much as possible, which involves disk IO with traversing DIT. When an entry is added, the server receives the DN info (with possible random cases like dc=eXAmple,dc=COM). That is, we don't have to assemble the DN string from the entryrdn index. And we don't. We use the given DN after normalized. That's the reason why you see the user passed cases. Please note that the DN is temporarily stored in the entry cache in memory. So, once the entry is evicted and retrieved from the db, you see the updated result (no more eXAmple.COM, e.g.). Once you restart the server, the entry cache is reset. Following search has to go to the entryrdn to assemble the DN string, which shows the original RDN in the index. Compared with the entryrdn index access, lowering case is much less expensive. I can alter the behaviour that way with minimum effort. BTW, I tested your original entry with my openldap 2.4.23. It behaves in the same way as 389 does. 1) add an entry 2) search the entry # bug-normalizer, paYROll, eXAmple.COM dn: uid=bug-normalizer,ou=paYROll,dc=eXAmple,dc=COM entryDN: uid=bug-normalizer,ou=paYROll,dc=eXAmple,dc=COM 3) restart the server, then search the entry dn: uid=bug-normalizer,ou=Payroll,dc=example,dc=com entryDN: uid=bug-normalizer,ou=Payroll,dc=example,dc=com > This sort of normalization is done, for example, by Active Directory(i've just > tested). I think openldap does the same thing. > Another interesting notice is that moving the entry to another subtree works > correctly, even if you put the name of the new subtree in a way like > "ou=paYROll,dc=eXAmple,dc=COM" - the final returned DN will be correct, that is > the new DN is recalculated during the entry subtree move. Internally, the server also contains case lowered normalized RDN and use it for its process. So, despite of the representation, it works just fine. Yes, the mechanism why it happens this way is quite clear - to reduce the expensive (in disk IO or entryrdn access) re-constitution of dn. If openldap behaves in the same way i guess we should leave the things as they are now. In any case, the lowering is not a good idea - the client just after adding the entry will expect to see the retrieved entry's DN either exactly the same way it was in the added entry (the way it works now) or a "reconstructed" DN (that's what happens after the server restarts). Created attachment 442069 [details] git patch file (master) Description: Code for supporting entrydn (added for Bug 578296) contained a bug. If an entry was found in the entry cache, id2entry_ext returned it without adding the entrydn attribute value. This patch fixes the problem. In addition, if the parent DN in the to-be-added entry is not identical to the real parent DN (e.g., dc=eXAmple vs. dc=example), replace the string with the real parent DN. This check & replace is done only when the parent entry is in the entry cache not to sacrifice the performance. Thank you, Noriko! I think it's a very bright idea and a good tradeoff - to make the dn "path normalization" only if the parent is already in the cache. The client will probably make one or several searches at the parent level before adding a new entry, so the parent will most certainly go to cache. Moreover, i've just tested your patch. It works in all the cases, even if the entry add is the first operation after the server restart (that is, no caches are still present)! Not sure why it happens. Maybe during an entry addition the server code puts the entry's parent (or its DN) into cache automatically? There seem to be also a strange side effect of this patch: when using db2ldif (offline) i have an additional informational (or error?) line : entrycache_clear_int: there are still 17 entries in the entry cache. :/ This message does not appear without your patch. Here is the complete log : [31/Aug/2010:10:18:45 +0200] - WARNING: Import is running with nsslapd-db-private-import-mem on; No other process is allowed to access the database [31/Aug/2010:10:18:45 +0200] - check_and_set_import_cache: pagesize: 4096, pages: 240234, procpages: 47617 [31/Aug/2010:10:18:45 +0200] - WARNING: After allocating import cache 384372KB, the available memory is 576564KB, which is less than the soft limit 1048576KB. You may want to decrease the import cache size and rerun import. [31/Aug/2010:10:18:45 +0200] - Import allocates 384372KB import cache. [31/Aug/2010:10:18:45 +0200] - import userRoot: Beginning import job... [31/Aug/2010:10:18:45 +0200] - import userRoot: Index buffering enabled with bucket size 100 [31/Aug/2010:10:18:45 +0200] - import userRoot: Processing file "/tmp/prod_base_current.ldif" [31/Aug/2010:10:18:55 +0200] - import userRoot: Finished scanning file "/tmp/prod_base_current.ldif" (9527 entries) [31/Aug/2010:10:18:56 +0200] - import userRoot: Workers finished; cleaning up... [31/Aug/2010:10:18:56 +0200] - import userRoot: Workers cleaned up. [31/Aug/2010:10:18:56 +0200] - import userRoot: Cleaning up producer thread... [31/Aug/2010:10:18:56 +0200] - import userRoot: Indexing complete. Post-processing... [31/Aug/2010:10:18:57 +0200] - import userRoot: Flushing caches... [31/Aug/2010:10:18:57 +0200] - import userRoot: Closing files... [31/Aug/2010:10:18:57 +0200] - entrycache_clear_int: there are still 17 entries in the entry cache. :/ [31/Aug/2010:10:18:57 +0200] - All database threads now stopped [31/Aug/2010:10:18:57 +0200] - import userRoot: Import complete. Processed 9527 entries in 12 seconds. (793.92 entries/sec) Created attachment 442238 [details]
git patch file (master)
Good catch! Thanks a lot, Andrey. I forgot to return the found entries to the cache. :(
I'm attaching the revised code.
Reviewed by Rich (Thanks!!!) Pushed to master. $ git push Counting objects: 67, done. Delta compression using up to 4 threads. Compressing objects: 100% (41/41), done. Writing objects: 100% (41/41), 14.98 KiB, done. Total 41 (delta 33), reused 0 (delta 0) To ssh://git.fedorahosted.org/git/389/ds.git a2bcd81..cc36301 master -> master https://bugzilla.redhat.com/show_bug.cgi?id=578296 is VERIFIED (related bug) and some more testing : ========================= [root@testvm slapd-testvm1]# ldapadd -x -h localhost -p 1389 -D "cn=Directory Manager" -w Secret123 << EOF > dn: uid=bug-normalizer,ou=paYROll,dc=eXAmple,dc=COM > changetype: add > objectClass: top > objectClass: person > objectClass: organizationalPerson > objectClass: inetOrgPerson > cn: Bug Normalizer Test > sn: Bug Normalizer > uid: bug-normalizer > EOF adding new entry "uid=bug-normalizer,ou=paYROll,dc=eXAmple,dc=COM" [root@testvm slapd-testvm1]# ldapsearch -x -h localhost -p 1389 -D "cn=Directory Manager" -w Secret123 -b "ou=Payroll,dc=example,dc=com" entrydn | grep uid=bug-normalizer dn: uid=bug-normalizer,ou=Payroll,dc=example,dc=com entrydn: uid=bug-normalizer,ou=payroll,dc=example,dc=com |