From 9014367bd87e07941134259d87f4d2c327ff37d4 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Tue, 31 Mar 2020 21:59:03 +0200 Subject: [PATCH] Fix background jobs not using locks like they are supposed to (#13361) Also: - Fix locks not being removed when jobs go to the dead job queue - Add UI for managing locks to the Sidekiq dashboard - Remove unused Sidekiq workers Fix #13349 --- app/workers/activitypub/distribute_poll_update_worker.rb | 2 +- .../synchronize_featured_collection_worker.rb | 2 +- app/workers/after_remote_follow_request_worker.rb | 9 --------- app/workers/after_remote_follow_worker.rb | 9 --------- app/workers/notification_worker.rb | 9 --------- app/workers/poll_expiration_notify_worker.rb | 2 +- app/workers/processing_worker.rb | 9 --------- app/workers/publish_scheduled_status_worker.rb | 2 +- app/workers/pubsubhubbub/confirmation_worker.rb | 9 --------- app/workers/pubsubhubbub/delivery_worker.rb | 9 --------- app/workers/pubsubhubbub/distribution_worker.rb | 9 --------- app/workers/pubsubhubbub/raw_distribution_worker.rb | 9 --------- app/workers/pubsubhubbub/subscribe_worker.rb | 9 --------- app/workers/pubsubhubbub/unsubscribe_worker.rb | 9 --------- app/workers/regeneration_worker.rb | 2 +- app/workers/remote_profile_update_worker.rb | 9 --------- app/workers/resolve_account_worker.rb | 2 +- app/workers/salmon_worker.rb | 9 --------- app/workers/scheduler/backup_cleanup_scheduler.rb | 2 +- app/workers/scheduler/doorkeeper_cleanup_scheduler.rb | 2 +- app/workers/scheduler/email_scheduler.rb | 2 +- app/workers/scheduler/feed_cleanup_scheduler.rb | 2 +- app/workers/scheduler/ip_cleanup_scheduler.rb | 2 +- app/workers/scheduler/media_cleanup_scheduler.rb | 2 +- app/workers/scheduler/pghero_scheduler.rb | 2 +- app/workers/scheduler/scheduled_statuses_scheduler.rb | 2 +- app/workers/scheduler/subscriptions_cleanup_scheduler.rb | 9 --------- app/workers/scheduler/subscriptions_scheduler.rb | 9 --------- app/workers/scheduler/trending_tags_scheduler.rb | 2 +- app/workers/scheduler/user_cleanup_scheduler.rb | 2 +- app/workers/verify_account_links_worker.rb | 2 +- config/initializers/sidekiq.rb | 5 +++++ config/routes.rb | 2 +- spec/services/import_service_spec.rb | 8 -------- 34 files changed, 23 insertions(+), 152 deletions(-) delete mode 100644 app/workers/after_remote_follow_request_worker.rb delete mode 100644 app/workers/after_remote_follow_worker.rb delete mode 100644 app/workers/notification_worker.rb delete mode 100644 app/workers/processing_worker.rb delete mode 100644 app/workers/pubsubhubbub/confirmation_worker.rb delete mode 100644 app/workers/pubsubhubbub/delivery_worker.rb delete mode 100644 app/workers/pubsubhubbub/distribution_worker.rb delete mode 100644 app/workers/pubsubhubbub/raw_distribution_worker.rb delete mode 100644 app/workers/pubsubhubbub/subscribe_worker.rb delete mode 100644 app/workers/pubsubhubbub/unsubscribe_worker.rb delete mode 100644 app/workers/remote_profile_update_worker.rb delete mode 100644 app/workers/salmon_worker.rb delete mode 100644 app/workers/scheduler/subscriptions_cleanup_scheduler.rb delete mode 100644 app/workers/scheduler/subscriptions_scheduler.rb diff --git a/app/workers/activitypub/distribute_poll_update_worker.rb b/app/workers/activitypub/distribute_poll_update_worker.rb index 1e87fa4bf2a..25dee4ee259 100644 --- a/app/workers/activitypub/distribute_poll_update_worker.rb +++ b/app/workers/activitypub/distribute_poll_update_worker.rb @@ -4,7 +4,7 @@ class ActivityPub::DistributePollUpdateWorker include Sidekiq::Worker include Payloadable - sidekiq_options queue: 'push', unique: :until_executed, retry: 0 + sidekiq_options queue: 'push', lock: :until_executed, retry: 0 def perform(status_id) @status = Status.find(status_id) diff --git a/app/workers/activitypub/synchronize_featured_collection_worker.rb b/app/workers/activitypub/synchronize_featured_collection_worker.rb index 7b16d3426a2..7a0898e89de 100644 --- a/app/workers/activitypub/synchronize_featured_collection_worker.rb +++ b/app/workers/activitypub/synchronize_featured_collection_worker.rb @@ -3,7 +3,7 @@ class ActivityPub::SynchronizeFeaturedCollectionWorker include Sidekiq::Worker - sidekiq_options queue: 'pull', unique: :until_executed + sidekiq_options queue: 'pull', lock: :until_executed def perform(account_id) ActivityPub::FetchFeaturedCollectionService.new.call(Account.find(account_id)) diff --git a/app/workers/after_remote_follow_request_worker.rb b/app/workers/after_remote_follow_request_worker.rb deleted file mode 100644 index ce9c65834c3..00000000000 --- a/app/workers/after_remote_follow_request_worker.rb +++ /dev/null @@ -1,9 +0,0 @@ -# frozen_string_literal: true - -class AfterRemoteFollowRequestWorker - include Sidekiq::Worker - - sidekiq_options queue: 'pull', retry: 5 - - def perform(follow_request_id); end -end diff --git a/app/workers/after_remote_follow_worker.rb b/app/workers/after_remote_follow_worker.rb deleted file mode 100644 index d9719f2bf8a..00000000000 --- a/app/workers/after_remote_follow_worker.rb +++ /dev/null @@ -1,9 +0,0 @@ -# frozen_string_literal: true - -class AfterRemoteFollowWorker - include Sidekiq::Worker - - sidekiq_options queue: 'pull', retry: 5 - - def perform(follow_id); end -end diff --git a/app/workers/notification_worker.rb b/app/workers/notification_worker.rb deleted file mode 100644 index 1c0f001cf20..00000000000 --- a/app/workers/notification_worker.rb +++ /dev/null @@ -1,9 +0,0 @@ -# frozen_string_literal: true - -class NotificationWorker - include Sidekiq::Worker - - sidekiq_options queue: 'push', retry: 5 - - def perform(xml, source_account_id, target_account_id); end -end diff --git a/app/workers/poll_expiration_notify_worker.rb b/app/workers/poll_expiration_notify_worker.rb index e08f0c2496f..64b4cbd7e46 100644 --- a/app/workers/poll_expiration_notify_worker.rb +++ b/app/workers/poll_expiration_notify_worker.rb @@ -3,7 +3,7 @@ class PollExpirationNotifyWorker include Sidekiq::Worker - sidekiq_options unique: :until_executed + sidekiq_options lock: :until_executed def perform(poll_id) poll = Poll.find(poll_id) diff --git a/app/workers/processing_worker.rb b/app/workers/processing_worker.rb deleted file mode 100644 index cf3bd83979a..00000000000 --- a/app/workers/processing_worker.rb +++ /dev/null @@ -1,9 +0,0 @@ -# frozen_string_literal: true - -class ProcessingWorker - include Sidekiq::Worker - - sidekiq_options backtrace: true - - def perform(account_id, body); end -end diff --git a/app/workers/publish_scheduled_status_worker.rb b/app/workers/publish_scheduled_status_worker.rb index 850610c4e3c..ce42f7be7c6 100644 --- a/app/workers/publish_scheduled_status_worker.rb +++ b/app/workers/publish_scheduled_status_worker.rb @@ -3,7 +3,7 @@ class PublishScheduledStatusWorker include Sidekiq::Worker - sidekiq_options unique: :until_executed + sidekiq_options lock: :until_executed def perform(scheduled_status_id) scheduled_status = ScheduledStatus.find(scheduled_status_id) diff --git a/app/workers/pubsubhubbub/confirmation_worker.rb b/app/workers/pubsubhubbub/confirmation_worker.rb deleted file mode 100644 index 783a8c95fb0..00000000000 --- a/app/workers/pubsubhubbub/confirmation_worker.rb +++ /dev/null @@ -1,9 +0,0 @@ -# frozen_string_literal: true - -class Pubsubhubbub::ConfirmationWorker - include Sidekiq::Worker - - sidekiq_options queue: 'push', retry: false - - def perform(subscription_id, mode, secret = nil, lease_seconds = nil); end -end diff --git a/app/workers/pubsubhubbub/delivery_worker.rb b/app/workers/pubsubhubbub/delivery_worker.rb deleted file mode 100644 index 1260060bd55..00000000000 --- a/app/workers/pubsubhubbub/delivery_worker.rb +++ /dev/null @@ -1,9 +0,0 @@ -# frozen_string_literal: true - -class Pubsubhubbub::DeliveryWorker - include Sidekiq::Worker - - sidekiq_options queue: 'push', retry: 3, dead: false - - def perform(subscription_id, payload); end -end diff --git a/app/workers/pubsubhubbub/distribution_worker.rb b/app/workers/pubsubhubbub/distribution_worker.rb deleted file mode 100644 index 75bac5d6fad..00000000000 --- a/app/workers/pubsubhubbub/distribution_worker.rb +++ /dev/null @@ -1,9 +0,0 @@ -# frozen_string_literal: true - -class Pubsubhubbub::DistributionWorker - include Sidekiq::Worker - - sidekiq_options queue: 'push' - - def perform(stream_entry_ids); end -end diff --git a/app/workers/pubsubhubbub/raw_distribution_worker.rb b/app/workers/pubsubhubbub/raw_distribution_worker.rb deleted file mode 100644 index ece9c80ac5d..00000000000 --- a/app/workers/pubsubhubbub/raw_distribution_worker.rb +++ /dev/null @@ -1,9 +0,0 @@ -# frozen_string_literal: true - -class Pubsubhubbub::RawDistributionWorker - include Sidekiq::Worker - - sidekiq_options queue: 'push' - - def perform(xml, source_account_id); end -end diff --git a/app/workers/pubsubhubbub/subscribe_worker.rb b/app/workers/pubsubhubbub/subscribe_worker.rb deleted file mode 100644 index b861b5e67a1..00000000000 --- a/app/workers/pubsubhubbub/subscribe_worker.rb +++ /dev/null @@ -1,9 +0,0 @@ -# frozen_string_literal: true - -class Pubsubhubbub::SubscribeWorker - include Sidekiq::Worker - - sidekiq_options queue: 'push', retry: 10, unique: :until_executed, dead: false - - def perform(account_id); end -end diff --git a/app/workers/pubsubhubbub/unsubscribe_worker.rb b/app/workers/pubsubhubbub/unsubscribe_worker.rb deleted file mode 100644 index 0c1c263f6e0..00000000000 --- a/app/workers/pubsubhubbub/unsubscribe_worker.rb +++ /dev/null @@ -1,9 +0,0 @@ -# frozen_string_literal: true - -class Pubsubhubbub::UnsubscribeWorker - include Sidekiq::Worker - - sidekiq_options queue: 'push', retry: false, unique: :until_executed, dead: false - - def perform(account_id); end -end diff --git a/app/workers/regeneration_worker.rb b/app/workers/regeneration_worker.rb index 5c6a040bd58..5c13c894fea 100644 --- a/app/workers/regeneration_worker.rb +++ b/app/workers/regeneration_worker.rb @@ -3,7 +3,7 @@ class RegenerationWorker include Sidekiq::Worker - sidekiq_options unique: :until_executed + sidekiq_options lock: :until_executed def perform(account_id, _ = :home) account = Account.find(account_id) diff --git a/app/workers/remote_profile_update_worker.rb b/app/workers/remote_profile_update_worker.rb deleted file mode 100644 index 01e8daf8f28..00000000000 --- a/app/workers/remote_profile_update_worker.rb +++ /dev/null @@ -1,9 +0,0 @@ -# frozen_string_literal: true - -class RemoteProfileUpdateWorker - include Sidekiq::Worker - - sidekiq_options queue: 'pull' - - def perform(account_id, body, resubscribe); end -end diff --git a/app/workers/resolve_account_worker.rb b/app/workers/resolve_account_worker.rb index cd7c4d7dda5..2b5be6d1b21 100644 --- a/app/workers/resolve_account_worker.rb +++ b/app/workers/resolve_account_worker.rb @@ -3,7 +3,7 @@ class ResolveAccountWorker include Sidekiq::Worker - sidekiq_options queue: 'pull', unique: :until_executed + sidekiq_options queue: 'pull', lock: :until_executed def perform(uri) ResolveAccountService.new.call(uri) diff --git a/app/workers/salmon_worker.rb b/app/workers/salmon_worker.rb deleted file mode 100644 index 10200b06cf9..00000000000 --- a/app/workers/salmon_worker.rb +++ /dev/null @@ -1,9 +0,0 @@ -# frozen_string_literal: true - -class SalmonWorker - include Sidekiq::Worker - - sidekiq_options backtrace: true - - def perform(account_id, body); end -end diff --git a/app/workers/scheduler/backup_cleanup_scheduler.rb b/app/workers/scheduler/backup_cleanup_scheduler.rb index d436606999f..d69ca2556ae 100644 --- a/app/workers/scheduler/backup_cleanup_scheduler.rb +++ b/app/workers/scheduler/backup_cleanup_scheduler.rb @@ -3,7 +3,7 @@ class Scheduler::BackupCleanupScheduler include Sidekiq::Worker - sidekiq_options unique: :until_executed, retry: 0 + sidekiq_options lock: :until_executed, retry: 0 def perform old_backups.reorder(nil).find_each(&:destroy!) diff --git a/app/workers/scheduler/doorkeeper_cleanup_scheduler.rb b/app/workers/scheduler/doorkeeper_cleanup_scheduler.rb index e5e5f6bc42f..94788a85bb8 100644 --- a/app/workers/scheduler/doorkeeper_cleanup_scheduler.rb +++ b/app/workers/scheduler/doorkeeper_cleanup_scheduler.rb @@ -3,7 +3,7 @@ class Scheduler::DoorkeeperCleanupScheduler include Sidekiq::Worker - sidekiq_options unique: :until_executed, retry: 0 + sidekiq_options lock: :until_executed, retry: 0 def perform Doorkeeper::AccessToken.where('revoked_at IS NOT NULL').where('revoked_at < NOW()').delete_all diff --git a/app/workers/scheduler/email_scheduler.rb b/app/workers/scheduler/email_scheduler.rb index 1eeeee412d7..9a735552482 100644 --- a/app/workers/scheduler/email_scheduler.rb +++ b/app/workers/scheduler/email_scheduler.rb @@ -3,7 +3,7 @@ class Scheduler::EmailScheduler include Sidekiq::Worker - sidekiq_options unique: :until_executed, retry: 0 + sidekiq_options lock: :until_executed, retry: 0 FREQUENCY = 7.days.freeze SIGN_IN_OFFSET = 1.day.freeze diff --git a/app/workers/scheduler/feed_cleanup_scheduler.rb b/app/workers/scheduler/feed_cleanup_scheduler.rb index bf5e2075700..458fe6193e5 100644 --- a/app/workers/scheduler/feed_cleanup_scheduler.rb +++ b/app/workers/scheduler/feed_cleanup_scheduler.rb @@ -4,7 +4,7 @@ class Scheduler::FeedCleanupScheduler include Sidekiq::Worker include Redisable - sidekiq_options unique: :until_executed, retry: 0 + sidekiq_options lock: :until_executed, retry: 0 def perform clean_home_feeds! diff --git a/app/workers/scheduler/ip_cleanup_scheduler.rb b/app/workers/scheduler/ip_cleanup_scheduler.rb index 4f44078d8e9..6d38b52a298 100644 --- a/app/workers/scheduler/ip_cleanup_scheduler.rb +++ b/app/workers/scheduler/ip_cleanup_scheduler.rb @@ -5,7 +5,7 @@ class Scheduler::IpCleanupScheduler RETENTION_PERIOD = 1.year - sidekiq_options unique: :until_executed, retry: 0 + sidekiq_options lock: :until_executed, retry: 0 def perform time_ago = RETENTION_PERIOD.ago diff --git a/app/workers/scheduler/media_cleanup_scheduler.rb b/app/workers/scheduler/media_cleanup_scheduler.rb index fb01aa70c99..671ebf6e02d 100644 --- a/app/workers/scheduler/media_cleanup_scheduler.rb +++ b/app/workers/scheduler/media_cleanup_scheduler.rb @@ -3,7 +3,7 @@ class Scheduler::MediaCleanupScheduler include Sidekiq::Worker - sidekiq_options unique: :until_executed, retry: 0 + sidekiq_options lock: :until_executed, retry: 0 def perform unattached_media.find_each(&:destroy) diff --git a/app/workers/scheduler/pghero_scheduler.rb b/app/workers/scheduler/pghero_scheduler.rb index 4453bf2cd00..cf557004864 100644 --- a/app/workers/scheduler/pghero_scheduler.rb +++ b/app/workers/scheduler/pghero_scheduler.rb @@ -3,7 +3,7 @@ class Scheduler::PgheroScheduler include Sidekiq::Worker - sidekiq_options unique: :until_executed, retry: 0 + sidekiq_options lock: :until_executed, retry: 0 def perform PgHero.capture_space_stats diff --git a/app/workers/scheduler/scheduled_statuses_scheduler.rb b/app/workers/scheduler/scheduled_statuses_scheduler.rb index 9cfe949dec5..25df3c07dee 100644 --- a/app/workers/scheduler/scheduled_statuses_scheduler.rb +++ b/app/workers/scheduler/scheduled_statuses_scheduler.rb @@ -3,7 +3,7 @@ class Scheduler::ScheduledStatusesScheduler include Sidekiq::Worker - sidekiq_options unique: :until_executed, retry: 0 + sidekiq_options lock: :until_executed, retry: 0 def perform publish_scheduled_statuses! diff --git a/app/workers/scheduler/subscriptions_cleanup_scheduler.rb b/app/workers/scheduler/subscriptions_cleanup_scheduler.rb deleted file mode 100644 index 75fe681a9c3..00000000000 --- a/app/workers/scheduler/subscriptions_cleanup_scheduler.rb +++ /dev/null @@ -1,9 +0,0 @@ -# frozen_string_literal: true - -class Scheduler::SubscriptionsCleanupScheduler - include Sidekiq::Worker - - sidekiq_options unique: :until_executed, retry: 0 - - def perform; end -end diff --git a/app/workers/scheduler/subscriptions_scheduler.rb b/app/workers/scheduler/subscriptions_scheduler.rb deleted file mode 100644 index 6903cadc760..00000000000 --- a/app/workers/scheduler/subscriptions_scheduler.rb +++ /dev/null @@ -1,9 +0,0 @@ -# frozen_string_literal: true - -class Scheduler::SubscriptionsScheduler - include Sidekiq::Worker - - sidekiq_options unique: :until_executed, retry: 0 - - def perform; end -end diff --git a/app/workers/scheduler/trending_tags_scheduler.rb b/app/workers/scheduler/trending_tags_scheduler.rb index 77f0d574757..e9891424e0a 100644 --- a/app/workers/scheduler/trending_tags_scheduler.rb +++ b/app/workers/scheduler/trending_tags_scheduler.rb @@ -3,7 +3,7 @@ class Scheduler::TrendingTagsScheduler include Sidekiq::Worker - sidekiq_options unique: :until_executed, retry: 0 + sidekiq_options lock: :until_executed, retry: 0 def perform TrendingTags.update! if Setting.trends diff --git a/app/workers/scheduler/user_cleanup_scheduler.rb b/app/workers/scheduler/user_cleanup_scheduler.rb index 881b911be75..6113edde17b 100644 --- a/app/workers/scheduler/user_cleanup_scheduler.rb +++ b/app/workers/scheduler/user_cleanup_scheduler.rb @@ -3,7 +3,7 @@ class Scheduler::UserCleanupScheduler include Sidekiq::Worker - sidekiq_options unique: :until_executed, retry: 0 + sidekiq_options lock: :until_executed, retry: 0 def perform User.where('confirmed_at is NULL AND confirmation_sent_at <= ?', 2.days.ago).reorder(nil).find_in_batches do |batch| diff --git a/app/workers/verify_account_links_worker.rb b/app/workers/verify_account_links_worker.rb index 901498583e8..8114d59bea3 100644 --- a/app/workers/verify_account_links_worker.rb +++ b/app/workers/verify_account_links_worker.rb @@ -3,7 +3,7 @@ class VerifyAccountLinksWorker include Sidekiq::Worker - sidekiq_options queue: 'pull', retry: false, unique: :until_executed + sidekiq_options queue: 'pull', retry: false, lock: :until_executed def perform(account_id) account = Account.find(account_id) diff --git a/config/initializers/sidekiq.rb b/config/initializers/sidekiq.rb index 288453a046c..f2733562fb1 100644 --- a/config/initializers/sidekiq.rb +++ b/config/initializers/sidekiq.rb @@ -13,6 +13,11 @@ Sidekiq.configure_server do |config| config.server_middleware do |chain| chain.add SidekiqErrorHandler end + + config.death_handlers << lambda do |job, _ex| + digest = job['lock_digest'] + SidekiqUniqueJobs::Digests.delete_by_digest(digest) if digest + end end Sidekiq.configure_client do |config| diff --git a/config/routes.rb b/config/routes.rb index 89d446d10f8..49ab851e2cf 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require 'sidekiq/web' +require 'sidekiq_unique_jobs/web' require 'sidekiq-scheduler/web' Sidekiq::Web.set :session_secret, Rails.application.secrets[:secret_key_base] diff --git a/spec/services/import_service_spec.rb b/spec/services/import_service_spec.rb index 5355133f400..7618e907618 100644 --- a/spec/services/import_service_spec.rb +++ b/spec/services/import_service_spec.rb @@ -91,10 +91,6 @@ RSpec.describe ImportService, type: :service do let(:csv) { attachment_fixture('mute-imports.txt') } - before do - allow(NotificationWorker).to receive(:perform_async) - end - describe 'when no accounts are followed' do let(:import) { Import.create(account: account, type: 'following', data: csv) } it 'follows the listed accounts, including boosts' do @@ -135,10 +131,6 @@ RSpec.describe ImportService, type: :service do let(:csv) { attachment_fixture('new-following-imports.txt') } - before do - allow(NotificationWorker).to receive(:perform_async) - end - describe 'when no accounts are followed' do let(:import) { Import.create(account: account, type: 'following', data: csv) } it 'follows the listed accounts, respecting boosts' do