From 14d54872f07ab7d356b7232fb5226c09567b078b Mon Sep 17 00:00:00 2001 From: Alan Guo Xiang Tan Date: Tue, 20 Dec 2022 08:20:45 +0800 Subject: [PATCH] PERF: Use `MessageBus.last_ids` instead of `MessageBus.last_id` for chat (#19523) Prior to this change, each request executed 2 Redis calls per chat channel that was loaded. The number of Redis calls quickly adds up once a user is following multiple channels. --- .../serializers/chat_channel_serializer.rb | 6 ++- .../structured_channel_serializer.rb | 53 +++++++++++++++---- plugins/chat/app/services/chat_publisher.rb | 36 ++++++++++--- 3 files changed, 76 insertions(+), 19 deletions(-) diff --git a/plugins/chat/app/serializers/chat_channel_serializer.rb b/plugins/chat/app/serializers/chat_channel_serializer.rb index 4b7b2c5a8e6..fe321c7fa0e 100644 --- a/plugins/chat/app/serializers/chat_channel_serializer.rb +++ b/plugins/chat/app/serializers/chat_channel_serializer.rb @@ -24,6 +24,8 @@ class ChatChannelSerializer < ApplicationSerializer def initialize(object, opts) super(object, opts) + + @opts = opts @current_user_membership = opts[:membership] end @@ -98,8 +100,8 @@ class ChatChannelSerializer < ApplicationSerializer def message_bus_last_ids { - new_messages: MessageBus.last_id("/chat/#{object.id}/new-messages"), - new_mentions: MessageBus.last_id("/chat/#{object.id}/new-mentions"), + new_messages: @opts[:new_messages_message_bus_last_id] || MessageBus.last_id(ChatPublisher.new_messages_message_bus_channel(object.id)), + new_mentions: @opts[:new_mentions_message_bus_last_id] || MessageBus.last_id(ChatPublisher.new_mentions_message_bus_channel(object.id)), } end diff --git a/plugins/chat/app/serializers/structured_channel_serializer.rb b/plugins/chat/app/serializers/structured_channel_serializer.rb index 5da682e3ec8..8ecbc6103d8 100644 --- a/plugins/chat/app/serializers/structured_channel_serializer.rb +++ b/plugins/chat/app/serializers/structured_channel_serializer.rb @@ -10,6 +10,8 @@ class StructuredChannelSerializer < ApplicationSerializer root: nil, scope: scope, membership: channel_membership(channel.id), + new_messages_message_bus_last_id: chat_message_bus_last_ids[ChatPublisher.new_messages_message_bus_channel(channel.id)], + new_mentions_message_bus_last_id: chat_message_bus_last_ids[ChatPublisher.new_mentions_message_bus_channel(channel.id)] ) end end @@ -21,6 +23,8 @@ class StructuredChannelSerializer < ApplicationSerializer root: nil, scope: scope, membership: channel_membership(channel.id), + new_messages_message_bus_last_id: chat_message_bus_last_ids[ChatPublisher.new_messages_message_bus_channel(channel.id)], + new_mentions_message_bus_last_id: chat_message_bus_last_ids[ChatPublisher.new_mentions_message_bus_channel(channel.id)] ) end end @@ -31,17 +35,46 @@ class StructuredChannelSerializer < ApplicationSerializer end def message_bus_last_ids - last_ids = { - channel_metadata: MessageBus.last_id("/chat/channel-metadata"), - channel_edits: MessageBus.last_id("/chat/channel-edits"), - channel_status: MessageBus.last_id("/chat/channel-status"), - new_channel: MessageBus.last_id("/chat/new-channel"), + ids = { + channel_metadata: chat_message_bus_last_ids[ChatPublisher::CHANNEL_METADATA_MESSAGE_BUS_CHANNEL], + channel_edits: chat_message_bus_last_ids[ChatPublisher::CHANNEL_EDITS_MESSAGE_BUS_CHANNEL], + channel_status: chat_message_bus_last_ids[ChatPublisher::CHANNEL_STATUS_MESSAGE_BUS_CHANNEL], + new_channel: chat_message_bus_last_ids[ChatPublisher::NEW_CHANNEL_MESSAGE_BUS_CHANNEL] } - if !scope.anonymous? - last_ids[:user_tracking_state] = MessageBus.last_id( - "/chat/user-tracking-state/#{scope.user.id}", - ) + + if id = chat_message_bus_last_ids[ChatPublisher.user_tracking_state_message_bus_channel(scope.user.id)] + ids[:user_tracking_state] = id + end + + ids + end + + private + + def chat_message_bus_last_ids + @chat_message_bus_last_ids ||= begin + message_bus_channels = [ + ChatPublisher::CHANNEL_METADATA_MESSAGE_BUS_CHANNEL, + ChatPublisher::CHANNEL_EDITS_MESSAGE_BUS_CHANNEL, + ChatPublisher::CHANNEL_STATUS_MESSAGE_BUS_CHANNEL, + ChatPublisher::NEW_CHANNEL_MESSAGE_BUS_CHANNEL, + ] + + if !scope.anonymous? + message_bus_channels.push(ChatPublisher.user_tracking_state_message_bus_channel(scope.user.id)) + end + + object[:public_channels].each do |channel| + message_bus_channels.push(ChatPublisher.new_messages_message_bus_channel(channel.id)) + message_bus_channels.push(ChatPublisher.new_mentions_message_bus_channel(channel.id)) + end + + object[:direct_message_channels].each do |channel| + message_bus_channels.push(ChatPublisher.new_messages_message_bus_channel(channel.id)) + message_bus_channels.push(ChatPublisher.new_mentions_message_bus_channel(channel.id)) + end + + MessageBus.last_ids(*message_bus_channels) end - last_ids end end diff --git a/plugins/chat/app/services/chat_publisher.rb b/plugins/chat/app/services/chat_publisher.rb index 638903da40f..5a1027d04d4 100644 --- a/plugins/chat/app/services/chat_publisher.rb +++ b/plugins/chat/app/services/chat_publisher.rb @@ -1,6 +1,10 @@ # frozen_string_literal: true module ChatPublisher + def self.new_messages_message_bus_channel(chat_channel_id) + "/chat/#{chat_channel_id}/new-messages" + end + def self.publish_new!(chat_channel, chat_message, staged_id) content = ChatMessageSerializer.new( @@ -10,9 +14,11 @@ module ChatPublisher content[:type] = :sent content[:stagedId] = staged_id permissions = permissions(chat_channel) + MessageBus.publish("/chat/#{chat_channel.id}", content.as_json, permissions) + MessageBus.publish( - "/chat/#{chat_channel.id}/new-messages", + self.new_messages_message_bus_channel(chat_channel.id), { message_id: chat_message.id, user_id: chat_message.user.id, @@ -120,22 +126,32 @@ module ChatPublisher ) end + def self.user_tracking_state_message_bus_channel(user_id) + "/chat/user-tracking-state/#{user_id}" + end + def self.publish_user_tracking_state(user, chat_channel_id, chat_message_id) MessageBus.publish( - "/chat/user-tracking-state/#{user.id}", + self.user_tracking_state_message_bus_channel(user.id), { chat_channel_id: chat_channel_id, chat_message_id: chat_message_id.to_i }.as_json, user_ids: [user.id], ) end + def self.new_mentions_message_bus_channel(chat_channel_id) + "/chat/#{chat_channel_id}/new-mentions" + end + def self.publish_new_mention(user_id, chat_channel_id, chat_message_id) MessageBus.publish( - "/chat/#{chat_channel_id}/new-mentions", + self.new_mentions_message_bus_channel(chat_channel_id), { message_id: chat_message_id }.as_json, user_ids: [user_id], ) end + NEW_CHANNEL_MESSAGE_BUS_CHANNEL = "/chat/new-channel" + def self.publish_new_channel(chat_channel, users) users.each do |user| serialized_channel = @@ -145,7 +161,7 @@ module ChatPublisher root: :chat_channel, membership: chat_channel.membership_for(user), ).as_json - MessageBus.publish("/chat/new-channel", serialized_channel, user_ids: [user.id]) + MessageBus.publish(NEW_CHANNEL_MESSAGE_BUS_CHANNEL, serialized_channel, user_ids: [user.id]) end end @@ -171,9 +187,11 @@ module ChatPublisher ) end + CHANNEL_EDITS_MESSAGE_BUS_CHANNEL = "/chat/channel-edits" + def self.publish_chat_channel_edit(chat_channel, acting_user) MessageBus.publish( - "/chat/channel-edits", + CHANNEL_EDITS_MESSAGE_BUS_CHANNEL, { chat_channel_id: chat_channel.id, name: chat_channel.title(acting_user), @@ -183,17 +201,21 @@ module ChatPublisher ) end + CHANNEL_STATUS_MESSAGE_BUS_CHANNEL = "/chat/channel-status" + def self.publish_channel_status(chat_channel) MessageBus.publish( - "/chat/channel-status", + CHANNEL_STATUS_MESSAGE_BUS_CHANNEL, { chat_channel_id: chat_channel.id, status: chat_channel.status }, permissions(chat_channel), ) end + CHANNEL_METADATA_MESSAGE_BUS_CHANNEL = "/chat/channel-metadata" + def self.publish_chat_channel_metadata(chat_channel) MessageBus.publish( - "/chat/channel-metadata", + CHANNEL_METADATA_MESSAGE_BUS_CHANNEL, { chat_channel_id: chat_channel.id, memberships_count: chat_channel.user_count }, permissions(chat_channel), )