]> cat aescling's git repositories - mastodon.git/commitdiff
Fix background jobs not using locks like they are supposed to (#13361)
authorEugen Rochko <eugen@zeonfederated.com>
Tue, 31 Mar 2020 19:59:03 +0000 (21:59 +0200)
committerGitHub <noreply@github.com>
Tue, 31 Mar 2020 19:59:03 +0000 (21:59 +0200)
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

34 files changed:
app/workers/activitypub/distribute_poll_update_worker.rb
app/workers/activitypub/synchronize_featured_collection_worker.rb
app/workers/after_remote_follow_request_worker.rb [deleted file]
app/workers/after_remote_follow_worker.rb [deleted file]
app/workers/notification_worker.rb [deleted file]
app/workers/poll_expiration_notify_worker.rb
app/workers/processing_worker.rb [deleted file]
app/workers/publish_scheduled_status_worker.rb
app/workers/pubsubhubbub/confirmation_worker.rb [deleted file]
app/workers/pubsubhubbub/delivery_worker.rb [deleted file]
app/workers/pubsubhubbub/distribution_worker.rb [deleted file]
app/workers/pubsubhubbub/raw_distribution_worker.rb [deleted file]
app/workers/pubsubhubbub/subscribe_worker.rb [deleted file]
app/workers/pubsubhubbub/unsubscribe_worker.rb [deleted file]
app/workers/regeneration_worker.rb
app/workers/remote_profile_update_worker.rb [deleted file]
app/workers/resolve_account_worker.rb
app/workers/salmon_worker.rb [deleted file]
app/workers/scheduler/backup_cleanup_scheduler.rb
app/workers/scheduler/doorkeeper_cleanup_scheduler.rb
app/workers/scheduler/email_scheduler.rb
app/workers/scheduler/feed_cleanup_scheduler.rb
app/workers/scheduler/ip_cleanup_scheduler.rb
app/workers/scheduler/media_cleanup_scheduler.rb
app/workers/scheduler/pghero_scheduler.rb
app/workers/scheduler/scheduled_statuses_scheduler.rb
app/workers/scheduler/subscriptions_cleanup_scheduler.rb [deleted file]
app/workers/scheduler/subscriptions_scheduler.rb [deleted file]
app/workers/scheduler/trending_tags_scheduler.rb
app/workers/scheduler/user_cleanup_scheduler.rb
app/workers/verify_account_links_worker.rb
config/initializers/sidekiq.rb
config/routes.rb
spec/services/import_service_spec.rb

index 1e87fa4bf2a784966773e3aeb654ad84cb45fed3..25dee4ee2597431fad6e95938e30579ace3e8c09 100644 (file)
@@ -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)
index 7b16d3426a29b0c8989be2a0b2f1b11757d5c7fc..7a0898e89de431fd221b9fd164222c55e0c33a44 100644 (file)
@@ -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 (file)
index ce9c658..0000000
+++ /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 (file)
index d9719f2..0000000
+++ /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 (file)
index 1c0f001..0000000
+++ /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
index e08f0c2496fa7d13ec538195df5074916ae1359d..64b4cbd7e46b01db2f4aea7847b7e6b0c2754cc4 100644 (file)
@@ -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 (file)
index cf3bd83..0000000
+++ /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
index 850610c4e3cdb932dc16dd150a9a58d22039d44f..ce42f7be7c6e21fdf05514adf95ef9f1f323ea95 100644 (file)
@@ -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 (file)
index 783a8c9..0000000
+++ /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 (file)
index 1260060..0000000
+++ /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 (file)
index 75bac5d..0000000
+++ /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 (file)
index ece9c80..0000000
+++ /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 (file)
index b861b5e..0000000
+++ /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 (file)
index 0c1c263..0000000
+++ /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
index 5c6a040bd5884611c3590d83e2dbe3d1cb443855..5c13c894fea8d09b6851a03bc7946ea4939e5de5 100644 (file)
@@ -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 (file)
index 01e8daf..0000000
+++ /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
index cd7c4d7dda59051b517e3fb7380a438d84079879..2b5be6d1b217da5aed9979ce7d8cf1fec1700d27 100644 (file)
@@ -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 (file)
index 10200b0..0000000
+++ /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
index d436606999f1f5c5eac4bebceecdd5b510622871..d69ca2556aef9ceeaf77bdb31b73e593a35179b6 100644 (file)
@@ -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!)
index e5e5f6bc42fcb035a3453442180d3529be3875fa..94788a85bb839b02f88362c5bf170c30f790078d 100644 (file)
@@ -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
index 1eeeee412d7b12249f624090fe4b67e02bbb8b8c..9a73555248287a0d8c9946c9bbb4bd11fcb172ea 100644 (file)
@@ -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
index bf5e20757007d893913d077021256eb03ceba062..458fe6193e53d69877baeb5968bb49c3624a702f 100644 (file)
@@ -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!
index 4f44078d8e903971dab4be0a4897822dd5be0ef6..6d38b52a2988de2965a53cfbe5dcf819a69b5049 100644 (file)
@@ -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
index fb01aa70c9957237ebdd5fdfb02039e04f9fbc81..671ebf6e02d8ccd3dfbd86eb558cd3ee9b5c8ab0 100644 (file)
@@ -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)
index 4453bf2cd0018b504d5dac83f0d0baf523172530..cf55700486465869de3fa6515a98a1793e544449 100644 (file)
@@ -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
index 9cfe949dec52c85031e28368ac8e4bb4dba39ac9..25df3c07deec366e6bda941e3d0d27dc6f722c8c 100644 (file)
@@ -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 (file)
index 75fe681..0000000
+++ /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 (file)
index 6903cad..0000000
+++ /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
index 77f0d574757c920dab85212602053830c2434153..e9891424e0a90dbb35e64522cf8c3501c26e7eb3 100644 (file)
@@ -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
index 881b911be75c2b3967426432546e75852a4148ea..6113edde17b301a15f2ea67f846bf7b498b0ff12 100644 (file)
@@ -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|
index 901498583e86dc19ed2f4881453051a183bda11f..8114d59bea3d697523c278e0fca674bcc91265bf 100644 (file)
@@ -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)
index 288453a046c1d444fab865c6fb0e4c072eba972f..f2733562fb143fd998f8d19101a9339b65b12063 100644 (file)
@@ -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|
index 89d446d10f8d950e2bed51dd0a05907dce15cd8f..49ab851e2cf854fcbe0a6e487427202de4331c56 100644 (file)
@@ -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]
index 5355133f4000826dcca7529f38015d7c0dc5abf1..7618e9076184b90fb9ecb5f1593ff6adfbc1ecb0 100644 (file)
@@ -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