PERF: automatically join users to channels more efficiently (#28392)

- Only ever auto join 10k users to channels (ordered by last seen)
- Join users to all channels at once, instead of batching and splitting
This commit is contained in:
Sam 2024-08-16 13:58:12 +10:00 committed by GitHub
parent de79e5628e
commit ade001604b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 110 additions and 9 deletions

View File

@ -5,14 +5,62 @@ module Jobs
class AutoJoinUsers < ::Jobs::Scheduled
every 1.hour
LAST_SEEN_DAYS = 30
def execute(_args)
return if !SiteSetting.chat_enabled
::Chat::Channel
.where(auto_join_users: true)
.each do |channel|
::Chat::ChannelMembershipManager.new(channel).enforce_automatic_channel_memberships
end
allowed_group_permissions = [
CategoryGroup.permission_types[:create_post],
CategoryGroup.permission_types[:full],
]
join_mode = ::Chat::UserChatChannelMembership.join_modes[:automatic]
sql = <<~SQL
WITH users AS (
SELECT id FROM users u
JOIN user_options uo ON uo.user_id = u.id
WHERE id > 0 AND (u.suspended_till IS NULL OR u.suspended_till <= :now)
AND (u.last_seen_at IS NULL OR u.last_seen_at > :last_seen_at)
AND u.active
AND NOT u.staged
AND uo.chat_enabled
AND NOT EXISTS (SELECT 1 FROM anonymous_users a WHERE a.user_id = u.id)
ORDER BY last_seen_at desc
LIMIT :max_users
)
INSERT INTO user_chat_channel_memberships (user_id, chat_channel_id, following, created_at, updated_at, join_mode)
SELECT DISTINCT users.id AS user_id,
chat_channels.id AS chat_channel_id,
TRUE AS following,
:now::timestamp AS created_at,
:now::timestamp AS updated_at,
:join_mode AS join_mode
FROM users
JOIN chat_channels on auto_join_users AND chatable_type = 'Category'
JOIN categories c on c.id = chat_channels.chatable_id
LEFT OUTER JOIN user_chat_channel_memberships uccm ON uccm.chat_channel_id = chat_channels.id
AND uccm.user_id = users.id
LEFT OUTER JOIN group_users gu ON gu.user_id = users.id
LEFT OUTER JOIN category_groups cg ON cg.group_id = gu.group_id
AND cg.permission_type in (:allowed_group_permissions)
AND c.id = cg.category_id
WHERE (cg.group_id is NOT null OR NOT c.read_restricted) AND uccm.id IS NULL
ON CONFLICT DO NOTHING
SQL
DB.exec(
sql,
now: Time.zone.now,
last_seen_at: LAST_SEEN_DAYS.days.ago,
allowed_group_permissions: allowed_group_permissions,
join_mode: join_mode,
max_users: SiteSetting.max_chat_auto_joined_users,
)
end
end
end

View File

@ -3,17 +3,70 @@
describe Jobs::Chat::AutoJoinUsers do
subject(:job) { described_class.new }
it "works" do
Jobs.run_immediately!
channel = Fabricate(:category_channel, auto_join_users: true)
user = Fabricate(:user, last_seen_at: 1.minute.ago, active: true)
fab!(:channel) { Fabricate(:category_channel, auto_join_users: true) }
fab!(:user) { Fabricate(:user, last_seen_at: 1.minute.ago, active: true) }
fab!(:group)
fab!(:user_without_chat) do
user = Fabricate(:user)
user.user_option.update!(chat_enabled: false)
user
end
fab!(:stage_user) { Fabricate(:user, staged: true) }
fab!(:suspended_user) { Fabricate(:user, suspended_till: 1.day.from_now) }
fab!(:inactive_user) { Fabricate(:user, active: false) }
fab!(:anonymous_user) { Fabricate(:anonymous) }
before { Jobs.run_immediately! }
it "is does not auto join users without permissions" do
channel.category.read_restricted = true
channel.category.set_permissions(group => :full)
channel.category.save!
job.execute({})
membership = Chat::UserChatChannelMembership.find_by(user: user, chat_channel: channel)
expect(membership).to be_nil
GroupUser.create!(group: group, user: user)
job.execute({})
membership = Chat::UserChatChannelMembership.find_by(user: user, chat_channel: channel)
expect(membership).to_not be_nil
end
it "works for simple workflows" do
# this is just to avoid test fragility, we should always have negative users
bot_id = (User.minimum(:id) - 1)
bot_id = -1 if bot_id > 0
_bot_user = Fabricate(:user, id: bot_id)
membership = Chat::UserChatChannelMembership.find_by(user: user, chat_channel: channel)
expect(membership).to be_nil
job.execute({})
# should exclude bot / inactive / staged / suspended users
# note category fabricator creates a user so we are stuck with that user in the channel
expect(
Chat::UserChatChannelMembership.where(chat_channel: channel).pluck(:user_id),
).to contain_exactly(user.id, channel.category.user.id)
membership = Chat::UserChatChannelMembership.find_by(user: user, chat_channel: channel)
expect(membership.following).to eq(true)
membership.update!(following: false)
job.execute({})
membership.reload
expect(membership.following).to eq(false)
channel = Fabricate(:category_channel, auto_join_users: false)
job.execute({})
membership = Chat::UserChatChannelMembership.find_by(user: user, chat_channel: channel)
expect(membership).to be_nil
end
end