Skip to content

Commit

Permalink
PERF: speed up user deletion logic
Browse files Browse the repository at this point in the history
Previously any user deletion would scan a very large number of tables

This avoids scans on 6 tables/indexes on delete
  • Loading branch information
SamSaffron committed Apr 26, 2019
1 parent be0ae2e commit c009a6a
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 1 deletion.
10 changes: 9 additions & 1 deletion app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,9 @@ class User < ActiveRecord::Base
has_many :user_warnings
has_many :user_archived_messages, dependent: :destroy
has_many :email_change_requests, dependent: :destroy
has_many :directory_items, dependent: :delete_all

# see before_destroy
has_many :directory_items
has_many :user_auth_tokens, dependent: :destroy
has_many :user_auth_token_logs, dependent: :destroy

Expand Down Expand Up @@ -141,6 +143,12 @@ class User < ActiveRecord::Base
# we need to bypass the default scope here, which appears not bypassed for :delete_all
# however :destroy it is bypassed
PostAction.with_deleted.where(user_id: self.id).delete_all

# This is a perf optimisation to ensure we hit the index
# without this we need to scan a much larger number of rows
DirectoryItem.where(user_id: self.id)
.where('period_type in (?)', DirectoryItem.period_types.values)
.delete_all
end

# Skip validating email, for example from a particular auth provider plugin
Expand Down
10 changes: 10 additions & 0 deletions db/migrate/20190426074404_add_missing_user_destroyer_indexes.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
class AddMissingUserDestroyerIndexes < ActiveRecord::Migration[5.2]
def change
# these indexes are required to make deletions of users fast
add_index :user_actions, [:target_user_id], where: 'target_user_id IS NOT NULL'
add_index :post_actions, [:user_id]
add_index :user_uploads, [:user_id, :upload_id]

This comment has been minimized.

Copy link
@tgxworld

tgxworld Apr 27, 2019

Contributor

This is interesting. Do you know why the unique index wouldn't be used here? I tried it out locally as well and the planner doesn't opt for the unique index.

This comment has been minimized.

Copy link
@SamSaffron

SamSaffron Apr 27, 2019

Author Member

The index on upload_id, user_id is useless for this purpose cause it it in reverse order. This is for finding all the uploads by a user_id.

add_index :user_auth_token_logs, [:user_id]
add_index :topic_link, [:user_id]
end
end

0 comments on commit c009a6a

Please sign in to comment.