update_to_dict_cache: Use bulk queries when preparing msgs for cache.

During events such as stream / topic name edit for a topic, we were
running queries to db in loop for each message for reactions,
submessages and realm_id. This commit reduces the queries to be
done only for realm_id, which is yet to be fixed.

This is accomplished by building messages with empty reactions
and submessages and then updating them in the messages using bulk
queries.
This commit is contained in:
Aman Agrawal
2020-05-19 13:52:49 +05:30
committed by Tim Abbott
parent b8fe6245e3
commit 2a4c62a326
4 changed files with 46 additions and 39 deletions

View File

@@ -4308,11 +4308,11 @@ def update_to_dict_cache(changed_messages: List[Message]) -> List[int]:
messages)."""
items_for_remote_cache = {}
message_ids = []
for changed_message in changed_messages:
message_ids.append(changed_message.id)
key = to_dict_cache_key_id(changed_message.id)
value = MessageDict.to_dict_uncached(changed_message)
items_for_remote_cache[key] = (value,)
changed_messages_to_dict = MessageDict.to_dict_uncached(changed_messages)
for msg_id, msg in changed_messages_to_dict.items():
message_ids.append(msg_id)
key = to_dict_cache_key_id(msg_id)
items_for_remote_cache[key] = (msg,)
cache_set_many(items_for_remote_cache)
return message_ids

View File

@@ -41,7 +41,7 @@ def message_cache_items(items_for_remote_cache: Dict[str, Tuple[bytes]],
commented out for a while.
'''
key = to_dict_cache_key_id(message.id)
value = MessageDict.to_dict_uncached(message)
value = MessageDict.to_dict_uncached([message])[message.id]
items_for_remote_cache[key] = (value,)
def user_cache_items(items_for_remote_cache: Dict[str, Tuple[UserProfile]],

View File

@@ -171,7 +171,7 @@ def stringify_message_dict(message_dict: Dict[str, Any]) -> bytes:
@cache_with_key(to_dict_cache_key, timeout=3600*24)
def message_to_dict_json(message: Message) -> bytes:
return MessageDict.to_dict_uncached(message)
return MessageDict.to_dict_uncached([message])[message.id]
def save_message_rendered_content(message: Message, content: str) -> str:
rendered_content = render_markdown(message, content, realm=message.get_realm())
@@ -263,10 +263,6 @@ class MessageDict:
del obj['sender_is_mirror_dummy']
@staticmethod
def to_dict_uncached(message: Message) -> bytes:
dct = MessageDict.to_dict_uncached_helper(message)
return stringify_message_dict(dct)
def sew_submessages_and_reactions_to_msgs(messages: List[Dict[str, Any]]) -> List[Dict[str, Any]]:
msg_ids = [msg['id'] for msg in messages]
submessages = SubMessage.get_raw_db_rows(msg_ids)
@@ -276,26 +272,37 @@ class MessageDict:
return sew_messages_and_reactions(messages, reactions)
@staticmethod
def to_dict_uncached_helper(message: Message) -> Dict[str, Any]:
return MessageDict.build_message_dict(
message = message,
message_id = message.id,
last_edit_time = message.last_edit_time,
edit_history = message.edit_history,
content = message.content,
topic_name = message.topic_name(),
date_sent = message.date_sent,
rendered_content = message.rendered_content,
rendered_content_version = message.rendered_content_version,
sender_id = message.sender.id,
sender_realm_id = message.sender.realm_id,
sending_client_name = message.sending_client.name,
recipient_id = message.recipient.id,
recipient_type = message.recipient.type,
recipient_type_id = message.recipient.type_id,
reactions = Reaction.get_raw_db_rows([message.id]),
submessages = SubMessage.get_raw_db_rows([message.id]),
)
def to_dict_uncached(messages: List[Message]) -> Dict[int, bytes]:
messages_dict = MessageDict.to_dict_uncached_helper(messages)
encoded_messages = {msg['id']: stringify_message_dict(msg) for msg in messages_dict}
return encoded_messages
@staticmethod
def to_dict_uncached_helper(messages: List[Message]) -> List[Dict[str, Any]]:
message_dicts = [
MessageDict.build_message_dict(
message = message,
message_id = message.id,
last_edit_time = message.last_edit_time,
edit_history = message.edit_history,
content = message.content,
topic_name = message.topic_name(),
date_sent = message.date_sent,
rendered_content = message.rendered_content,
rendered_content_version = message.rendered_content_version,
sender_id = message.sender.id,
sender_realm_id = message.sender.realm_id,
sending_client_name = message.sending_client.name,
recipient_id = message.recipient.id,
recipient_type = message.recipient.type,
recipient_type_id = message.recipient.type_id,
) for message in messages
]
# We do bulk query for reactions and submessages to avoid
# running queries for them in a loop.
MessageDict.sew_submessages_and_reactions_to_msgs(message_dicts)
return message_dicts
@staticmethod
def get_raw_db_rows(needed_ids: List[int]) -> List[Dict[str, Any]]:
@@ -363,8 +370,8 @@ class MessageDict:
recipient_id: int,
recipient_type: int,
recipient_type_id: int,
reactions: List[Dict[str, Any]],
submessages: List[Dict[str, Any]]
reactions: List[Dict[str, Any]]=[],
submessages: List[Dict[str, Any]]=[]
) -> Dict[str, Any]:
obj = dict(

View File

@@ -1313,7 +1313,7 @@ class MessageDictTest(ZulipTestCase):
apply_markdown: bool,
client_gravatar: bool) -> Dict[str, Any]:
msg = reload_message(msg_id)
unhydrated_dict = MessageDict.to_dict_uncached_helper(msg)
unhydrated_dict = MessageDict.to_dict_uncached_helper([msg])[0]
# The next step mutates the dict in place
# for performance reasons.
MessageDict.post_process_dicts(
@@ -1492,7 +1492,7 @@ class MessageDictTest(ZulipTestCase):
return Message.objects.get(id=msg_id)
def assert_topic_links(links: List[str], msg: Message) -> None:
dct = MessageDict.to_dict_uncached_helper(msg)
dct = MessageDict.to_dict_uncached_helper([msg])[0]
self.assertEqual(dct[TOPIC_LINKS], links)
# Send messages before and after saving the realm filter from each user.
@@ -2687,10 +2687,10 @@ class EditMessageTest(ZulipTestCase):
# Check number of queries performed
with queries_captured() as queries:
for msg in messages:
MessageDict.to_dict_uncached(msg)
# 3 query for realm_id, reactions & submessage per message = 6
self.assertEqual(len(queries), 6)
MessageDict.to_dict_uncached(messages)
# 1 query for realm_id per message = 2
# 1 query each for reactions & submessage for all messages = 2
self.assertEqual(len(queries), 4)
def test_save_message(self) -> None:
"""This is also tested by a client test, but here we can verify