[PATCH] Use idmap cache for all pdb backends.

[Andrew Bartlett]

On Fri, 2014-05-02 at 15:38 +0200, Alexander Werth wrote:
> On Wed, 2014-04-30 at 15:46 -0700, Christof Schmitt wrote:
> > On Wed, Apr 30, 2014 at 04:15:38PM +0200, Alexander Werth wrote:
> > > On Wed, 2014-04-30 at 15:51 +0200, Volker Lendecke wrote:
> > > > On Wed, Apr 30, 2014 at 03:16:45PM +0200, Alexander Werth wrote:
> > > > > Currently only the passdb ldapsam backend uses the idmap cache.
> > > > > And even that only uses the cache when the option ldapsam:trusted is set.
> > > > > By moving the set cache entry calls up one layer all passdb backends
> > > > > will use the idmap cache.
> > > > > Without the idmap cache any id<->sid mapping request from ACLs for
> > > > > example would result in another set of ldap queries. Up to the point
> > > > > where the LDAP would be overloaded.
> > > > 
> > > > Looks good to me, thanks. When compiling I now get
> > > > 
> > > > [2299/4156] Compiling source3/passdb/pdb_ldap.c
> > > > ../source3/passdb/pdb_ldap.c: In function ‘ldapsam_uid_to_sid’:
> > > > ../source3/passdb/pdb_ldap.c:5014:16: warning: variable ‘id’ set but not used [-Wunused-but-set-variable]
> > > > ../source3/passdb/pdb_ldap.c: In function ‘ldapsam_gid_to_sid’:
> > > > ../source3/passdb/pdb_ldap.c:5087:16: warning: variable ‘id’ set but not used [-Wunused-but-set-variable]
> > > > 
> > > > It looks that if the "id" variable can be removed in both
> > > > cases. Do you want me to do this, or do you want to send a
> > > > new patch?
> > > 
> > > Thanks, for the quick review.
> > > And I'm adding a mental note to check for compiler warnings.
> > > I've removed the unused variable and updated the patch.
> > > 
> > > Cheers,
> > > Alexander
> > 
> > > >From ad56d1e18450237b906f41179f13ca42fd8d1aad Mon Sep 17 00:00:00 2001
> > > From: Alexander Werth <alexander.werth at de.ibm.com>
> > > Date: Fri, 25 Apr 2014 13:53:48 +0200
> > > Subject: [PATCH] s3: Always cache idmapping results of pdb backend.
> > > 
> > > And don't cache in the pdb_ldap module on the id_to_sid calls.
> > > 
> > > Signed-off-by: Alexander Werth <alexander.werth at de.ibm.com>
> > > ---
> > >  source3/passdb/pdb_interface.c | 19 ++++++++++++++++++-
> > >  source3/passdb/pdb_ldap.c      | 14 --------------
> > >  2 files changed, 18 insertions(+), 15 deletions(-)
> > 
> > Why are you removing the caching in pdb_ldap? As far as i can see,
> > ldapsam_sid_to_id, ldapsam_uid_to_sid and ldapsam_gid_to_sid do not call
> > the default methods, so the cache calls should probably be kept in the
> > ldapsam functions.
> 
> Right, I meant to move that call up one level but didn't.
> What about this version of the patch?
> I've added the cache call right into pdb_sid_to_id, pdb_uid_to_sid, and
> pdb_gid_to_sid which are calling the backend specific methods.

Sorry to be a pin, but not all callers go via the pdb_ functions.  The
Python passdb API (for example) does not, it calls the backends directly
via the methods pointers.

Also, can you explain a bit more why we should cache for the
pdb_smbpasswd and pdb_tdbsam modules?  Caches naturally bring up issues
of cache coherency, and we should only use them when looking up the
result has a significant cost.  You imply that some situation using
other than pdb_ldapsam calls LDAP, can you provide some more detail on
that?

Thanks,

Andrew Bartlett

-- 
Andrew Bartlett                       http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba