[Freeipa-devel] [PATCH] new LDAP connection pool
Pete Rowley
prowley at redhat.com
Mon Oct 8 22:31:23 UTC 2007
Rob Crittenden wrote:
ok
> The old connection pool lacked locking and would iterate over the
> entire list for each request.
>
> I've changed it to use a dict of connections keyed on principal.
>
> There is a separate list keyed on principal that holds the order of
> the entries.
>
> When an entry is requested it looks for it in the dictionary and if
> found, pops it off the list too.
>
> When finished, it creates a new dict entry for it then appends it to
> the end of the list (so it is the most recently used).
>
> If we ever have a full list, just pop element 0 as it is the LRU.
>
> Remember that Apache is running multi-process so each process will
> have its own connection cache. This limits the effectiveness of the
> pooling but I suspect that with the GUI it will help somewhat.
>
> This is the reason I've gone with a fairly conservative value for the
> max number of entries at 128.
>
> It seems fairly weak in terms of catching possible errors. Some
> suggestions would be helpful. I don't think it can blow up and leave a
> hanging lock, so it won't hork the server. I also don't want to go
> overboard and put try/except around every call (or should I?)
>
> rob
> ------------------------------------------------------------------------
>
> # HG changeset patch
> # User Rob Crittenden <rcritten at redhat.com>
> # Date 1191874718 14400
> # Node ID dd67f3ae267949f4fc30493448cbd66a566766e3
> # Parent 1f5fc351120318e56161cf2327b46f62a6a98f18
> New LDAP connection pool that does locking
>
> diff -r 1f5fc3511203 -r dd67f3ae2679 ipa-server/ipaserver/ipaldap.py
> --- a/ipa-server/ipaserver/ipaldap.py Fri Oct 05 13:59:04 2007 -0700
> +++ b/ipa-server/ipaserver/ipaldap.py Mon Oct 08 16:18:38 2007 -0400
> @@ -265,10 +265,11 @@ class IPAdmin(SimpleLDAPObject):
> def set_proxydn(self, proxydn):
> self.proxydn = proxydn
>
> - def set_krbccache(self, krbccache):
> + def set_krbccache(self, krbccache, principal):
> if krbccache is not None:
> os.environ["KRB5CCNAME"] = krbccache
> self.sasl_interactive_bind_s("", sasl_auth)
> + self.principal = principal
> self.proxydn = None
>
> def getEntry(self,*args):
> diff -r 1f5fc3511203 -r dd67f3ae2679 ipa-server/xmlrpc-server/funcs.py
> --- a/ipa-server/xmlrpc-server/funcs.py Fri Oct 05 13:59:04 2007 -0700
> +++ b/ipa-server/xmlrpc-server/funcs.py Mon Oct 08 16:18:38 2007 -0400
> @@ -34,6 +34,11 @@ import os
> import os
> import re
>
> +try:
> + from threading import Lock
> +except ImportError:
> + from dummy_threading import Lock
> +
> # Need a global to store this between requests
> _LDAPPool = None
>
> @@ -45,35 +50,68 @@ DefaultGroupContainer = "cn=groups,cn=ac
> # connection. This could theoretically drive the total number of connections
> # very high but since this represents just the administrative interface
> # this is not anticipated.
> +#
> +# The pool consists of two things, a dictionary keyed on the principal name
> +# that contains the connection and a list that is used to keep track of the
> +# order. If the list fills up just pop the top entry off and you've got
> +# the least recently used.
> +
> +# maxsize = 0 means no limit
> class IPAConnPool:
> - def __init__(self):
> - self.freelist = []
> -
> - def getConn(self, host, port, bindca, bindcert, bindkey, proxydn=None, krbccache=None, debug=None):
> + def __init__(self, maxsize = 0):
> + self._dict = {}
> + self._lru = []
> + self._lock = Lock()
> + self._maxsize = maxsize
> + self._ctx = krbV.default_context()
> +
> + def getConn(self, host, port, krbccache=None, debug=None):
> conn = None
> - if len(self.freelist) > 0:
> - for i in range(len(self.freelist)):
> - c = self.freelist[i]
> - if ((c.host == host) and (c.port == port)):
> - conn = self.freelist.pop(i)
> - break
> - if conn is None:
> - conn = ipaserver.ipaldap.IPAdmin(host,port,bindca,bindcert,bindkey,None,debug)
> - if proxydn is not None:
> - conn.set_proxydn(proxydn)
> - else:
> - conn.set_krbccache(krbccache)
> + ccache = krbV.CCache(name=krbccache, context=self._ctx)
> + cprinc = ccache.principal()
> + self._lock.acquire()
> + try:
> + try:
> + conn = self._dict[cprinc.name]
> + del self._dict[cprinc.name]
> + self._lru.remove(cprinc.name)
> + except KeyError:
> + conn = ipaserver.ipaldap.IPAdmin(host,port,None,None,None,debug)
> + conn.set_krbccache(krbccache, cprinc.name)
> + finally:
> + self._lock.release()
> +
> return conn
>
> def releaseConn(self, conn):
> if conn is None:
> return
> - # We can't re-use SASL connections. If proxydn is None it means
> - # we have a Kerberos credentials cache set. See ipaldap.set_krbccache
> - if conn.proxydn is None:
> - conn.unbind_s()
> - else:
> - self.freelist.append(conn)
> +
> + cprinc = conn.principal
> +
> + self._lock.acquire()
> + try:
> +
> + # Look to see if we are already on the list from another source and
> + # if so, remove it.
> + try:
> + c = self._dict[cprinc]
> + del self._dict[cprinc]
> + self._lru.remove(cprinc)
> + except KeyError:
> + # Not in the list
> + pass
> +
> + if self._maxsize and len(self._dict) > self._maxsize:
> + princ = self._lru.pop(0)
> + c = self._dict[princ]
> + c.unbind_s()
> + del self._dict[cprinc]
> +
> + self._lru.append(cprinc)
> + self._dict[cprinc] = conn
> + finally:
> + self._lock.release()
>
> class IPAServer:
>
> @@ -90,7 +128,7 @@ class IPAServer:
> self.realm = self.krbctx.default_realm
>
> if _LDAPPool is None:
> - _LDAPPool = IPAConnPool()
> + _LDAPPool = IPAConnPool(128)
> self.basedn = ipa.ipautil.realm_to_suffix(self.realm)
> self.scope = ldap.SCOPE_SUBTREE
> self.princ = None
> @@ -169,7 +207,7 @@ class IPAServer:
> raise ipaerror.gen_exception(ipaerror.CONNECTION_NO_CCACHE)
>
> try:
> - conn = _LDAPPool.getConn(self.host,port,bindca,bindcert,bindkey,proxy_dn,krbccache,debug)
> + conn = _LDAPPool.getConn(self.host,port,krbccache,debug)
> except ldap.INVALID_CREDENTIALS, e:
> raise ipaerror.gen_exception(ipaerror.CONNECTION_GSSAPI_CREDENTIALS, nested_exception=e)
>
>
> ------------------------------------------------------------------------
>
> _______________________________________________
> Freeipa-devel mailing list
> Freeipa-devel at redhat.com
> https://www.redhat.com/mailman/listinfo/freeipa-devel
--
Pete
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 3241 bytes
Desc: S/MIME Cryptographic Signature
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20071008/324d6f74/attachment.bin>
More information about the Freeipa-devel
mailing list