PERF: Update all user_histories with one query in UserDestroyer (#16717)

7a284164 previously switched the UserDestroyer to use find_each when iterating over UserHistory records. Unfortunately, since this logic is wrapped in a transaction, this didn't actually solve the memory usage problem. ActiveRecord maintains references to all modified models within a transaction.

This commit updates the logic to use a single SQL query, rather than updating models one-by-one
This commit is contained in:
David Taylor 2022-05-11 13:39:31 +01:00 committed by GitHub
parent d90065e0ef
commit 67b23c0e22
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 18 additions and 12 deletions

View File

@ -47,13 +47,10 @@ class UserDestroyer
# Add info about the user to staff action logs # Add info about the user to staff action logs
UserHistory.staff_action_records( UserHistory.staff_action_records(
Discourse.system_user, acting_user: user.username Discourse.system_user, acting_user: user.username
).unscope(:order).find_each do |log| ).update_all([
log.details ||= '' "details = CONCAT(details, ?)",
log.details = (log.details.split("\n") + "\nuser_id: #{user.id}\nusername: #{user.username}"
["user_id: #{user.id}", "username: #{user.username}"] ])
).join("\n")
log.save!
end
# keep track of emails used # keep track of emails used
user_emails = user.user_emails.pluck(:email) user_emails = user.user_emails.pluck(:email)

View File

@ -442,19 +442,28 @@ describe UserDestroyer do
logger.log_site_setting_change( logger.log_site_setting_change(
'site_description', 'site_description',
'Our friendly community', 'Our friendly community',
'My favourite community' 'My favourite community',
)
logger.log_site_setting_change(
'site_description',
'Our friendly community',
'My favourite community',
details: "existing details"
) )
end end
it "should keep the staff action log and add the username" do it "should keep the staff action log and add the username" do
username = user.username username = user.username
log = UserHistory.staff_action_records( ids = UserHistory.staff_action_records(
Discourse.system_user, Discourse.system_user,
acting_user: username acting_user: username
).to_a[0] ).map(&:id)
UserDestroyer.new(admin).destroy(user, delete_posts: true) UserDestroyer.new(admin).destroy(user, delete_posts: true)
log.reload details = UserHistory.where(id: ids).map(&:details)
expect(log.details).to include(username) expect(details).to contain_exactly(
"\nuser_id: #{user.id}\nusername: #{username}",
"existing details\nuser_id: #{user.id}\nusername: #{username}"
)
end end
end end