From 8ad7133351cfaa76cfcd8b30cdc734f53ad48ea6 Mon Sep 17 00:00:00 2001 From: Steve Howell Date: Sat, 16 Sep 2017 12:44:03 -0700 Subject: [PATCH] Cache active_user_ids() more directly. We now have a dedicated cache for active_user_ids() that only stores a list of user_ids. Before this commit, active_user_ids() used a cache of UserProfile dictionaries, so it incurred unnecessary deserialization costs for all the user fields that it sliced away in a list comprehension. Because the cache is skinnier here, we also need to invalidate it less frequently. Basically, all we care about is new users, realm deactivations, and user deactivations. It's hard to measure how much this will improve performance, because the speedup for any operation here is pretty minor, but we use this function a lot, so hopefully it will make the overall system more healthy. --- zerver/lib/actions.py | 6 +----- zerver/lib/cache.py | 9 +++++++++ zerver/models.py | 11 ++++++++++- zerver/tests/test_subs.py | 4 ++-- 4 files changed, 22 insertions(+), 8 deletions(-) diff --git a/zerver/lib/actions.py b/zerver/lib/actions.py index f78da9ed73..560f2b6cad 100644 --- a/zerver/lib/actions.py +++ b/zerver/lib/actions.py @@ -53,7 +53,7 @@ from zerver.models import Realm, RealmEmoji, Stream, UserProfile, UserActivity, get_realm, bulk_get_recipients, \ email_allowed_for_realm, email_to_username, display_recipient_cache_key, \ get_user_profile_by_email, get_user, get_stream_cache_key, \ - UserActivityInterval, get_active_user_dicts_in_realm, get_active_streams, \ + UserActivityInterval, active_user_ids, get_active_streams, \ realm_filters_for_realm, RealmFilter, receives_offline_notifications, \ get_owned_bot_dicts, \ get_old_unclaimed_attachments, get_cross_realm_emails, \ @@ -147,10 +147,6 @@ def log_event(event): with open(template % ('events',), 'a') as log: log.write(force_str(ujson.dumps(event) + u'\n')) -def active_user_ids(realm_id): - # type: (int) -> List[int] - return [userdict['id'] for userdict in get_active_user_dicts_in_realm(realm_id)] - def can_access_stream_user_ids(stream): # type: (Stream) -> Set[int] diff --git a/zerver/lib/cache.py b/zerver/lib/cache.py index 935fd359ac..6ce4a0fc75 100644 --- a/zerver/lib/cache.py +++ b/zerver/lib/cache.py @@ -337,6 +337,10 @@ def active_user_dicts_in_realm_cache_key(realm_id): # type: (int) -> Text return u"active_user_dicts_in_realm:%s" % (realm_id,) +def active_user_ids_cache_key(realm_id): + # type: (int) -> Text + return u"active_user_ids:%s" % (realm_id,) + bot_dict_fields = ['id', 'full_name', 'short_name', 'bot_type', 'email', 'is_active', 'default_sending_stream__name', 'realm_id', @@ -387,6 +391,10 @@ def flush_user_profile(sender, **kwargs): set(kwargs['update_fields'])) > 0: cache_delete(active_user_dicts_in_realm_cache_key(user_profile.realm_id)) + if kwargs.get('update_fields') is None or \ + ('is_active' in kwargs['update_fields']): + cache_delete(active_user_ids_cache_key(user_profile.realm_id)) + if kwargs.get('updated_fields') is None or \ 'email' in kwargs['update_fields']: delete_display_recipient_cache(user_profile) @@ -413,6 +421,7 @@ def flush_realm(sender, **kwargs): if realm.deactivated: cache_delete(active_user_dicts_in_realm_cache_key(realm.id)) + cache_delete(active_user_ids_cache_key(realm.id)) cache_delete(bot_dicts_in_realm_cache_key(realm)) cache_delete(realm_alert_words_cache_key(realm)) diff --git a/zerver/models.py b/zerver/models.py index b16a7e37d1..f31965bb48 100644 --- a/zerver/models.py +++ b/zerver/models.py @@ -19,7 +19,7 @@ from zerver.lib.cache import cache_with_key, flush_user_profile, flush_realm, \ user_profile_by_api_key_cache_key, \ user_profile_by_id_cache_key, user_profile_by_email_cache_key, \ user_profile_cache_key, generic_bulk_cached_fetch, cache_set, flush_stream, \ - display_recipient_cache_key, cache_delete, \ + display_recipient_cache_key, cache_delete, active_user_ids_cache_key, \ get_stream_cache_key, active_user_dicts_in_realm_cache_key, \ bot_dicts_in_realm_cache_key, active_user_dict_fields, \ bot_dict_fields, flush_message, bot_profile_cache_key @@ -1530,6 +1530,15 @@ def get_active_user_dicts_in_realm(realm_id): is_active=True ).values(*active_user_dict_fields) +@cache_with_key(active_user_ids_cache_key, timeout=3600*24*7) +def active_user_ids(realm_id): + # type: (int) -> List[int] + query = UserProfile.objects.filter( + realm_id=realm_id, + is_active=True + ).values_list('id', flat=True) + return list(query) + @cache_with_key(bot_dicts_in_realm_cache_key, timeout=3600*24*7) def get_bot_dicts_in_realm(realm): # type: (Realm) -> List[Dict[str, Any]] diff --git a/zerver/tests/test_subs.py b/zerver/tests/test_subs.py index 4807914819..543a423b0f 100644 --- a/zerver/tests/test_subs.py +++ b/zerver/tests/test_subs.py @@ -36,7 +36,7 @@ from zerver.lib.test_runner import ( from zerver.models import ( get_display_recipient, Message, Realm, Recipient, Stream, Subscription, - DefaultStream, UserProfile, get_user_profile_by_id + DefaultStream, UserProfile, get_user_profile_by_id, active_user_ids, ) from zerver.lib.actions import ( @@ -45,7 +45,7 @@ from zerver.lib.actions import ( gather_subscriptions_helper, bulk_add_subscriptions, bulk_remove_subscriptions, gather_subscriptions, get_default_streams_for_realm, get_realm, get_stream, get_user, set_default_streams, check_stream_name, - create_stream_if_needed, create_streams_if_needed, active_user_ids, + create_stream_if_needed, create_streams_if_needed, do_deactivate_stream, stream_welcome_message, )