Performance improvements to marking expired users as inactive (#47)

This is a performance-related improvement to #13, which queried and hid active *and* already inactive users, one-by-one. This PR updates the code to query only **active**, expired users, all at once, and then mark them as inactive, all at once.

4 files changed, 37 insertions, 24 deletions

diff --git a/synapse/handlers/account_validity.py b/synapse/handlers/account_validity.py
index a6c907b9c9..0c2bcda4d0 100644
--- a/synapse/handlers/account_validity.py
+++ b/synapse/handlers/account_validity.py
@@ -281,23 +281,21 @@ class AccountValidityHandler(object):
         if self._show_users_in_user_directory:
             # Show the user in the directory again by setting them to active
             await self.profile_handler.set_active(
-                UserID.from_string(user_id), True, True
+                [UserID.from_string(user_id)], True, True
             )
 
         return expiration_ts
 
     @defer.inlineCallbacks
     def _mark_expired_users_as_inactive(self):
-        """Iterate over expired users. Mark them as inactive in order to hide them from the
-        user directory.
+        """Iterate over active, expired users. Mark them as inactive in order to hide them
+        from the user directory.
 
         Returns:
             Deferred
         """
-        # Get expired users
-        expired_user_ids = yield self.store.get_expired_users()
-        expired_users = [UserID.from_string(user_id) for user_id in expired_user_ids]
+        # Get active, expired users
+        active_expired_users = yield self.store.get_expired_users()
 
-        # Mark each one as non-active
-        for user in expired_users:
-            yield self.profile_handler.set_active(user, False, True)
+        # Mark each as non-active
+        yield self.profile_handler.set_active(active_expired_users, False, True)
diff --git a/synapse/handlers/deactivate_account.py b/synapse/handlers/deactivate_account.py
index f624c2a3f9..fe62c3f973 100644
--- a/synapse/handlers/deactivate_account.py
+++ b/synapse/handlers/deactivate_account.py
@@ -106,7 +106,7 @@ class DeactivateAccountHandler(BaseHandler):
         await self.store.user_set_password_hash(user_id, None)
 
         user = UserID.from_string(user_id)
-        await self._profile_handler.set_active(user, False, False)
+        await self._profile_handler.set_active([user], False, False)
 
         # Add the user to a table of users pending deactivation (ie.
         # removal from all the rooms they're a member of)
diff --git a/synapse/handlers/profile.py b/synapse/handlers/profile.py
index bca0d8d380..1880bb2dd9 100644
--- a/synapse/handlers/profile.py
+++ b/synapse/handlers/profile.py
@@ -15,6 +15,7 @@
 # limitations under the License.
 
 import logging
+from typing import List
 
 from six import raise_from
 from six.moves import range
@@ -67,6 +68,7 @@ class BaseProfileHandler(BaseHandler):
 
         self.max_avatar_size = hs.config.max_avatar_size
         self.allowed_avatar_mimetypes = hs.config.allowed_avatar_mimetypes
+        self.replicate_user_profiles_to = hs.config.replicate_user_profiles_to
 
         if hs.config.worker_app is None:
             self.clock.looping_call(
@@ -293,29 +295,42 @@ class BaseProfileHandler(BaseHandler):
         run_in_background(self._replicate_profiles)
 
     @defer.inlineCallbacks
-    def set_active(self, target_user, active, hide):
+    def set_active(
+        self, users: List[UserID], active: bool, hide: bool,
+    ):
         """
-        Sets the 'active' flag on a user profile. If set to false, the user
-        account is considered deactivated or hidden.
+        Sets the 'active' flag on a set of user profiles. If set to false, the
+        accounts are considered deactivated or hidden.
 
         If 'hide' is true, then we interpret active=False as a request to try to
-        hide the user rather than deactivating it.  This means withholding the
-        profile from replication (and mark it as inactive) rather than clearing
-        the profile from the HS DB. Note that unlike set_displayname and
-        set_avatar_url, this does *not* perform authorization checks! This is
-        because the only place it's used currently is in account deactivation
-        where we've already done these checks anyway.
+        hide the users rather than deactivating them. This means withholding the
+        profiles from replication (and mark it as inactive) rather than clearing
+        the profile from the HS DB.
+
+        Note that unlike set_displayname and set_avatar_url, this does *not*
+        perform authorization checks! This is because the only place it's used
+        currently is in account deactivation where we've already done these
+        checks anyway.
+
+        Args:
+            users: The users to modify
+            active: Whether to set the user to active or inactive
+            hide: Whether to hide the user (withold from replication). If
+                False and active is False, user will have their profile
+                erased
+
+        Returns:
+            Deferred
         """
-        if len(self.hs.config.replicate_user_profiles_to) > 0:
+        if len(self.replicate_user_profiles_to) > 0:
             cur_batchnum = (
                 yield self.store.get_latest_profile_replication_batch_number()
             )
             new_batchnum = 0 if cur_batchnum is None else cur_batchnum + 1
         else:
             new_batchnum = None
-        yield self.store.set_profile_active(
-            target_user.localpart, active, hide, new_batchnum
-        )
+
+        yield self.store.set_profiles_active(users, active, hide, new_batchnum)
 
         # start a profile replication push
         run_in_background(self._replicate_profiles)
diff --git a/synapse/handlers/register.py b/synapse/handlers/register.py
index c7e3861054..8f9841117a 100644
--- a/synapse/handlers/register.py
+++ b/synapse/handlers/register.py
@@ -281,7 +281,7 @@ class RegistrationHandler(BaseHandler):
             yield self.store.add_account_data_for_user(
                 user_id, "im.vector.hide_profile", {"hide_profile": True}
             )
-            yield self.profile_handler.set_active(user, False, True)
+            yield self.profile_handler.set_active([user], False, True)
 
         return user_id