mirror of
https://github.com/zulip/zulip.git
synced 2025-11-03 21:43:21 +00:00
presence: Optimize performance of mobile_query.
I'm not sure exactly what series of history got us here, but we were fetching the mobile_user_ids data for all users in the organization, regardless of whether they were recently active (and thus relevant for the main presence data set). And doing so in a sloppy fashion (sending every user ID over the wire, rather than just having the database join on Realm). Fixing this saves a factor of 4-5 on the total runtime of a presence request on organizations with 10Ks of users like chat.zulip.org; more like 25% in an organization with 150. Since large organizations are very heavily weighted in the overall cost of presence, this is a huge win. Fixes part of #13734.
This commit is contained in:
@@ -146,22 +146,6 @@ def get_presence_for_user(user_profile_id: int,
|
||||
|
||||
|
||||
def get_status_dict_by_realm(realm_id: int, slim_presence: bool = False) -> Dict[str, Dict[str, Any]]:
|
||||
user_profile_ids = UserProfile.objects.filter(
|
||||
realm_id=realm_id,
|
||||
is_active=True,
|
||||
is_bot=False
|
||||
).order_by('id').values_list('id', flat=True)
|
||||
|
||||
user_profile_ids = list(user_profile_ids)
|
||||
if not user_profile_ids: # nocoverage
|
||||
# This conditional is necessary because query_for_ids
|
||||
# throws an exception if passed an empty list.
|
||||
#
|
||||
# It's not clear this condition is actually possible,
|
||||
# though, because it shouldn't be possible to end up with
|
||||
# a realm with 0 active users.
|
||||
return {}
|
||||
|
||||
two_weeks_ago = timezone_now() - datetime.timedelta(weeks=2)
|
||||
query = UserPresence.objects.filter(
|
||||
realm_id=realm_id,
|
||||
@@ -186,6 +170,16 @@ def get_status_dict_by_realm(realm_id: int, slim_presence: bool = False) -> Dict
|
||||
flat=True
|
||||
)
|
||||
|
||||
user_profile_ids = [presence_row['user_profile__id'] for presence_row in presence_rows]
|
||||
if len(user_profile_ids) == 0:
|
||||
# This conditional is necessary because query_for_ids
|
||||
# throws an exception if passed an empty list.
|
||||
#
|
||||
# It's not clear this condition is actually possible,
|
||||
# though, because it shouldn't be possible to end up with
|
||||
# a realm with 0 active users.
|
||||
return {}
|
||||
|
||||
mobile_query = query_for_ids(
|
||||
query=mobile_query,
|
||||
user_ids=user_profile_ids,
|
||||
|
||||
Reference in New Issue
Block a user