LDAP optimization and fix for one small bug caused huge performance leak.

Dashboard's benchmarks showed overall x22 boost in page request completion time.

nova/auth/ldapdriver.py

@@ -24,6 +24,7 @@ other backends by creating another class that exposes the same
 public methods.
 """
 
+import functools
 import sys
 
 from nova import exception
@@ -68,6 +69,12 @@ flags.DEFINE_string('ldap_developer',
 LOG = logging.getLogger("nova.ldapdriver")
 
 
+if FLAGS.memcached_servers:
+    import memcache
+else:
+    from nova import fakememcache as memcache
+
+
 # TODO(vish): make an abstract base class with the same public methods
 #             to define a set interface for AuthDrivers. I'm delaying
 #             creating this now because I'm expecting an auth refactor
@@ -85,6 +92,7 @@ def _clean(attr):
 
 def sanitize(fn):
     """Decorator to sanitize all args"""
+    @functools.wraps(fn)
     def _wrapped(self, *args, **kwargs):
         args = [_clean(x) for x in args]
         kwargs = dict((k, _clean(v)) for (k, v) in kwargs)
@@ -103,29 +111,56 @@ class LdapDriver(object):
     isadmin_attribute = 'isNovaAdmin'
     project_attribute = 'owner'
     project_objectclass = 'groupOfNames'
+    conn = None
+    mc = None
 
     def __init__(self):
         """Imports the LDAP module"""
         self.ldap = __import__('ldap')
-        self.conn = None
         if FLAGS.ldap_schema_version == 1:
             LdapDriver.project_pattern = '(objectclass=novaProject)'
             LdapDriver.isadmin_attribute = 'isAdmin'
             LdapDriver.project_attribute = 'projectManager'
             LdapDriver.project_objectclass = 'novaProject'
+        self.__cache = None
+        if LdapDriver.conn is None:
+            LdapDriver.conn = self.ldap.initialize(FLAGS.ldap_url)
+            LdapDriver.conn.simple_bind_s(FLAGS.ldap_user_dn,
+                                          FLAGS.ldap_password)
+        if LdapDriver.mc is None:
+            LdapDriver.mc = memcache.Client(FLAGS.memcached_servers, debug=0)
 
     def __enter__(self):
-        """Creates the connection to LDAP"""
+        # TODO(yorik-sar): Should be per-request cache, not per-driver-request
-        self.conn = self.ldap.initialize(FLAGS.ldap_url)
+        self.__cache = {}
-        self.conn.simple_bind_s(FLAGS.ldap_user_dn, FLAGS.ldap_password)
         return self
 
     def __exit__(self, exc_type, exc_value, traceback):
-        """Destroys the connection to LDAP"""
+        self.__cache = None
-        self.conn.unbind_s()
         return False
 
+    def __local_cache(key_fmt):
+        """Wrap function to cache it's result in self.__cache.
+        Works only with functions with one fixed argument.
+        """
+        def do_wrap(fn):
+            @functools.wraps(fn)
+            def inner(self, arg, **kwargs):
+                cache_key = key_fmt % (arg,)
+                try:
+                    res = self.__cache[cache_key]
+                    LOG.debug('Local cache hit for %s by key %s' %
+                              (fn.__name__, cache_key))
+                    return res
+                except KeyError:
+                    res = fn(self, arg, **kwargs)
+                    self.__cache[cache_key] = res
+                    return res
+            return inner
+        return do_wrap
+
     @sanitize
+    @__local_cache('uid_user-%s')
     def get_user(self, uid):
         """Retrieve user by id"""
         attr = self.__get_ldap_user(uid)
@@ -134,15 +169,31 @@ class LdapDriver(object):
     @sanitize
     def get_user_from_access_key(self, access):
         """Retrieve user by access key"""
+        cache_key = 'uak_dn_%s' % (access,)
+        user_dn = self.mc.get(cache_key)
+        if user_dn:
+            user = self.__to_user(
+                self.__find_object(user_dn, scope=self.ldap.SCOPE_BASE))
+            if user:
+                if user['access'] == access:
+                    return user
+                else:
+                    self.mc.set(cache_key, None)
         query = '(accessKey=%s)' % access
         dn = FLAGS.ldap_user_subtree
-        return self.__to_user(self.__find_object(dn, query))
+        user_obj = self.__find_object(dn, query)
+        user = self.__to_user(user_obj)
+        if user:
+            self.mc.set(cache_key, user_obj['dn'][0])
+        return user
 
     @sanitize
+    @__local_cache('pid_project-%s')
     def get_project(self, pid):
         """Retrieve project by id"""
-        dn = self.__project_to_dn(pid)
+        dn = self.__project_to_dn(pid, search=False)
-        attr = self.__find_object(dn, LdapDriver.project_pattern)
+        attr = self.__find_object(dn, LdapDriver.project_pattern,
+                                  scope=self.ldap.SCOPE_BASE)
         return self.__to_project(attr)
 
     @sanitize
@@ -395,6 +446,7 @@ class LdapDriver(object):
         """Check if project exists"""
         return self.get_project(project_id) is not None
 
+    @__local_cache('uid_attrs-%s')
     def __get_ldap_user(self, uid):
         """Retrieve LDAP user entry by id"""
         dn = FLAGS.ldap_user_subtree
@@ -426,12 +478,20 @@ class LdapDriver(object):
         if scope is None:
             # One of the flags is 0!
             scope = self.ldap.SCOPE_SUBTREE
+        if query is None:
+            query = "(objectClass=*)"
         try:
             res = self.conn.search_s(dn, scope, query)
         except self.ldap.NO_SUCH_OBJECT:
             return []
         # Just return the attributes
-        return [attributes for dn, attributes in res]
+        # FIXME(yorik-sar): Whole driver should be refactored to
+        #                   prevent this hack
+        res1 = []
+        for dn, attrs in res:
+            attrs['dn'] = [dn]
+            res1.append(attrs)
+        return res1
 
     def __find_role_dns(self, tree):
         """Find dns of role objects in given tree"""
@@ -564,6 +624,7 @@ class LdapDriver(object):
             'description': attr.get('description', [None])[0],
             'member_ids': [self.__dn_to_uid(x) for x in member_dns]}
 
+    @__local_cache('uid_dn-%s')
     def __uid_to_dn(self, uid, search=True):
         """Convert uid to dn"""
         # By default return a generated DN
@@ -576,6 +637,7 @@ class LdapDriver(object):
                 userdn = user[0]
         return userdn
 
+    @__local_cache('pid_dn-%s')
     def __project_to_dn(self, pid, search=True):
         """Convert pid to dn"""
         # By default return a generated DN
@@ -603,16 +665,18 @@ class LdapDriver(object):
         else:
             return None
 
+    @__local_cache('dn_uid-%s')
     def __dn_to_uid(self, dn):
         """Convert user dn to uid"""
         query = '(objectclass=novaUser)'
-        user = self.__find_object(dn, query)
+        user = self.__find_object(dn, query, scope=self.ldap.SCOPE_BASE)
         return user[FLAGS.ldap_user_id_attribute][0]
 
 
 class FakeLdapDriver(LdapDriver):
     """Fake Ldap Auth driver"""
 
-    def __init__(self):  # pylint: disable=W0231
+    def __init__(self):
-        __import__('nova.auth.fakeldap')
+        import nova.auth.fakeldap
-        self.ldap = sys.modules['nova.auth.fakeldap']
+        sys.modules['ldap'] = nova.auth.fakeldap
+        super(FakeLdapDriver, self).__init__()

nova/auth/manager.py

@@ -73,6 +73,12 @@ flags.DEFINE_string('auth_driver', 'nova.auth.dbdriver.DbDriver',
 LOG = logging.getLogger('nova.auth.manager')
 
 
+if FLAGS.memcached_servers:
+    import memcache
+else:
+    from nova import fakememcache as memcache
+
+
 class AuthBase(object):
     """Base class for objects relating to auth
 
@@ -206,6 +212,7 @@ class AuthManager(object):
     """
 
     _instance = None
+    mc = None
 
     def __new__(cls, *args, **kwargs):
         """Returns the AuthManager singleton"""
@@ -222,13 +229,8 @@ class AuthManager(object):
         self.network_manager = utils.import_object(FLAGS.network_manager)
         if driver or not getattr(self, 'driver', None):
             self.driver = utils.import_class(driver or FLAGS.auth_driver)
-
+        if AuthManager.mc is None:
-        if FLAGS.memcached_servers:
+            AuthManager.mc = memcache.Client(FLAGS.memcached_servers, debug=0)
-            import memcache
-        else:
-            from nova import fakememcache as memcache
-        self.mc = memcache.Client(FLAGS.memcached_servers,
-                                  debug=0)
 
     def authenticate(self, access, signature, params, verb='GET',
                      server_string='127.0.0.1:8773', path='/',

nova/tests/test_auth.py

@@ -86,6 +86,7 @@ class _AuthManagerBaseTestCase(test.TestCase):
         super(_AuthManagerBaseTestCase, self).setUp()
         self.flags(connection_type='fake')
         self.manager = manager.AuthManager(new=True)
+        self.manager.mc.cache = {}
 
     def test_create_and_find_user(self):
         with user_generator(self.manager):