From 6ddf0432e71aea7c62f1ee7946da5538d2526a13 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Mon, 3 Dec 2018 01:32:08 +0100 Subject: Improve account suspension speed and completeness (#9290) - Some associations were missing from the clean-up - Some attributes were not reset on suspension - Skip federation and streaming deletes when purging a dead domain - Move account association definitions to concern --- app/models/account.rb | 59 +++++++++-------------------- app/models/concerns/account_associations.rb | 53 ++++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 42 deletions(-) create mode 100644 app/models/concerns/account_associations.rb (limited to 'app/models') diff --git a/app/models/account.rb b/app/models/account.rb index f25263306..fb089de90 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -49,6 +49,7 @@ class Account < ApplicationRecord USERNAME_RE = /[a-z0-9_]+([a-z0-9_\.-]+[a-z0-9_]+)?/i MENTION_RE = /(?<=^|[^\/[:word:]])@((#{USERNAME_RE})(?:@[a-z0-9\.\-]+[a-z0-9]+)?)/i + include AccountAssociations include AccountAvatar include AccountFinderConcern include AccountHeader @@ -59,9 +60,6 @@ class Account < ApplicationRecord enum protocol: [:ostatus, :activitypub] - # Local users - has_one :user, inverse_of: :account - validates :username, presence: true # Remote user validations @@ -76,45 +74,6 @@ class Account < ApplicationRecord validates :note, length: { maximum: 160 }, if: -> { local? && will_save_change_to_note? } validates :fields, length: { maximum: 4 }, if: -> { local? && will_save_change_to_fields? } - # Timelines - has_many :stream_entries, inverse_of: :account, dependent: :destroy - has_many :statuses, inverse_of: :account, dependent: :destroy - has_many :favourites, inverse_of: :account, dependent: :destroy - has_many :mentions, inverse_of: :account, dependent: :destroy - has_many :notifications, inverse_of: :account, dependent: :destroy - - # Pinned statuses - has_many :status_pins, inverse_of: :account, dependent: :destroy - has_many :pinned_statuses, -> { reorder('status_pins.created_at DESC') }, through: :status_pins, class_name: 'Status', source: :status - - # Endorsements - has_many :account_pins, inverse_of: :account, dependent: :destroy - has_many :endorsed_accounts, through: :account_pins, class_name: 'Account', source: :target_account - - # Media - has_many :media_attachments, dependent: :destroy - - # PuSH subscriptions - has_many :subscriptions, dependent: :destroy - - # Report relationships - has_many :reports - has_many :targeted_reports, class_name: 'Report', foreign_key: :target_account_id - - has_many :report_notes, dependent: :destroy - has_many :custom_filters, inverse_of: :account, dependent: :destroy - - # Moderation notes - has_many :account_moderation_notes, dependent: :destroy - has_many :targeted_moderation_notes, class_name: 'AccountModerationNote', foreign_key: :target_account_id, dependent: :destroy - - # Lists - has_many :list_accounts, inverse_of: :account, dependent: :destroy - has_many :lists, through: :list_accounts - - # Account migrations - belongs_to :moved_to_account, class_name: 'Account', optional: true - scope :remote, -> { where.not(domain: nil) } scope :local, -> { where(domain: nil) } scope :expiring, ->(time) { remote.where.not(subscription_expires_at: nil).where('subscription_expires_at < ?', time) } @@ -452,6 +411,7 @@ class Account < ApplicationRecord before_create :generate_keys before_validation :normalize_domain before_validation :prepare_contents, if: :local? + before_destroy :clean_feed_manager private @@ -477,4 +437,19 @@ class Account < ApplicationRecord def emojifiable_text [note, display_name, fields.map(&:value)].join(' ') end + + def clean_feed_manager + reblog_key = FeedManager.instance.key(:home, id, 'reblogs') + reblogged_id_set = Redis.current.zrange(reblog_key, 0, -1) + + Redis.current.pipelined do + Redis.current.del(FeedManager.instance.key(:home, id)) + Redis.current.del(reblog_key) + + reblogged_id_set.each do |reblogged_id| + reblog_set_key = FeedManager.instance.key(:home, id, "reblogs:#{reblogged_id}") + Redis.current.del(reblog_set_key) + end + end + end end diff --git a/app/models/concerns/account_associations.rb b/app/models/concerns/account_associations.rb new file mode 100644 index 000000000..0f7482fa6 --- /dev/null +++ b/app/models/concerns/account_associations.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +module AccountAssociations + extend ActiveSupport::Concern + + included do + # Local users + has_one :user, inverse_of: :account, dependent: :destroy + + # Timelines + has_many :stream_entries, inverse_of: :account, dependent: :destroy + has_many :statuses, inverse_of: :account, dependent: :destroy + has_many :favourites, inverse_of: :account, dependent: :destroy + has_many :mentions, inverse_of: :account, dependent: :destroy + has_many :notifications, inverse_of: :account, dependent: :destroy + has_many :conversations, class_name: 'AccountConversation', dependent: :destroy, inverse_of: :account + + # Pinned statuses + has_many :status_pins, inverse_of: :account, dependent: :destroy + has_many :pinned_statuses, -> { reorder('status_pins.created_at DESC') }, through: :status_pins, class_name: 'Status', source: :status + + # Endorsements + has_many :account_pins, inverse_of: :account, dependent: :destroy + has_many :endorsed_accounts, through: :account_pins, class_name: 'Account', source: :target_account + + # Media + has_many :media_attachments, dependent: :destroy + + # PuSH subscriptions + has_many :subscriptions, dependent: :destroy + + # Report relationships + has_many :reports, dependent: :destroy, inverse_of: :account + has_many :targeted_reports, class_name: 'Report', foreign_key: :target_account_id, dependent: :destroy, inverse_of: :target_account + + has_many :report_notes, dependent: :destroy + has_many :custom_filters, inverse_of: :account, dependent: :destroy + + # Moderation notes + has_many :account_moderation_notes, dependent: :destroy, inverse_of: :account + has_many :targeted_moderation_notes, class_name: 'AccountModerationNote', foreign_key: :target_account_id, dependent: :destroy, inverse_of: :target_account + + # Lists (that the account is on, not owned by the account) + has_many :list_accounts, inverse_of: :account, dependent: :destroy + has_many :lists, through: :list_accounts + + # Lists (owned by the account) + has_many :owned_lists, class_name: 'List', dependent: :destroy, inverse_of: :account + + # Account migrations + belongs_to :moved_to_account, class_name: 'Account', optional: true + end +end -- cgit From 1a22eff1e00194e50751f2ea91ec7326ef15b4fc Mon Sep 17 00:00:00 2001 From: ThibG Date: Wed, 5 Dec 2018 22:51:12 +0100 Subject: Attempt fixing deadlocks by moving account stats update outside transaction (#9437) * Use `update_column` instead of `update_attribute` in callback `update_attribute` would normally cause callbacks to be called. Called from a callback, it seems to stop further callbacks from executing. `update_column` does the same work, but without calling callbacks or preventing other callbacks from executing. * Fix deadlocks by moving account stats update outside transaction --- app/models/status.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'app/models') diff --git a/app/models/status.rb b/app/models/status.rb index 2e894a6f1..0705ba4c1 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -236,8 +236,8 @@ class Status < ApplicationRecord update_status_stat!(key => [public_send(key) - 1, 0].max) end - after_create :increment_counter_caches - after_destroy :decrement_counter_caches + after_create_commit :increment_counter_caches + after_destroy_commit :decrement_counter_caches after_create_commit :store_uri, if: :local? after_create_commit :update_statistics, if: :local? @@ -426,7 +426,7 @@ class Status < ApplicationRecord end def store_uri - update_attribute(:uri, ActivityPub::TagManager.instance.uri_for(self)) if uri.nil? + update_column(:uri, ActivityPub::TagManager.instance.uri_for(self)) if uri.nil? end def prepare_contents -- cgit