[Date Prev][Date Next] [Chronological] [Thread] [Top]

Re: (ITS#3432) back-sql enhancements

> during the xmas vacations I've been able to work on incorporating your 
> patches.

Thanks for taking the time to look at them.

> 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.

This patch became critical to us here at CMU when we started using groups. 
Some of our group objects have thousands of members, and when slapd tried 
loading one in from the RDBM it would render the LDAP server unusable. Anytime 
you have two memory buffers that you are growing by realloc'ing them back and 
forth they are going to start leapfrogging through memory until the process 
crashes.  The frontend attr_merge() doesn't know how many values will be 
coming later on, so the backend(s) cannot rely on it when an unknown number of 
values, possibly thousands, are going to be loaded in.  back-bdb (and some 
others) don't face this problem since they're loading a single hunk off disk.
   Try creating a group with 10,000 members and then try to load it with 
   The 4.count_query patch seemed like a reasonable way to fix the leapfrog 
issue.  Another possibility is to load all the values and THEN, afterwards, 
normalize them all, so that you don't have two growing memory segments 
alternating through realloc().

> 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().

I had to use avl_find_lin() in that one case because the functions calling the 
taint() function don't always have the *Operation available, which is a part 
of the key used in the AVL.  You'll note the comparison function used with 
avl_find_lin() is different from the function used in all the avl_find() 
calls.  Since the entire AVL key is not available, the entire AVL tree has to 
be searched to find any node whose "dbh" matches that given to the taint() 

> 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 :)

There may be something I don't understand about the LDAP protocol, then. Is it 
possible, or not, to change the objectClass of an entry?

An LDAP database has an entry "cn=adamson,dc=cmu,dc=edu"  of objectClass 
"posixAccount", which has a subclass "cmuAccount" and the entry already 
matches all of the MAY and MUST directives of the schema for both cmuAccount 
and posixAccount.  Now the server receives this LDIF:

   dn: cn=adamson,dc=cmu,dc=edu
   changetype: modify
   replace: objectClass
   objectClass: cmuAccount

what should the server do?

This patch #7 attempts to make the necessary changes in the RDBM to change the 
objectClass of the entry.  If LDAP says that entries cannot change OC, then 
yeah this is a bad patch.

    Thanks again for your time.

        -Mark Adamson
         Carnegie Mellon