Re: (ITS#3432) back-sql enhancements

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