Re: (ITS#3432) back-sql enhancements

[ando@sys-net.it]

Mark,

during the xmas vacations I've been able to work on incorporating your 
patches.  Unfortunately there were some significant differences between 
2.2.18 and HEAD (I didn't realize they were so much) and since this work 
led to some review of HEAD code as well, I decided to go ahead and 
manually apply those changes that wouldn't apply by simply unsing patch 
and, in the meanwhile, apply my corrections accordingly.  Following your 
description of the work:

1.id_query.patch
2.shortcut.patch
3.create_hint.patch
4.count_query.patch
5.returncodes.patch
6.connpool.patch
7.modoc.patch
8.miscfixes.patch

I applied patches 1, 2, 3, 5 and 8 with as little changes as required to 
harmonize with the status of HEAD code.  I'm about to commit those 
changes, so that you can see the outcome.  The remaining patches need 
some thought yet.

4: I'm oriented toward rejecting it at the moment, because first of all 
I'm not really convinced about its need: all slapd is based on using 
attr_merge() and its variations about normalization; note that in 
back-sql we're passing the memory context to the normalization routines, 
so temporary memory shouldn't really be an issue.  We could save some 
temporaries more by using the memory context throughout creating entries 
from the fetched results, but I wouldn't muck with entry creation and 
attribute population, otherwise we really risk to duplicate code too 
much.  I note that back-ldap is also creating attributes bypassing the 
attr_merge() routines as well, but in that case back-ldap can obtain all 
the reaults at once, while back-sql allows different queries to populate 
the same attribute, so at some point we need to realloc something inside 
backsql_entry_addattr().  To make it short: we need to heavily rework 
the patch, to handle this case, so we need to decide if we really want 
to build e_attrs straightforwardly, bypassing the frontend helpers.

6: I didn't apply this yet because I want to study it more deeply, since 
it may heavily impact the code.  One thing I don't understand is why you 
use avl_find_lin() instead of avl_find().

7: I'm not sure about the need for this patch: all backends require to 
delete an entry if the structuralObjectClass needs be changed.  And, in 
case, I think there's something I didn't understand about the whole 
procedure, but this might be my fault :)

Happy new year, p.

adamson@andrew.cmu.edu wrote:

>Full_Name: Mark Adamson
>Version: 2.2.18
>OS: Solaris
>URL: http://nil.andrew.cmu.edu/openldap
>Submission from: (NULL) (128.2.123.80)
>
>
>As I mentioned on the openldap-devel mailing list, over the last 2 years or so I
>
>have made several enhancements to the SQL backend to OpenLDAP.  Our server is
>now
>running very stable in production, so I would like to offer these changes back
>to
>the OpenLDAP source repository. 
>At the web server where the patches reside there is a "summary.txt" file which
>gives a synopsis of the 8 patch files.  The patches include:
>
>* adding configuration options
>* added an optimization for searches where the searchbase = the root DSE
>* added an option to pass one attr=val pair to the create_proc, for RDBM
>indexing
>* count the number of values for an attribute before loading them, to prevent
>massive memory fragmentation when loading large entries
>* interpret return codes from SQL functions as LDAP error codes
>* added RDBM connection pooling
>* add ability to change the objectClass of an entry
>* misc bug fixes
>
>I hope you find these enhancements valuable.
>
>
>   -Mark Adamson
>    Carnegie Mellon
>
>
>  
>





    SysNet - via Dossi,8 27100 Pavia Tel: +390382573859 Fax: +390382476497