Mirror
/
zulip
mirror of https://github.com/zulip/zulip.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

performance: Avoid call to access_stream_by_id. 

We already trust ids that are put on our queue
for deferred work. For example, see the code for
"mark_stream_messages_as_read_for_everyone"

We now pass stream_recipient_id when we queue
up work for do_mark_stream_messages_as_read.

This generally saves about 3 queries per
user when we unsubscribe them from a stream.
This commit is contained in:
Steve Howell 2020-10-16 14:40:25 +00:00 committed by Tim Abbott
parent 2256d72015
commit 378062cc83
3 changed files with 14 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
zerver/lib/actions.py
View File

@ -3099,7 +3099,7 @@ def bulk_remove_subscriptions(users: Iterable[UserProfile],
event = {'type': 'mark_stream_messages_as_read',
'user_profile_id': user_profile.id,
'stream_ids': [stream.id for stream in streams]}
'stream_recipient_ids': [stream.recipient_id for stream in streams]}
queue_json_publish("deferred_work", event)
send_peer_remove_events(

22
zerver/tests/test_subs.py
View File

@ -1382,7 +1382,7 @@ class StreamAdminTest(ZulipTestCase):
those you aren't on.
"""
result = self.attempt_unsubscribe_of_principal(
query_count=19,
query_count=16,
target_users=[self.example_user('cordelia')],
is_realm_admin=True,
is_subbed=True,
@ -1398,7 +1398,7 @@ class StreamAdminTest(ZulipTestCase):
If you're a realm admin, you can remove multiple users from a stream.
TODO: We have too many queries for this situation--each additional
user leads to 7 more queries.
user leads to 4 more queries.
Fortunately, some of the extra work here is in
do_mark_stream_messages_as_read, which gets deferred
@ -1409,7 +1409,7 @@ class StreamAdminTest(ZulipTestCase):
for name in ['cordelia', 'prospero', 'iago', 'hamlet', 'ZOE']
]
result = self.attempt_unsubscribe_of_principal(
query_count=47,
query_count=32,
cache_count=9,
target_users=target_users,
is_realm_admin=True,
@ -1427,7 +1427,7 @@ class StreamAdminTest(ZulipTestCase):
are on.
"""
result = self.attempt_unsubscribe_of_principal(
query_count=20,
query_count=17,
target_users=[self.example_user('cordelia')],
is_realm_admin=True,
is_subbed=True,
@ -1444,7 +1444,7 @@ class StreamAdminTest(ZulipTestCase):
streams you aren't on.
"""
result = self.attempt_unsubscribe_of_principal(
query_count=20,
query_count=17,
target_users=[self.example_user('cordelia')],
is_realm_admin=True,
is_subbed=False,
@ -1461,7 +1461,7 @@ class StreamAdminTest(ZulipTestCase):
You can remove others from public streams you're a stream administrator of.
"""
result = self.attempt_unsubscribe_of_principal(
query_count=19,
query_count=16,
target_users=[self.example_user('cordelia')],
is_realm_admin=False,
is_stream_admin=True,
@ -1482,7 +1482,7 @@ class StreamAdminTest(ZulipTestCase):
for name in ['cordelia', 'prospero', 'othello', 'hamlet', 'ZOE']
]
result = self.attempt_unsubscribe_of_principal(
query_count=47,
query_count=32,
cache_count=9,
target_users=target_users,
is_realm_admin=False,
@ -1500,7 +1500,7 @@ class StreamAdminTest(ZulipTestCase):
You can remove others from private streams you're a stream administrator of.
"""
result = self.attempt_unsubscribe_of_principal(
query_count=20,
query_count=17,
target_users=[self.example_user('cordelia')],
is_realm_admin=False,
is_stream_admin=True,
@ -1522,7 +1522,7 @@ class StreamAdminTest(ZulipTestCase):
def test_admin_remove_others_from_stream_legacy_emails(self) -> None:
result = self.attempt_unsubscribe_of_principal(
query_count=19,
query_count=16,
target_users=[self.example_user('cordelia')],
is_realm_admin=True,
is_subbed=True,
@ -1536,7 +1536,7 @@ class StreamAdminTest(ZulipTestCase):
def test_admin_remove_multiple_users_from_stream_legacy_emails(self) -> None:
result = self.attempt_unsubscribe_of_principal(
query_count=26,
query_count=20,
target_users=[self.example_user('cordelia'), self.example_user('prospero')],
is_realm_admin=True,
is_subbed=True,
@ -3262,7 +3262,7 @@ class SubscriptionAPITest(ZulipTestCase):
get_client("website"),
)
self.assert_length(query_count, 52)
self.assert_length(query_count, 28)
self.assert_length(cache_count, 4)
peer_events = [e for e in events

10
zerver/worker/queue_processors.py
View File

@ -85,7 +85,6 @@ from zerver.lib.send_email import (
send_email_from_dict,
send_future_email,
)
from zerver.lib.streams import access_stream_by_id
from zerver.lib.timestamp import timestamp_to_datetime
from zerver.lib.url_preview import preview as url_preview
from zerver.models import (
@ -771,13 +770,8 @@ class DeferredWorker(QueueProcessingWorker):
if event['type'] == 'mark_stream_messages_as_read':
user_profile = get_user_profile_by_id(event['user_profile_id'])
for stream_id in event['stream_ids']:
# Since the user just unsubscribed, we don't require
# an active Subscription object (otherwise, private
# streams would never be accessible)
(stream, recipient, sub) = access_stream_by_id(user_profile, stream_id,
require_active=False)
do_mark_stream_messages_as_read(user_profile, stream.recipient_id)
for recipient_id in event['stream_recipient_ids']:
do_mark_stream_messages_as_read(user_profile, recipient_id)
elif event["type"] == 'mark_stream_messages_as_read_for_everyone':
# This event is generated by the stream deactivation code path.
batch_size = 100