From f665901e3c0930fb8b3741f6bc6f6a15dd0343f6 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Sun, 6 Oct 2019 22:11:17 +0200 Subject: Fix performance of home feed regeneration (#12084) Fetching statuses from all followed accounts at once takes too long within Postgres. Fetching them one by one and merging in Ruby could be a lot less resource-intensive Because the query for dynamically fetching the home timeline is so heavy, we can no longer offer it when the home timeline is missing --- app/lib/feed_manager.rb | 106 +++++++++++++++++++++++++++++++++++------------- 1 file changed, 78 insertions(+), 28 deletions(-) (limited to 'app/lib') diff --git a/app/lib/feed_manager.rb b/app/lib/feed_manager.rb index 871ec5c19..d8b486b60 100644 --- a/app/lib/feed_manager.rb +++ b/app/lib/feed_manager.rb @@ -19,7 +19,7 @@ class FeedManager def filter?(timeline_type, status, receiver_id) if timeline_type == :home - filter_from_home?(status, receiver_id) + filter_from_home?(status, receiver_id, build_crutches(receiver_id, [status])) elsif timeline_type == :mentions filter_from_mentions?(status, receiver_id) else @@ -29,6 +29,7 @@ class FeedManager def push_to_home(account, status) return false unless add_to_feed(:home, account.id, status, account.user&.aggregates_reblogs?) + trim(:home, account.id) PushUpdateWorker.perform_async(account.id, status.id, "timeline:#{account.id}") if push_update_required?("timeline:#{account.id}") true @@ -36,6 +37,7 @@ class FeedManager def unpush_from_home(account, status) return false unless remove_from_feed(:home, account.id, status, account.user&.aggregates_reblogs?) + redis.publish("timeline:#{account.id}", Oj.dump(event: :delete, payload: status.id.to_s)) true end @@ -46,7 +48,9 @@ class FeedManager should_filter &&= !ListAccount.where(list_id: list.id, account_id: status.in_reply_to_account_id).exists? return false if should_filter end + return false unless add_to_feed(:list, list.id, status, list.account.user&.aggregates_reblogs?) + trim(:list, list.id) PushUpdateWorker.perform_async(list.account_id, status.id, "timeline:list:#{list.id}") if push_update_required?("timeline:list:#{list.id}") true @@ -54,6 +58,7 @@ class FeedManager def unpush_from_list(list, status) return false unless remove_from_feed(:list, list.id, status, list.account.user&.aggregates_reblogs?) + redis.publish("timeline:list:#{list.id}", Oj.dump(event: :delete, payload: status.id.to_s)) true end @@ -85,16 +90,21 @@ class FeedManager def merge_into_timeline(from_account, into_account) timeline_key = key(:home, into_account.id) - query = from_account.statuses.limit(FeedManager::MAX_ITEMS / 4) + aggregate = into_account.user&.aggregates_reblogs? + query = from_account.statuses.where(visibility: [:public, :unlisted, :private]).includes(:preloadable_poll, reblog: :account).limit(FeedManager::MAX_ITEMS / 4) if redis.zcard(timeline_key) >= FeedManager::MAX_ITEMS / 4 - oldest_home_score = redis.zrange(timeline_key, 0, 0, with_scores: true)&.first&.last&.to_i || 0 + oldest_home_score = redis.zrange(timeline_key, 0, 0, with_scores: true).first.last.to_i query = query.where('id > ?', oldest_home_score) end - query.each do |status| - next if status.direct_visibility? || status.limited_visibility? || filter?(:home, status, into_account) - add_to_feed(:home, into_account.id, status, into_account.user&.aggregates_reblogs?) + statuses = query.to_a + crutches = build_crutches(into_account.id, statuses) + + statuses.each do |status| + next if filter_from_home?(status, into_account, crutches) + + add_to_feed(:home, into_account.id, status, aggregate) end trim(:home, into_account.id) @@ -120,24 +130,35 @@ class FeedManager end def populate_feed(account) - added = 0 - limit = FeedManager::MAX_ITEMS / 2 - max_id = nil + limit = FeedManager::MAX_ITEMS / 2 + aggregate = account.user&.aggregates_reblogs? + timeline_key = key(:home, account.id) - loop do - statuses = Status.as_home_timeline(account) - .paginate_by_max_id(limit, max_id) + account.statuses.where.not(visibility: :direct).limit(limit).each do |status| + add_to_feed(:home, account.id, status, aggregate) + end - break if statuses.empty? + account.following.includes(:account_stat).find_each do |target_account| + if redis.zcard(timeline_key) >= limit + oldest_home_score = redis.zrange(timeline_key, 0, 0, with_scores: true).first.last.to_i + last_status_score = Mastodon::Snowflake.id_at(account.last_status_at) - statuses.each do |status| - next if filter_from_home?(status, account) - added += 1 if add_to_feed(:home, account.id, status, account.user&.aggregates_reblogs?) + # If the feed is full and this account has not posted more recently + # than the last item on the feed, then we can skip the whole account + # because none of its statuses would stay on the feed anyway + next if last_status_score < oldest_home_score end - break unless added.zero? + statuses = target_account.statuses.where(visibility: [:public, :unlisted, :private]).includes(:preloadable_poll, reblog: :account).limit(limit) + crutches = build_crutches(account.id, statuses) + + statuses.each do |status| + next if filter_from_home?(status, account, crutches) + + add_to_feed(:home, account.id, status, aggregate) + end - max_id = statuses.last.id + trim(:home, account.id) end end @@ -152,31 +173,33 @@ class FeedManager (context == :home ? Mute.where(account_id: receiver_id, target_account_id: account_ids).any? : Mute.where(account_id: receiver_id, target_account_id: account_ids, hide_notifications: true).any?) end - def filter_from_home?(status, receiver_id) + def filter_from_home?(status, receiver_id, crutches) return false if receiver_id == status.account_id return true if status.reply? && (status.in_reply_to_id.nil? || status.in_reply_to_account_id.nil?) return true if phrase_filtered?(status, receiver_id, :home) - check_for_blocks = status.active_mentions.pluck(:account_id) + check_for_blocks = crutches[:active_mentions][status.id] || [] check_for_blocks.concat([status.account_id]) if status.reblog? check_for_blocks.concat([status.reblog.account_id]) - check_for_blocks.concat(status.reblog.active_mentions.pluck(:account_id)) + check_for_blocks.concat(crutches[:active_mentions][status.reblog_of_id] || []) end - return true if blocks_or_mutes?(receiver_id, check_for_blocks, :home) + return true if check_for_blocks.any? { |target_account_id| crutches[:blocking][target_account_id] || crutches[:muting][target_account_id] } if status.reply? && !status.in_reply_to_account_id.nil? # Filter out if it's a reply - should_filter = !Follow.where(account_id: receiver_id, target_account_id: status.in_reply_to_account_id).exists? # and I'm not following the person it's a reply to + should_filter = !crutches[:following][status.in_reply_to_account_id] # and I'm not following the person it's a reply to should_filter &&= receiver_id != status.in_reply_to_account_id # and it's not a reply to me should_filter &&= status.account_id != status.in_reply_to_account_id # and it's not a self-reply - return should_filter + + return !!should_filter elsif status.reblog? # Filter out a reblog - should_filter = Follow.where(account_id: receiver_id, target_account_id: status.account_id, show_reblogs: false).exists? # if the reblogger's reblogs are suppressed - should_filter ||= Block.where(account_id: status.reblog.account_id, target_account_id: receiver_id).exists? # or if the author of the reblogged status is blocking me - should_filter ||= AccountDomainBlock.where(account_id: receiver_id, domain: status.reblog.account.domain).exists? # or the author's domain is blocked - return should_filter + should_filter = crutches[:hiding_reblogs][status.account_id] # if the reblogger's reblogs are suppressed + should_filter ||= crutches[:blocked_by][status.reblog.account_id] # or if the author of the reblogged status is blocking me + should_filter ||= crutches[:domain_blocking][status.reblog.account.domain] # or the author's domain is blocked + + return !!should_filter end false @@ -308,4 +331,31 @@ class FeedManager redis.zrem(timeline_key, status.id) end + + def build_crutches(receiver_id, statuses) + crutches = {} + + crutches[:active_mentions] = Mention.active.where(status_id: statuses.flat_map { |s| [s.id, s.reblog_of_id] }.compact).pluck(:status_id, :account_id).each_with_object({}) { |(id, account_id), mapping| (mapping[id] ||= []).push(account_id) } + + check_for_blocks = statuses.flat_map do |s| + arr = crutches[:active_mentions][s.id] || [] + arr.concat([s.account_id]) + + if s.reblog? + arr.concat([s.reblog.account_id]) + arr.concat(crutches[:active_mentions][s.reblog_of_id] || []) + end + + arr + end + + crutches[:following] = Follow.where(account_id: receiver_id, target_account_id: statuses.map(&:in_reply_to_account_id).compact).pluck(:target_account_id).each_with_object({}) { |id, mapping| mapping[id] = true } + crutches[:hiding_reblogs] = Follow.where(account_id: receiver_id, target_account_id: statuses.map { |s| s.account_id if s.reblog? }.compact, show_reblogs: false).pluck(:target_account_id).each_with_object({}) { |id, mapping| mapping[id] = true } + crutches[:blocking] = Block.where(account_id: receiver_id, target_account_id: check_for_blocks).pluck(:target_account_id).each_with_object({}) { |id, mapping| mapping[id] = true } + crutches[:muting] = Mute.where(account_id: receiver_id, target_account_id: check_for_blocks).pluck(:target_account_id).each_with_object({}) { |id, mapping| mapping[id] = true } + crutches[:domain_blocking] = AccountDomainBlock.where(account_id: receiver_id, domain: statuses.map { |s| s.reblog&.account&.domain }.compact).pluck(:domain).each_with_object({}) { |domain, mapping| mapping[domain] = true } + crutches[:blocked_by] = Block.where(target_account_id: receiver_id, account_id: statuses.map { |s| s.reblog&.account_id }.compact).pluck(:account_id).each_with_object({}) { |id, mapping| mapping[id] = true } + + crutches + end end -- cgit From b5f7e12817356b9b1795ab0187fe08d07f13a485 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Wed, 9 Oct 2019 07:11:23 +0200 Subject: Remove auto-silence behaviour from spam check (#12117) Fix #12113 --- app/lib/spam_check.rb | 7 +------ app/models/account.rb | 2 +- app/models/admin/account_action.rb | 12 ++++++++++++ config/locales/en.yml | 2 +- spec/lib/spam_check_spec.rb | 4 ---- 5 files changed, 15 insertions(+), 12 deletions(-) (limited to 'app/lib') diff --git a/app/lib/spam_check.rb b/app/lib/spam_check.rb index 441697364..235e44230 100644 --- a/app/lib/spam_check.rb +++ b/app/lib/spam_check.rb @@ -44,7 +44,6 @@ class SpamCheck end def flag! - auto_silence_account! auto_report_status! end @@ -134,17 +133,13 @@ class SpamCheck text.gsub(/\s+/, ' ').strip end - def auto_silence_account! - @account.silence! - end - def auto_report_status! status_ids = Status.where(visibility: %i(public unlisted)).where(id: matching_status_ids).pluck(:id) + [@status.id] if @status.distributable? ReportService.new.call(Account.representative, @account, status_ids: status_ids, comment: I18n.t('spam_check.spam_detected_and_silenced')) end def already_flagged? - @account.silenced? + @account.silenced? || @account.targeted_reports.unresolved.where(account_id: -99).exists? end def trusted? diff --git a/app/models/account.rb b/app/models/account.rb index 2f43f337f..05936def3 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -198,7 +198,7 @@ class Account < ApplicationRecord end def unsilence! - update!(silenced_at: nil, trust_level: trust_level == TRUST_LEVELS[:untrusted] ? TRUST_LEVELS[:trusted] : trust_level) + update!(silenced_at: nil) end def suspended? diff --git a/app/models/admin/account_action.rb b/app/models/admin/account_action.rb index b30a82369..e9da003a3 100644 --- a/app/models/admin/account_action.rb +++ b/app/models/admin/account_action.rb @@ -62,6 +62,8 @@ class Admin::AccountAction def process_action! case type + when 'none' + handle_resolve! when 'disable' handle_disable! when 'silence' @@ -103,6 +105,16 @@ class Admin::AccountAction end end + def handle_resolve! + if with_report? && report.account_id == -99 && target_account.trust_level == Account::TRUST_LEVELS[:untrusted] + # This is an automated report and it is being dismissed, so it's + # a false positive, in which case update the account's trust level + # to prevent further spam checks + + target_account.update(trust_level: Account::TRUST_LEVELS[:trusted]) + end + end + def handle_disable! authorize(target_account.user, :disable?) log_action(:disable, target_account.user) diff --git a/config/locales/en.yml b/config/locales/en.yml index 0e8ee6a76..1ffc99eb3 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -497,7 +497,7 @@ en: title: Custom terms of service site_title: Server name spam_check_enabled: - desc_html: Mastodon can auto-silence and auto-report accounts that send repeated unsolicited messages. There may be false positives. + desc_html: Mastodon can auto-report accounts that send repeated unsolicited messages. There may be false positives. title: Anti-spam automation thumbnail: desc_html: Used for previews via OpenGraph and API. 1200x630px recommended diff --git a/spec/lib/spam_check_spec.rb b/spec/lib/spam_check_spec.rb index 4cae46111..d4d66a499 100644 --- a/spec/lib/spam_check_spec.rb +++ b/spec/lib/spam_check_spec.rb @@ -181,10 +181,6 @@ RSpec.describe SpamCheck do described_class.new(status2).flag! end - it 'silences the account' do - expect(sender.silenced?).to be true - end - it 'creates a report about the account' do expect(sender.targeted_reports.unresolved.count).to eq 1 end -- cgit