Re: (ITS#3432) back-sql enhancements

[ando@sys-net.it]

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

well, I do object to your approach essentially because of the code
duplication and because it currently doesn't handle the case of an
attribute being already present in the entry, resulting in the need to
call someting like modify_add_values(), possibly with (permissive == TRUE)
to avoid duplications.  In that case I'd suggest that both normalize
__AND__ pretty be called on the returned values, much like it's done with
back-ldap.  I'll look at it a bit more.

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

In fact, I'd rather add an ldap_opid member to the backsql_db_conn structure.

>
>
>> 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?
>
> Example:
> 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 should rejected as an attempt to change the entry's
structuralObjectClass.  According to RFC 2251:


3.2.1. Attributes of Entries
                                            ... Servers may restrict the
   modifications of this attribute to prevent the basic structural class
   of the entry from being changed (e.g. one cannot change a person into
   a country).

Moreover, draft-ietf-ldapbis-models-12 states that

2.4.2. Structural Object Classes
        ...
        The structural object class of an entry shall not be changed.
        ...

  Servers which follow X.500(93) models SHALL restrict modifications of
  this attribute to prevent the basic structural class of the entry from
  being changed.  That is, one cannot change a 'person' into a
  'country'.

I understand that changing an entry's structuralObjectClass into something
that's a subclass of it sounds like going into no man's land, but I think
this is the current interpretation of the specs (this is how slapd
currently behaves).

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

Stay tuned for further fixes, and please test those that are already in.

Cheers, p.

-- 
Pierangelo Masarati
mailto:pierangelo.masarati@sys-net.it


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