Filter by entity_type in get_domain_mapping_list

with many users and groups in a domain fetching all mappings (for both
users and groups) may become inefficient.

In an environment with approx 125k users and 150 groups in the mapping
table and SAML2+LDAP auth/backend, this patch reduced the time
for first (uncached) 'openstack token issue' command from 12 to 3 seconds.
Similar improvements were seen with time to login to Horizon as well.

Change-Id: Iccbef534ff7e723f8b1461bb1169e2da66cc1dea
Closes-Bug: #1775207

keystone/identity/core.py

@@ -676,7 +676,7 @@ class Manager(manager.Manager):
         # fetch all mappings for the domain, lookup the user at the map built
         # at previous step and replace his id.
         domain_mappings = PROVIDERS.id_mapping_api.get_domain_mapping_list(
-            domain_id)
+            domain_id, entity_type=entity_type)
         for _mapping in domain_mappings:
             idx = (_mapping.local_id, _mapping.entity_type, _mapping.domain_id)
             try:

keystone/identity/mapping_backends/base.py

@@ -36,10 +36,13 @@ class MappingDriverBase(provider_api.ProviderAPIMixin, object):
         raise exception.NotImplemented()  # pragma: no cover
 
     @abc.abstractmethod
-    def get_domain_mapping_list(self, domain_id):
+    def get_domain_mapping_list(self, domain_id, entity_type=None):
         """Return mappings for the domain.
 
         :param domain_id: Domain ID to get mappings for.
+        :param entity_type: Optional entity_type to get mappings for.
+        :type entity_type: String, one of mappings defined in
+            keystone.identity.mapping_backends.mapping.EntityType
         :returns: list of mappings.
         """
         raise exception.NotImplemented()  # pragma: no cover

keystone/identity/mapping_backends/sql.py

@@ -55,9 +55,12 @@ class Mapping(base.MappingDriverBase):
             except sql.NotFound:
                 return None
 
-    def get_domain_mapping_list(self, domain_id):
+    def get_domain_mapping_list(self, domain_id, entity_type=None):
+        filters = {'domain_id': domain_id}
+        if entity_type is not None:
+            filters['entity_type'] = entity_type
         with sql.session_for_read() as session:
-            return session.query(IDMapping).filter_by(domain_id=domain_id)
+            return session.query(IDMapping).filter_by(**filters)
 
     def get_id_mapping(self, public_id):
         with sql.session_for_read() as session:

keystone/tests/unit/test_backend_id_mapping_sql.py

@@ -318,46 +318,28 @@ class SqlIDMapping(test_backend_sql.SqlTests):
             PROVIDERS.id_mapping_api.get_public_id(local_entity5)
         )
 
+    def _prepare_domain_mappings_for_list(self):
+        # Create five mappings:
+        # two users in domainA, one group and two users in domainB
+        local_entities = [
+            {'domain_id': self.domainA['id'],
+             'entity_type': mapping.EntityType.USER},
+            {'domain_id': self.domainA['id'],
+             'entity_type': mapping.EntityType.USER},
+            {'domain_id': self.domainB['id'],
+             'entity_type': mapping.EntityType.GROUP},
+            {'domain_id': self.domainB['id'],
+             'entity_type': mapping.EntityType.USER},
+            {'domain_id': self.domainB['id'],
+             'entity_type': mapping.EntityType.USER}
+        ]
+        for e in local_entities:
+            e['local_id'] = uuid.uuid4().hex
+            e['public_id'] = PROVIDERS.id_mapping_api.create_id_mapping(e)
+        return local_entities
+
     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'] = (
-            PROVIDERS.id_mapping_api.create_id_mapping(local_entity1)
-        )
-        local_entity2['public_id'] = (
-            PROVIDERS.id_mapping_api.create_id_mapping(local_entity2)
-        )
-        local_entity3['public_id'] = (
-            PROVIDERS.id_mapping_api.create_id_mapping(local_entity3)
-        )
-        local_entity4['public_id'] = (
-            PROVIDERS.id_mapping_api.create_id_mapping(local_entity4)
-        )
-        local_entity5['public_id'] = (
-            PROVIDERS.id_mapping_api.create_id_mapping(local_entity5)
-        )
-
+        local_entities = self._prepare_domain_mappings_for_list()
         # NOTE(notmorgan): Always call to_dict in an active session context to
         # ensure that lazy-loaded relationships succeed. Edge cases could cause
         # issues especially in attribute mappers.
@@ -369,5 +351,20 @@ class SqlIDMapping(test_backend_sql.SqlTests):
                 )
             )
             domain_a_mappings = [m.to_dict() for m in domain_a_mappings]
-        self.assertItemsEqual([local_entity1, local_entity2],
-                              domain_a_mappings)
+        self.assertItemsEqual(local_entities[:2], domain_a_mappings)
+
+    def test_get_domain_mapping_list_by_entity(self):
+        local_entities = self._prepare_domain_mappings_for_list()
+        # NOTE(notmorgan): Always call to_dict in an active session context to
+        # ensure that lazy-loaded relationships succeed. Edge cases could cause
+        # issues especially in attribute mappers.
+        with sql.session_for_read():
+            # list user mappings for domainB
+            domain_b_mappings_user = (
+                PROVIDERS.id_mapping_api.get_domain_mapping_list(
+                    self.domainB['id'], entity_type=mapping.EntityType.USER
+                )
+            )
+            domain_b_mappings_user = [m.to_dict()
+                                      for m in domain_b_mappings_user]
+        self.assertItemsEqual(local_entities[-2:], domain_b_mappings_user)

releasenotes/notes/filter-mappings-by-entity-77162a146d375385.yaml

@@ -0,0 +1,11 @@
+---
+upgrade:
+  - |
+    As a performance improvement, the base mapping driver's method
+    ``get_domain_mapping_list`` now accepts an optional named argument
+    ``entity_type`` that can be used to get the mappings for a given
+    entity type only.
+    As this new call signature is already used in the ``identity.core``
+    module, authors/maintainers of out-of-tree custom mapping drivers
+    are expected to update their implementations of ``get_domain_mapping_list``
+    method accordingly.