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
2020-03-31 21:59:03 +02:00 · 2020-03-31 21:59:03 +02:00 · 9014367bd8
commit 9014367bd8
parent 1fb92037e4
34 changed files with 23 additions and 152 deletions
--- 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)
--- 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))
--- a/app/workers/after_remote_follow_request_worker.rb
+++ b/app/workers/after_remote_follow_request_worker.rb
@ -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
--- a/app/workers/after_remote_follow_worker.rb
+++ b/app/workers/after_remote_follow_worker.rb
@ -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
--- a/app/workers/notification_worker.rb
+++ b/app/workers/notification_worker.rb
@ -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
--- 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)
--- a/app/workers/processing_worker.rb
+++ b/app/workers/processing_worker.rb
@ -1,9 +0,0 @@
-# frozen_string_literal: true
-
-class ProcessingWorker
-  include Sidekiq::Worker
-
-  sidekiq_options backtrace: true
-
-  def perform(account_id, body); end
-end
--- 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)
--- a/app/workers/pubsubhubbub/confirmation_worker.rb
+++ b/app/workers/pubsubhubbub/confirmation_worker.rb
@ -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
--- a/app/workers/pubsubhubbub/delivery_worker.rb
+++ b/app/workers/pubsubhubbub/delivery_worker.rb
@ -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
--- a/app/workers/pubsubhubbub/distribution_worker.rb
+++ b/app/workers/pubsubhubbub/distribution_worker.rb
@ -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
--- a/app/workers/pubsubhubbub/raw_distribution_worker.rb
+++ b/app/workers/pubsubhubbub/raw_distribution_worker.rb
@ -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
--- a/app/workers/pubsubhubbub/subscribe_worker.rb
+++ b/app/workers/pubsubhubbub/subscribe_worker.rb
@ -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
--- a/app/workers/pubsubhubbub/unsubscribe_worker.rb
+++ b/app/workers/pubsubhubbub/unsubscribe_worker.rb
@ -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
--- 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)
--- a/app/workers/remote_profile_update_worker.rb
+++ b/app/workers/remote_profile_update_worker.rb
@ -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
--- 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)
--- a/app/workers/salmon_worker.rb
+++ b/app/workers/salmon_worker.rb
@ -1,9 +0,0 @@
-# frozen_string_literal: true
-
-class SalmonWorker
-  include Sidekiq::Worker
-
-  sidekiq_options backtrace: true
-
-  def perform(account_id, body); end
-end
--- 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!)
--- 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
--- 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
--- 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!
--- 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
--- 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)
--- 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
--- 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!
--- a/app/workers/scheduler/subscriptions_cleanup_scheduler.rb
+++ b/app/workers/scheduler/subscriptions_cleanup_scheduler.rb
@ -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
--- a/app/workers/scheduler/subscriptions_scheduler.rb
+++ b/app/workers/scheduler/subscriptions_scheduler.rb
@ -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
--- 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
--- 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|
--- 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)
--- 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|
--- 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]
--- 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