From 9a19227f177fb328378bd63d45c3683a00d4eda9 Mon Sep 17 00:00:00 2001 From: Claire Date: Wed, 19 May 2021 23:52:08 +0200 Subject: [PATCH] Fix some RedisLocks auto-releasing too fast (#16276) * Fix Delete and Create-related locks expiring too fast Fixes #16238 By default, RedisLock expires after 10 seconds, which may not be enough to process statuses, especially when those have attached media files. This commit extends those 10 seconds to 15 minutes, which should be plenty enough to handle any status, while being short enough to not waste many sidekiq job retries in the exceedingly rare case in which a sidekiq process would crash when processing a `Create` or `Delete`. * Fix other RedisLock autorelease durations Fixes #15645 - things that only perform a few simple database queries (e.g. finding and saving a record) have been left unchanged, so they'll still use the default 10s duration - things that perform significantly more complex database queries have been changed to a 5 minutes timeout - things that perform multiple HTTP queries have been changed to a 15 minutes timeout --- app/lib/activitypub/activity.rb | 4 ++-- app/services/activitypub/process_account_service.rb | 2 +- app/services/fetch_link_card_service.rb | 2 +- app/services/remove_status_service.rb | 2 +- app/services/resolve_account_service.rb | 2 +- app/workers/distribution_worker.rb | 2 +- 6 files changed, 7 insertions(+), 7 deletions(-) diff --git a/app/lib/activitypub/activity.rb b/app/lib/activitypub/activity.rb index 3baee4ca46..d2ec122a4d 100644 --- a/app/lib/activitypub/activity.rb +++ b/app/lib/activitypub/activity.rb @@ -216,8 +216,8 @@ class ActivityPub::Activity redis.del(key) end - def lock_or_fail(key) - RedisLock.acquire({ redis: Redis.current, key: key }) do |lock| + def lock_or_fail(key, expire_after = 15.minutes.seconds) + RedisLock.acquire({ redis: Redis.current, key: key, autorelease: expire_after }) do |lock| if lock.acquired? yield else diff --git a/app/services/activitypub/process_account_service.rb b/app/services/activitypub/process_account_service.rb index 7e268f4d49..4ab6912e5d 100644 --- a/app/services/activitypub/process_account_service.rb +++ b/app/services/activitypub/process_account_service.rb @@ -290,7 +290,7 @@ class ActivityPub::ProcessAccountService < BaseService end def lock_options - { redis: Redis.current, key: "process_account:#{@uri}" } + { redis: Redis.current, key: "process_account:#{@uri}", autorelease: 15.minutes.seconds } end def process_tags diff --git a/app/services/fetch_link_card_service.rb b/app/services/fetch_link_card_service.rb index fa1636e411..5732ce8ace 100644 --- a/app/services/fetch_link_card_service.rb +++ b/app/services/fetch_link_card_service.rb @@ -175,6 +175,6 @@ class FetchLinkCardService < BaseService end def lock_options - { redis: Redis.current, key: "fetch:#{@url}" } + { redis: Redis.current, key: "fetch:#{@url}", autorelease: 15.minutes.seconds } end end diff --git a/app/services/remove_status_service.rb b/app/services/remove_status_service.rb index 6e4d6e72ab..b680c8e961 100644 --- a/app/services/remove_status_service.rb +++ b/app/services/remove_status_service.rb @@ -141,6 +141,6 @@ class RemoveStatusService < BaseService end def lock_options - { redis: Redis.current, key: "distribute:#{@status.id}" } + { redis: Redis.current, key: "distribute:#{@status.id}", autorelease: 5.minutes.seconds } end end diff --git a/app/services/resolve_account_service.rb b/app/services/resolve_account_service.rb index 493995447a..5400612bfd 100644 --- a/app/services/resolve_account_service.rb +++ b/app/services/resolve_account_service.rb @@ -146,6 +146,6 @@ class ResolveAccountService < BaseService end def lock_options - { redis: Redis.current, key: "resolve:#{@username}@#{@domain}" } + { redis: Redis.current, key: "resolve:#{@username}@#{@domain}", autorelease: 15.minutes.seconds } end end diff --git a/app/workers/distribution_worker.rb b/app/workers/distribution_worker.rb index 4e20ef31bf..e85cd7e95a 100644 --- a/app/workers/distribution_worker.rb +++ b/app/workers/distribution_worker.rb @@ -4,7 +4,7 @@ class DistributionWorker include Sidekiq::Worker def perform(status_id) - RedisLock.acquire(redis: Redis.current, key: "distribute:#{status_id}") do |lock| + RedisLock.acquire(redis: Redis.current, key: "distribute:#{status_id}", autorelease: 5.minutes.seconds) do |lock| if lock.acquired? FanOutOnWriteService.new.call(Status.find(status_id)) else