RFE - store full DN in database record #5494
Conversation
mreynolds389
added
the
work in progress
mreynolds389
force-pushed
the
issue5267
branch
5 times, most recently
from
90cde79
to
d60ad98
Compare
mreynolds389
force-pushed
the
issue5267
branch
from
d60ad98
to
abd2597
Compare
mreynolds389
removed
the
work in progress
|
Added a config setting to control how the DN is returned to a client (case intact, or normalized), but regardless what DN is returned to the client there is still the perf boost. So the config setting just controls what DN the client sees. |
|
Just to check is this going to disable subtree rename by default? It's a pretty rare operation IMO so I'd be okay with us optimising around the general (and simpler/faster) cases. |
|
@Firstyear ok so in the modrdn code it is adapted from working code (dsentrydn_* functions mirrored moddn functions). I'll have to go through it all. All my ASAN tests are good though, but I will clean this up. Just give me some time... |
|
No stress mate, beside those minor nits, I think it's a good idea and a good change :) |
No, this does not affect subtree rename. You can still move DN's between subtree's (hence all that extra modrdn code), but we do have all this code "switching" around entryrdn_get_switch(). With this patch we can eliminate the need for that old code, and we can stick with entryrdn. The only customers who had wanted this option ("nsslapd-subtree-rename-switch") were ones who needed to preserve the DN case (AFAIK) So this patch allows us to get a perf boost, and optionally preserve the case of DN's (which has been a pain point with some migrations to 389). |
mreynolds389
force-pushed
the
issue5267
branch
from
abd2597
to
dcdf4d7
Compare
|
Just found out the config setting has no effect. The DN case is always preserved in the entry. This could cause issues where client expects it to follow the old behavior. Will investigate... |
mreynolds389
force-pushed
the
issue5267
branch
from
dcdf4d7
to
b2ecb2f
Compare
This is fixed now, but it means the performance improvement will only happen if we configure the exact case to be returned. I was hoping the perf improvement would work regardless of how we return the DN to the client, but that's just not how it works. So if we want the perf improvement by default then the DN case behavior will change by default. We can discuss this later... |
mreynolds389
force-pushed
the
issue5267
branch
from
b2ecb2f
to
2cf07dd
Compare
mreynolds389
force-pushed
the
issue5267
branch
from
2cf07dd
to
54b4379
Compare
|
So I did a test with MODDN of a subtree with 100K entries. There is a performance drop, as expected, because we need to update "dsEntryDN" in all the children. I see 35% drop in performance. 100K subtree move goes from 12 seconds to 19 seconds. While this is a big drop, subtree moves should be very rare, and IMHO the gain we get in searches is worth it. |
mreynolds389
force-pushed
the
issue5267
branch
2 times, most recently
from
437ca28
to
17ec97e
Compare
|
Last question @progier389 @tbordaz Do we enable this by default? Support thinks we should, but what about us? |
|
Since it has a positive impact on the search performance my feeling is that we should better enable it by default |
|
@tbordaz So long as the "upgrade" is clean and doesn't require user intervention. Remember we don't do update-ds.pl anymore, so this has to work to upgrade from version X to Y without any user intervention. |
Well to fully utilize it you need to reimport the database, but nothing will break if the new attribute it not present in the entry. What you can NOT do is downgrade as the schema will be different but we don't support downgrades anyway. |
|
Okay, so long as it "just works" on upgrades, then that's fine by me. And yep, no downgrade is no issue IMO. |
My only concern is about the change of returned DN. All current 389ds clients are use to receive transformed DN (normalized) the risk would be that some clients could rely on that normalization. If that is the case, then they can disable the feature. Assuming the feature ran for some time and some entries do contain dsentryDN, if we disable the feature it will return the normalized DN (from entryrdn) ignoring dsentryDN. Am I correct ? |
This was my concern as well...
Correct, if the feature is disabled then the entyrdn index is used to generate the DN (See the change in id2entry.c) |
Description: For the Full unnormalized DN in the entry via "dsEntryDN" operational attribute. This allows for maintaining the case of a DN from when it was first added. There is also a significant performance improvement when loading an entry from disk to the entry cache (using entryrdn index was very exspensive). Added config setting to turn this behavior on and off (default on) relates: 389ds#5267 Reviewed by: firstyear, progier, tbordaz, and spichugi(Thanks!!!!)
mreynolds389
force-pushed
the
issue5267
branch
from
17ec97e
to
f08a415
Compare
While testing "nsslapd-subtree-rename-switch=off" there was noticeable performance improvement. The current implementation generates the DN dynamically by using the entryrdn attribute in the database record, and then obtaining the parent portion of the DN from other database indexes.
Disabling subtree-rename (nsslapd-subtree-rename-switch=off) improves throughput/response_time performance especially when the number of clients increases.
The range of improvement on highest load (high number of clients) ranges up to ~10%
This PR brings in the short-cut of already having a DN, and not needing to call entryrdn_lookup_dn() which is very expensive. This shows a 35% improvement when moving entries from disk into the entry cache, There is also an improvement once the cache is primed, but a full performance test run is needed. Regardless this definitely provides a nice perf boost...
Note - this new attribute "dsEntryDN" stores the DN in the exact case that was used when the entry was added. This helps with migrations from ODSEE.
#5267