hyc@OpenLDAP.org wrote:
Update of /repo/OpenLDAP/pkg/ldap/servers/slapd/back-bdb
Modified Files: add.c 1.160 -> 1.161 cache.c 1.126 -> 1.127
Log Message: Tweak bei_state so cache_lru_add doesn't ever try to free just-added entries. This allows us to use the frontend's entry directly instead of having to entry_dup it before adding to the cache.
I remember regretting having to use entry_dup here in the first place, 'way back when. Removing it shaves another minute off my test; the ldapadd that took an hour and 33 minutes now completes in just 5 minutes.
Howard Chu wrote:
hyc@OpenLDAP.org wrote:
Update of /repo/OpenLDAP/pkg/ldap/servers/slapd/back-bdb
Modified Files: add.c 1.160 -> 1.161 cache.c 1.126 -> 1.127
Log Message: Tweak bei_state so cache_lru_add doesn't ever try to free just-added entries. This allows us to use the frontend's entry directly instead of having to entry_dup it before adding to the cache.
I remember regretting having to use entry_dup here in the first place, 'way back when. Removing it shaves another minute off my test; the ldapadd that took an hour and 33 minutes now completes in just 5 minutes.
Of course this change affected a few other modules (ITS#4767, #4768). The entry in op->ora_e used to always be owned by the caller of be_add, so the caller would either free it unconditionally to cleanup or just allocate it from local/temporary memory. Now back-bdb/hdb takes ownership of the entry and zeroes out op->ora_e before returning, so the caller must always provide a normally allocated entry. The caller must also check for op->ora_e being non-NULL instead of freeing it unconditionally.
Currently the patches I wrote for the two ITS's instead check if op->ora_e == the orignally passed in entry, on the assumption that a response callback might change the entry after it's been added to the back-bdb entry cache. Frankly I can't think of any good reason for an overlay to change things at that point in time, so just checking for NULL ought to be sufficient.
Howard Chu wrote:
Of course this change affected a few other modules (ITS#4767, #4768). The entry in op->ora_e used to always be owned by the caller of be_add, so the caller would either free it unconditionally to cleanup or just allocate it from local/temporary memory. Now back-bdb/hdb takes ownership of the entry and zeroes out op->ora_e before returning, so the caller must always provide a normally allocated entry. The caller must also check for op->ora_e being non-NULL instead of freeing it unconditionally.
Currently the patches I wrote for the two ITS's instead check if op->ora_e == the orignally passed in entry, on the assumption that a response callback might change the entry after it's been added to the back-bdb entry cache. Frankly I can't think of any good reason for an overlay to change things at that point in time, so just checking for NULL ought to be sufficient.
I think it should be explicitly forbidden. For anything else, an overlay should replace an entry it's not allowed to modify, and modify the copy, under the assumption someone else (the overlay's cleanup handler?) will free the copy...
p.
-
Howard Chu -
Pierangelo Masarati