DEV: Fix excessive MessageBus#last_id calls in chat (#20855)

We noticed via profiling that chat was doing N redis calls
per channel. Part of this was from the kick_message_bus_last_id
from 520d4f504b being incorrectly
passed down for DM channels rather that public channels, and the
other part was from the root MessageBus channel last_id
being fetched in ChannelSerializer for every single channel.

This commit fixes both issues, for me going from 134 redis calls
on page load to 20 locally.

Also deletes an old file missed in 12a18d4d55
This commit is contained in:
Martin Brennan 2023-03-28 14:45:45 +10:00 committed by GitHub
parent 7038540af6
commit 3ea8df4b06
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 53 additions and 161 deletions

View File

@ -109,14 +109,14 @@ module Chat
end
def meta
{
message_bus_last_ids: {
channel_message_bus_last_id: MessageBus.last_id("/chat/#{object.id}"),
new_messages: new_messages_message_bus_id,
new_mentions: new_mentions_message_bus_id,
kick: kick_message_bus_id,
},
ids = {
channel_message_bus_last_id: channel_message_bus_last_id,
new_messages: new_messages_message_bus_id,
new_mentions: new_mentions_message_bus_id,
}
ids[:kick] = kick_message_bus_id if !object.direct_message_channel?
{ message_bus_last_ids: ids }
end
alias_method :include_archive_topic_id?, :include_archive_status?
@ -127,6 +127,10 @@ module Chat
private
def channel_message_bus_last_id
@opts[:channel_message_bus_last_id] || Chat::Publisher.root_message_bus_channel(object.id)
end
def new_messages_message_bus_id
@opts[:new_messages_message_bus_last_id] ||
MessageBus.last_id(Chat::Publisher.new_messages_message_bus_channel(object.id))

View File

@ -15,6 +15,10 @@ module Chat
chat_message_bus_last_ids[Chat::Publisher.new_messages_message_bus_channel(channel.id)],
new_mentions_message_bus_last_id:
chat_message_bus_last_ids[Chat::Publisher.new_mentions_message_bus_channel(channel.id)],
kick_message_bus_last_id:
chat_message_bus_last_ids[Chat::Publisher.kick_users_message_bus_channel(channel.id)],
channel_message_bus_last_id:
chat_message_bus_last_ids[Chat::Publisher.root_message_bus_channel(channel.id)],
)
end
end
@ -30,8 +34,8 @@ module Chat
chat_message_bus_last_ids[Chat::Publisher.new_messages_message_bus_channel(channel.id)],
new_mentions_message_bus_last_id:
chat_message_bus_last_ids[Chat::Publisher.new_mentions_message_bus_channel(channel.id)],
kick_message_bus_last_id:
chat_message_bus_last_ids[Chat::Publisher.kick_users_message_bus_channel(channel.id)],
channel_message_bus_last_id:
chat_message_bus_last_ids[Chat::Publisher.root_message_bus_channel(channel.id)],
)
end
end
@ -87,11 +91,13 @@ module Chat
message_bus_channels.push(Chat::Publisher.new_messages_message_bus_channel(channel.id))
message_bus_channels.push(Chat::Publisher.new_mentions_message_bus_channel(channel.id))
message_bus_channels.push(Chat::Publisher.kick_users_message_bus_channel(channel.id))
message_bus_channels.push(Chat::Publisher.root_message_bus_channel(channel.id))
end
object[:direct_message_channels].each do |channel|
message_bus_channels.push(Chat::Publisher.new_messages_message_bus_channel(channel.id))
message_bus_channels.push(Chat::Publisher.new_mentions_message_bus_channel(channel.id))
message_bus_channels.push(Chat::Publisher.root_message_bus_channel(channel.id))
end
MessageBus.last_ids(*message_bus_channels)

View File

@ -24,7 +24,8 @@ module Chat
can_moderate: scope.can_moderate_chat?(object.chat_channel.chatable),
can_delete_self: scope.can_delete_own_chats?(object.chat_channel.chatable),
can_delete_others: scope.can_delete_other_chats?(object.chat_channel.chatable),
channel_message_bus_last_id: MessageBus.last_id("/chat/#{object.chat_channel.id}"),
channel_message_bus_last_id:
Chat::Publisher.root_message_bus_channel(object.chat_channel.id),
}
meta_hash[
:can_load_more_past

View File

@ -1,143 +0,0 @@
# frozen_string_literal: true
class ChatChannelSerializer < ApplicationSerializer
attributes :id,
:auto_join_users,
:allow_channel_wide_mentions,
:chatable,
:chatable_id,
:chatable_type,
:chatable_url,
:description,
:title,
:slug,
:last_message_sent_at,
:status,
:archive_failed,
:archive_completed,
:archived_messages,
:total_messages,
:archive_topic_id,
:memberships_count,
:current_user_membership,
:meta,
:threading_enabled
def threading_enabled
SiteSetting.enable_experimental_chat_threaded_discussions && object.threading_enabled
end
def initialize(object, opts)
super(object, opts)
@opts = opts
@current_user_membership = opts[:membership]
end
def include_description?
object.description.present?
end
def memberships_count
object.user_count
end
def chatable_url
object.chatable_url
end
def title
object.name || object.title(scope.user)
end
def chatable
case object.chatable_type
when "Category"
BasicCategorySerializer.new(object.chatable, root: false).as_json
when "DirectMessage"
DirectMessageSerializer.new(object.chatable, scope: scope, root: false).as_json
when "Site"
nil
end
end
def archive
object.chat_channel_archive
end
def include_archive_status?
!object.direct_message_channel? && scope.is_staff? && archive.present?
end
def archive_completed
archive.complete?
end
def archive_failed
archive.failed?
end
def archived_messages
archive.archived_messages
end
def total_messages
archive.total_messages
end
def archive_topic_id
archive.destination_topic_id
end
def include_auto_join_users?
scope.can_edit_chat_channel?
end
def include_current_user_membership?
@current_user_membership.present?
end
def current_user_membership
@current_user_membership.chat_channel = object
BaseChatChannelMembershipSerializer.new(
@current_user_membership,
scope: scope,
root: false,
).as_json
end
def meta
{
message_bus_last_ids: {
new_messages: new_messages_message_bus_id,
new_mentions: new_mentions_message_bus_id,
kick: kick_message_bus_id,
channel_message_bus_last_id: MessageBus.last_id("/chat/#{object.id}"),
},
}
end
alias_method :include_archive_topic_id?, :include_archive_status?
alias_method :include_total_messages?, :include_archive_status?
alias_method :include_archived_messages?, :include_archive_status?
alias_method :include_archive_failed?, :include_archive_status?
alias_method :include_archive_completed?, :include_archive_status?
private
def new_messages_message_bus_id
@opts[:new_messages_message_bus_last_id] ||
MessageBus.last_id(Chat::Publisher.new_messages_message_bus_channel(object.id))
end
def new_mentions_message_bus_id
@opts[:new_mentions_message_bus_last_id] ||
MessageBus.last_id(Chat::Publisher.new_mentions_message_bus_channel(object.id))
end
def kick_message_bus_id
@opts[:kick_message_bus_last_id] ||
MessageBus.last_id(Chat::Publisher.kick_users_message_bus_channel(object.id))
end
end

View File

@ -6,6 +6,10 @@ module Chat
"/chat/#{chat_channel_id}/new-messages"
end
def self.root_message_bus_channel(chat_channel_id)
"/chat/#{chat_channel_id}"
end
def self.publish_new!(chat_channel, chat_message, staged_id)
content =
Chat::MessageSerializer.new(
@ -16,7 +20,7 @@ module Chat
content[:staged_id] = staged_id
permissions = permissions(chat_channel)
MessageBus.publish("/chat/#{chat_channel.id}", content.as_json, permissions)
MessageBus.publish(root_message_bus_channel(chat_channel.id), content.as_json, permissions)
MessageBus.publish(
self.new_messages_message_bus_channel(chat_channel.id),
@ -40,7 +44,11 @@ module Chat
cooked: chat_message.cooked,
},
}
MessageBus.publish("/chat/#{chat_channel.id}", content.as_json, permissions(chat_channel))
MessageBus.publish(
root_message_bus_channel(chat_channel.id),
content.as_json,
permissions(chat_channel),
)
end
def self.publish_edit!(chat_channel, chat_message)
@ -50,7 +58,11 @@ module Chat
{ scope: anonymous_guardian, root: :chat_message },
).as_json
content[:type] = :edit
MessageBus.publish("/chat/#{chat_channel.id}", content.as_json, permissions(chat_channel))
MessageBus.publish(
root_message_bus_channel(chat_channel.id),
content.as_json,
permissions(chat_channel),
)
end
def self.publish_refresh!(chat_channel, chat_message)
@ -60,7 +72,11 @@ module Chat
{ scope: anonymous_guardian, root: :chat_message },
).as_json
content[:type] = :refresh
MessageBus.publish("/chat/#{chat_channel.id}", content.as_json, permissions(chat_channel))
MessageBus.publish(
root_message_bus_channel(chat_channel.id),
content.as_json,
permissions(chat_channel),
)
end
def self.publish_reaction!(chat_channel, chat_message, action, user, emoji)
@ -71,7 +87,11 @@ module Chat
type: :reaction,
chat_message_id: chat_message.id,
}
MessageBus.publish("/chat/#{chat_channel.id}", content.as_json, permissions(chat_channel))
MessageBus.publish(
root_message_bus_channel(chat_channel.id),
content.as_json,
permissions(chat_channel),
)
end
def self.publish_presence!(chat_channel, user, typ)
@ -80,7 +100,7 @@ module Chat
def self.publish_delete!(chat_channel, chat_message)
MessageBus.publish(
"/chat/#{chat_channel.id}",
root_message_bus_channel(chat_channel.id),
{ type: "delete", deleted_id: chat_message.id, deleted_at: chat_message.deleted_at },
permissions(chat_channel),
)
@ -88,7 +108,7 @@ module Chat
def self.publish_bulk_delete!(chat_channel, deleted_message_ids)
MessageBus.publish(
"/chat/#{chat_channel.id}",
root_message_bus_channel(chat_channel.id),
{ typ: "bulk_delete", deleted_ids: deleted_message_ids, deleted_at: Time.zone.now },
permissions(chat_channel),
)
@ -101,7 +121,11 @@ module Chat
{ scope: anonymous_guardian, root: :chat_message },
).as_json
content[:type] = :restore
MessageBus.publish("/chat/#{chat_channel.id}", content.as_json, permissions(chat_channel))
MessageBus.publish(
root_message_bus_channel(chat_channel.id),
content.as_json,
permissions(chat_channel),
)
end
def self.publish_flag!(chat_message, user, reviewable, score)