paulmataruso/zulip
1
0
Fork 0
mirror of https://github.com/zulip/zulip.git synced 2025-11-04 05:53:43 +00:00
Code Issues Packages Projects Releases Wiki Activity

cache: Eliminate get-stream-by-name cache.

Browse Source 
We remove the cache functionality for the
get_realm_stream function, and we also change it to
return a thin Stream object (instead of calling
select_related with no arguments).

The main goal here is to remove code complexity, as we
have been prone to at least one caching validation bug
related to how Realm and UserGroup interact. That
particular bug was more theoretical than practical in
terms of its impact, to be clear.

Even if we were to be perfectly disciplined about only
caching thin stream objects and always making sure to
delete cache entries when stream data changed, we would
still be prone to ugly situations like having
transactions get rolled back before we delete the cache
entry. The do_deactivate_stream is a perfect example of
where we have to consider the best time to unset the
cache. If you unset it too early, then you are prone to
races where somebody else churns the cache right before
you update the database. If you set it too late, then
you can have an invalid entry after a rollback or
deadlock situation. If you just eliminate the cache as
a moving part, that whole debate is moot.

As the lack of test changes here indicates, we rarely
fetch streams by name any more in critical sections of
our code.

The one place where we fetch by name is in loading the
home page, but that is **only** when you specify a
stream name. And, of course, that only causes about an
extra millisecond of time.
This commit is contained in:
Steve Howell
2023-07-09 20:24:32 +00:00
committed by Tim Abbott
parent 046e4c715b
commit 89381a8072
6 changed files with 2 additions and 50 deletions
Download Patch File Download Diff File Expand all files Collapse all files

13
zerver/actions/streams.py
View File

@@ -17,11 +17,9 @@ from zerver.actions.default_streams import (
)
from zerver.actions.message_send import internal_send_stream_message
from zerver.lib.cache import (
cache_delete,
cache_delete_many,
cache_set,
display_recipient_cache_key,
get_stream_cache_key,
to_dict_cache_key_id,
)
from zerver.lib.email_mirror_helpers import encode_email_address
@@ -118,10 +116,6 @@ def do_deactivate_stream(stream: Stream, *, acting_user: Optional[UserProfile])
for group in default_stream_groups_for_stream:
do_remove_streams_from_default_stream_group(stream.realm, group, [stream])
# Remove the old stream information from remote cache.
old_cache_key = get_stream_cache_key(old_name, stream.realm_id)
cache_delete(old_cache_key)
stream_dict = stream.to_dict()
stream_dict.update(dict(name=old_name, invite_only=was_invite_only))
event = dict(type="stream", op="delete", streams=[stream_dict])
@@ -1102,13 +1096,6 @@ def do_rename_stream(stream: Stream, new_name: str, user_profile: UserProfile) -
recipient_id: int = stream.recipient_id
messages = Message.objects.filter(recipient_id=recipient_id).only("id")
# Update the display recipient and stream, which are easy single
# items to set.
old_cache_key = get_stream_cache_key(old_name, stream.realm_id)
new_cache_key = get_stream_cache_key(stream.name, stream.realm_id)
if old_cache_key != new_cache_key:
cache_delete(old_cache_key)
cache_set(new_cache_key, stream)
cache_set(display_recipient_cache_key(recipient_id), stream.name)
# Delete cache entries for everything else, which is cheaper and