Faster id mapping lookup

id_mapping_api was designed to make a query per entity to fetch
public ids. This lead to a very poor performance when there were many
entries in LDAP. For example, for 15k entries 15k MySQL queries were
required. For the first run 15k INSERTs were required, which makes
things even worse.

Change this behavior to fetch related mappings from MySQL as a list and
perform the necessary join in-memory.

bp ldap-preprocessing
Partial-Bug: 1582585
Change-Id: I2c266e91f2f05be760f8a3eea3738868243cc9c6

keystone/identity/core.py

@@ -573,8 +573,8 @@ class Manager(manager.Manager):
             return self._set_domain_id_and_mapping_for_single_ref(
                 ref, domain_id, driver, entity_type, conf)
         elif isinstance(ref, list):
-            return [self._set_domain_id_and_mapping(
-                    x, domain_id, driver, entity_type) for x in ref]
+            return self._set_domain_id_and_mapping_for_list(
+                ref, domain_id, driver, entity_type, conf)
         else:
             raise ValueError(_('Expected dict or list: %s') % type(ref))
 
@@ -583,6 +583,16 @@ class Manager(manager.Manager):
         return (driver is not self.driver or not driver.generates_uuids() or
                 not driver.is_domain_aware())
 
+    def _insert_new_public_id(self, local_entity, ref, driver):
+        # Need to create a mapping. If the driver generates UUIDs
+        # then pass the local UUID in as the public ID to use.
+        public_id = None
+        if driver.generates_uuids():
+            public_id = ref['id']
+        ref['id'] = self.id_mapping_api.create_id_mapping(
+            local_entity, public_id)
+        LOG.debug('Created new mapping to public ID: %s', ref['id'])
+
     def _set_domain_id_and_mapping_for_single_ref(self, ref, domain_id,
                                                   driver, entity_type, conf):
         LOG.debug('Local ID: %s', ref['id'])
@@ -600,16 +610,60 @@ class Manager(manager.Manager):
                 LOG.debug('Found existing mapping to public ID: %s',
                           ref['id'])
             else:
-                # Need to create a mapping. If the driver generates UUIDs
-                # then pass the local UUID in as the public ID to use.
-                if driver.generates_uuids():
-                    public_id = ref['id']
-                ref['id'] = self.id_mapping_api.create_id_mapping(
-                    local_entity, public_id)
-                LOG.debug('Created new mapping to public ID: %s',
-                          ref['id'])
+                self._insert_new_public_id(local_entity, ref, driver)
         return ref
 
+    def _set_domain_id_and_mapping_for_list(self, ref_list, domain_id, driver,
+                                            entity_type, conf):
+        """Set domain id and mapping for a list of refs.
+
+        The method modifies refs in-place.
+        """
+        if not ref_list:
+            return []
+
+        for r in ref_list:
+            self._insert_domain_id_if_needed(r, driver, domain_id, conf)
+
+        if not self._is_mapping_needed(driver):
+            return ref_list
+
+        # build a map of refs for fast look-up
+        refs_map = {}
+        for r in ref_list:
+            refs_map[(r['id'], entity_type, r['domain_id'])] = r
+
+        # NOTE(breton): there are cases when the driver is not domain aware and
+        # no domain_id was explicitely provided for list operation. domain_id
+        # gets inserted into refs, but not passed into this method. Lets use
+        # domain_id from one of the refs.
+        if not domain_id:
+            domain_id = ref_list[0]['domain_id']
+
+        # fetch all mappings for the domain, lookup the user at the map built
+        # at previous step and replace his id.
+        domain_mappings = self.id_mapping_api.get_domain_mapping_list(
+            domain_id)
+        for _mapping in domain_mappings:
+            idx = (_mapping.local_id, _mapping.entity_type, _mapping.domain_id)
+            try:
+                ref = refs_map.pop(idx)
+                # due to python specifics, `ref` still points to an item in
+                # `ref_list`. That's why when we change it here, it gets
+                # changed in `ref_list`.
+                ref['id'] = _mapping.public_id
+            except KeyError:
+                pass  # some old entry, skip it
+
+        # at this point, all known refs were granted a public_id. For the refs
+        # left, there are no mappings. They need to be created.
+        for ref in refs_map.values():
+            local_entity = {'domain_id': ref['domain_id'],
+                            'local_id': ref['id'],
+                            'entity_type': entity_type}
+            self._insert_new_public_id(local_entity, ref, driver)
+        return ref_list
+
     def _insert_domain_id_if_needed(self, ref, driver, domain_id, conf):
         """Insert the domain ID into the ref, if required.
 

keystone/identity/mapping_backends/base.py

@@ -34,6 +34,15 @@ class MappingDriverV8(object):
         """
         raise exception.NotImplemented()  # pragma: no cover
 
+    @abc.abstractmethod
+    def get_domain_mapping_list(self, domain_id):
+        """Return mappings for the domain.
+
+        :param domain_id: Domain ID to get mappings for.
+        :returns: list of mappings.
+        """
+        raise exception.NotImplemented()  # pragma: no cover
+
     @abc.abstractmethod
     def get_id_mapping(self, public_id):
         """Return the local mapping.

keystone/identity/mapping_backends/sql.py

@@ -57,6 +57,10 @@ class Mapping(base.MappingDriverV8):
             except sql.NotFound:
                 return None
 
+    def get_domain_mapping_list(self, domain_id):
+        with sql.session_for_read() as session:
+            return session.query(IDMapping).filter_by(domain_id=domain_id)
+
     def get_id_mapping(self, public_id):
         with sql.session_for_read() as session:
             mapping_ref = session.query(IDMapping).get(public_id)

keystone/tests/unit/test_backend_id_mapping_sql.py

@@ -290,3 +290,45 @@ class SqlIDMapping(test_backend_sql.SqlTests):
         # Purge mappings the remaining mappings
         self.id_mapping_api.purge_mappings({})
         self.assertIsNone(self.id_mapping_api.get_public_id(local_entity5))
+
+    def test_get_domain_mapping_list(self):
+        local_id1 = uuid.uuid4().hex
+        local_id2 = uuid.uuid4().hex
+        local_id3 = uuid.uuid4().hex
+        local_id4 = uuid.uuid4().hex
+        local_id5 = uuid.uuid4().hex
+
+        # Create five mappings,two in domainA, three in domainB
+        local_entity1 = {'domain_id': self.domainA['id'],
+                         'local_id': local_id1,
+                         'entity_type': mapping.EntityType.USER}
+        local_entity2 = {'domain_id': self.domainA['id'],
+                         'local_id': local_id2,
+                         'entity_type': mapping.EntityType.USER}
+        local_entity3 = {'domain_id': self.domainB['id'],
+                         'local_id': local_id3,
+                         'entity_type': mapping.EntityType.GROUP}
+        local_entity4 = {'domain_id': self.domainB['id'],
+                         'local_id': local_id4,
+                         'entity_type': mapping.EntityType.USER}
+        local_entity5 = {'domain_id': self.domainB['id'],
+                         'local_id': local_id5,
+                         'entity_type': mapping.EntityType.USER}
+
+        local_entity1['public_id'] = self.id_mapping_api.create_id_mapping(
+            local_entity1)
+        local_entity2['public_id'] = self.id_mapping_api.create_id_mapping(
+            local_entity2)
+        local_entity3['public_id'] = self.id_mapping_api.create_id_mapping(
+            local_entity3)
+        local_entity4['public_id'] = self.id_mapping_api.create_id_mapping(
+            local_entity4)
+        local_entity5['public_id'] = self.id_mapping_api.create_id_mapping(
+            local_entity5)
+
+        # list mappings for domainA
+        domain_a_mappings = self.id_mapping_api.get_domain_mapping_list(
+            self.domainA['id'])
+        domain_a_mappings = [m.to_dict() for m in domain_a_mappings]
+        self.assertItemsEqual([local_entity1, local_entity2],
+                              domain_a_mappings)

keystone/tests/unit/test_backend_ldap.py

@@ -3121,6 +3121,23 @@ class DomainSpecificLDAPandSQLIdentity(
                 domain_scope=self.domains['domain1']['id']),
             matchers.HasLength(1))
 
+    def test_get_domain_mapping_list_is_used(self):
+        # before get_domain_mapping_list was introduced, it was required to
+        # make N calls to the database for N users, and it was slow.
+        # get_domain_mapping_list solves this problem and should be used
+        # when multiple users are fetched from domain-specific backend.
+        for i in range(5):
+            unit.create_user(self.identity_api,
+                             domain_id=self.domains['domain1']['id'])
+
+        with mock.patch.multiple(self.id_mapping_api,
+                                 get_domain_mapping_list=mock.DEFAULT,
+                                 get_id_mapping=mock.DEFAULT) as mocked:
+            self.identity_api.list_users(
+                domain_scope=self.domains['domain1']['id'])
+            mocked['get_domain_mapping_list'].assert_called()
+            mocked['get_id_mapping'].assert_not_called()
+
     def test_user_id_comma(self):
         self.skip_test_overrides('Only valid if it is guaranteed to be '
                                  'talking to the fakeldap backend')

releasenotes/notes/domain_mapping_list-a368ac5a252ec84f.yaml

@@ -0,0 +1,6 @@
+---
+upgrade:
+  - ID Mapping driver interface has changed. A new method
+    ``get_domain_mapping_list`` was added for fetching mapping list
+    for a domain. If you have a custom implementation for the identity
+    driver, you will need to implement this new method.