[Date Prev][Date Next] [Chronological] [Thread] [Top]
Re: Some openldap fixes... (fwd) (fwd)

To: openldap-devel@OpenLDAP.org
Subject: Re: Some openldap fixes... (fwd) (fwd)
From: Marijn Meijles <marijn@bitpit.net>
Date: Mon, 18 Sep 2000 21:54:20 +0200
he was not yet subscribed to the list...

----- Forwarded message from peter <peter@twistor.admin.lion-access.net> -----

Date: Mon, 18 Sep 2000 19:46:24 +0200
From: peter <peter@twistor.admin.lion-access.net>
To: Kurt@OpenLDAP.org
Cc: marijn@bitpit.net, openldap-devel@OpenLDAP.org
Subject: Re: Some openldap fixes... (fwd)
User-Agent: Mutt/1.2.4i
In-Reply-To: <20000918022349.C1528@hoop.student.tue.nl>; from marijn@bitpit.net on Mon, Sep 18, 2000 at 02:23:50AM +0200

> >- Added an interface option so you can specify the interface you want to
> >bind.
> 
> Provided in 2.0... (and, IIRC, there's a contributed patch for
> 1.2 in the Issue Tracking System).

yeah,.. but why browse for 10 min. if one can hack the feature in 5 :)

> 
> >- The pagesize for id2entry is now configurable. If you put larger items
> >in the ldap db, you'll want to raise this value, but not necessarily
> >the pagesize for all the db's.
> 
> Knobs are nice... the new backend will be highly configurable...
> (if only I had more time to work on it).
> 
> >- Not included in the patch, but well worth mentioning is the fact that
> >you can put extra yieldpoints into the BDB2 code when using a cooperative
> >mt package. This will greatly enhance responsiveness. If you want more 
> >info on this, please ask.
> 
> 
> >Peter made the rest of the descriptions:
> >
> >- removed NEXTID file
> >        * instead use the value of the last key in the id2entry db
> 
> 2.0 maintains the next id in a DB file.

that's what the fix is all about,.. it's not maintained as such,.. the
information you want (the last seq. number + 1) is allready present
thus keeping a second copy of that number only adds programm complexity
and increases the chance of errors.

> 
> >- removed the explicit dn index
> >        * couldn't find any use, and the program kept working just fine :)
> 
> 1.2 (and 2.0) uses DN indexing to properly scope searches.  Removing
> them have side effects.  2.0 has new DN indexing to improve speed
> (1.2 used substrings indexing which were horible, so disabled by
> default [which meant that you cannot place multiple suffixes in
> one database by defaults).  

there was only 1 reference to that whole index, and that was where it was
created,.. why keep an index if it's never to be read ? trust me,.. 
I 'grep'-ed the whole source tree on this !

inside the butt-ugly (tm) for loop in back-ldbm/search.c i read:

			/* check scope */
			scopeok = 1;
			if ( scope == LDAP_SCOPE_ONELEVEL ) {
					if ( (dn = dn_parent( be, e->e_dn )) != NULL ) {
							(void) dn_normalize_case( dn );
							scopeok = (dn == realBase)
									? 1
									: (strcmp( dn, realBase ) ? 0 : 1 );
							free( dn );
					} else {
							scopeok = (realBase == NULL || *realBase == '\0');
					}
			} else if ( scope == LDAP_SCOPE_SUBTREE ) {
					dn = ch_strdup( e->e_ndn );
					scopeok = dn_issuffix( dn, realBase );
					free( dn );
			}

this enforces the scope. So the candidate tables can contain as many
OOS entries as they want. The trick offcourse it to keep those to a minimum :)

> 
> >- fixed search scopes
> >* modified/removed filter alteration in (onelevel|subtree)_candidates
> >          scope enforcement is done ldbm_back_search anyway.
> 
> DN indices don't enforce scope, they just limit the number of
> candidates you must test.  1.2 indices, especially for subtree
> scoping, were not terrible effective (and is turned off by default).
> 2.0 sports new DN indexing (and is always on).

okay let me be more clear,.. I removed/modified those filters
because they didn't have any added benefit and only slowed stuph down.

> 
> >        * fixed that butt-ugly for-loop in ldbm_back_search
> 
> That's not terribly specific.  The only for loop in ldbm_back_search
> looks fairly reasonable (at least in HEAD, but I don't recall 1.2
> being much different).

okay here the code:
        for ( id = idl_firstid( candidates ); id != NOID;
                  id = idl_nextid( candidates, id ) ) {

looks innocent and quite okay,.. until:

/*  return next ID after id
 *  if ALLIDS block, increment id.
 *      if id < NIDS return id
 *      otherwise NOID.
 *  otherwise SEARCH for next id (ugh!)
 */
ID
idl_nextid( ID_BLOCK *idl, ID id )
{
    unsigned int    i;

    if ( ID_BLOCK_ALLIDS( idl ) ) {
        return( ++id < ID_BLOCK_NIDS(idl) ? id : NOID );
    }

    for ( i = 0; i < ID_BLOCK_NIDS(idl) && ID_BLOCK_ID(idl, i) <= id; i++ ) {
        ;   /* NULL */
    }

    if ( i >= ID_BLOCK_NIDS(idl) ) {
        return( NOID );
    } else {
        return( ID_BLOCK_ID(idl, i) );
    }
}

which to my opinion more that deserves the title butt-ugly (tm)
somehow your own word seem to emphasise this: ugh!

> 
> >        * replaced all calls to idl_allids with give_children
> >
> >        id2children.c::ID_BLOCK * give_children( Beckend *, 
> >                                                 Entry * base, 
> >                                                 int scope) 
> >
> >          users the id2children db to construct a list of all id's within the
> >        specified base/scope pair.
> 
> id2children was replaced additional DN indices in 2.0.  The
> new code supports indices for scope base, one-level, and subtree.

dewd,.. i tried that myself,.. only yields f*** huge databases
the trick is to have only one index that has scope support
an index per scope is too slow/big

the full functionality of this fix is that when no index is found
applicable the candidate list only contains those entries that
are in scope,. not the whole database.

> 
> >- fixed scopelessness of indices:
> 
> attribute assertion indices should be orthogonal to scope.

erruhm,... every search has a scope to it,. or i didn't quite get 
the LDAP specs. why bother with entries that match your filter
if they're outside of your scope ?

someone once told me that you should never touch more data than is
absolutely nessesary,.. and i think he was right,.. it saves time

> 
> >TODO:
> >
> >- remove hard limit on idl block splits, this is VERY annoying when
> >the db gets a bit bigger ;(
> 
> The IDL code needs work.  There are a number of optimizations
> needing to get done such as removal of for loop copies (mostly
> done) and use of qsort()/bsearch() for large blocks.
> 
> I plan to ditch the IDL code in my replacement backend (if duplicate
> keys works well enough).

ow,.. they do,.. just not fast enough :)
I'd keep it like it is,.. only rip out the split part
and make sure that that ALLIDS idea never ever makes it back into
your code :)

> 
> >- rewrite cache in order to avoid one mutex for the whole cache
> 
> I think cache redesign is best left for the replacement backend.
> The new cache will be managed by the database.
> 
> >- rewrite str2entry and entry2str so that they use a number of memory
> >slots. this is to avoid the mutex locking they employ now.
> 
> We have a patch submitted against 2.0 to do this... it needs a
> little work.
> 

regardz,
Peter Zijlstra

----- End forwarded message -----

-- 
Marijn@bitpit.net
---
If at first you don't succeed, destroy all evidence that you tried.
Follow-Ups:
- Re: Some openldap fixes... (fwd) (fwd)
  - From: "Kurt D. Zeilenga" <Kurt@OpenLDAP.org>
Prev by Date: Re: Transactions in LDAP
Next by Date: RE: Transactions in LDAP
Index(es):
- Chronological
- Thread