PERF: Reduce memory footprint of `Chat::AutoRemove::HandleCategoryUpdated.call` (#28381)

This is a follow up to ed11ee9d05.

In `Chat::AutoRemove::HandleCategoryUpdated`, we are currently loading
the related users record in batches and then handing it off to
`Chat::Action::CalculateMembershipsForRemoval.call`. However, we are
still seeing memory spike as a result of this.

This commit eliminates the allocation of `User` ActiveRecord objects until
absolutely necessary. `Chat::Action::CalculateMembershipsForRemoval.call` has been
updated to accept an ActiveRecord relation instead which allows us to
avoid the ActiveRecord allocations.
This commit is contained in:
Alan Guo Xiang Tan 2024-08-16 05:37:31 +08:00 committed by GitHub
parent a92cf019db
commit 671f40ce07
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 17 additions and 17 deletions

View File

@ -19,7 +19,7 @@ module Chat
# out which of the users provided should have their [Chat::UserChatChannelMembership]
# records removed based on those security cases.
class CalculateMembershipsForRemoval
def self.call(scoped_users:, channel_ids: nil)
def self.call(scoped_users_query:, channel_ids: nil)
channel_permissions_map =
DB.query(<<~SQL, readonly: CategoryGroup.permission_types[:readonly])
WITH category_group_channel_map AS (
@ -59,12 +59,12 @@ module Chat
scoped_memberships =
Chat::UserChatChannelMembership
.joins(:chat_channel)
.where(user: scoped_users)
.where(user_id: scoped_users_query.select(:id))
.where(chat_channel_id: channel_permissions_map.map(&:channel_id))
memberships_to_remove = []
scoped_memberships.find_each do |membership|
scoped_user = scoped_users.find { |su| su.id == membership.user_id }
channel_permission =
channel_permissions_map.find { |cpm| cpm.channel_id == membership.chat_channel_id }
@ -91,9 +91,12 @@ module Chat
# At least one of the groups on the channel can create_post or
# has full permission, remove the membership if the user is in none
# of these groups.
if group_ids_with_write_permission.any? &&
!scoped_user.in_any_groups?(group_ids_with_write_permission)
memberships_to_remove << membership.id
if group_ids_with_write_permission.any?
scoped_user = scoped_users_query.where(id: membership.user_id).first
if !scoped_user&.in_any_groups?(group_ids_with_write_permission)
memberships_to_remove << membership.id
end
end
end

View File

@ -59,16 +59,11 @@ module Chat
end
def remove_users_without_channel_permission(users:, category_channel_ids:)
memberships_to_remove = []
users.find_in_batches do |batch_users|
memberships_to_remove.concat(
Chat::Action::CalculateMembershipsForRemoval.call(
scoped_users: batch_users,
channel_ids: category_channel_ids,
),
memberships_to_remove =
Chat::Action::CalculateMembershipsForRemoval.call(
scoped_users_query: users,
channel_ids: category_channel_ids,
)
end
return if memberships_to_remove.blank?

View File

@ -98,7 +98,7 @@ module Chat
def remove_users_without_channel_permission(scoped_users:)
memberships_to_remove =
Chat::Action::CalculateMembershipsForRemoval.call(scoped_users: scoped_users)
Chat::Action::CalculateMembershipsForRemoval.call(scoped_users_query: scoped_users)
return if memberships_to_remove.empty?

View File

@ -77,7 +77,9 @@ module Chat
def remove_from_private_channels(user:)
memberships_to_remove =
Chat::Action::CalculateMembershipsForRemoval.call(scoped_users: [user])
Chat::Action::CalculateMembershipsForRemoval.call(
scoped_users_query: User.where(id: user.id),
)
return if memberships_to_remove.empty?