Skip to content

Commit

Permalink
Fix some RedisLocks auto-releasing too fast (mastodon#16276)
Browse files Browse the repository at this point in the history
* Fix Delete and Create-related locks expiring too fast

Fixes mastodon#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 mastodon#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
  • Loading branch information
ClearlyClaire authored and chrisguida committed Feb 26, 2022
1 parent faa0c55 commit bb511ab
Show file tree
Hide file tree
Showing 6 changed files with 7 additions and 7 deletions.
4 changes: 2 additions & 2 deletions app/lib/activitypub/activity.rb
Original file line number Diff line number Diff line change
Expand Up @@ -216,8 +216,8 @@ def lock_or_return(key, expire_after = 2.hours.seconds)
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
Expand Down
2 changes: 1 addition & 1 deletion app/services/activitypub/process_account_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ def protocol_changed?
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
Expand Down
2 changes: 1 addition & 1 deletion app/services/fetch_link_card_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,6 @@ def meta_property(page, property)
end

def lock_options
{ redis: Redis.current, key: "fetch:#{@url}" }
{ redis: Redis.current, key: "fetch:#{@url}", autorelease: 15.minutes.seconds }
end
end
2 changes: 1 addition & 1 deletion app/services/remove_status_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,6 @@ def remove_media
end

def lock_options
{ redis: Redis.current, key: "distribute:#{@status.id}" }
{ redis: Redis.current, key: "distribute:#{@status.id}", autorelease: 5.minutes.seconds }
end
end
2 changes: 1 addition & 1 deletion app/services/resolve_account_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,6 @@ def queue_deletion!
end

def lock_options
{ redis: Redis.current, key: "resolve:#{@username}@#{@domain}" }
{ redis: Redis.current, key: "resolve:#{@username}@#{@domain}", autorelease: 15.minutes.seconds }
end
end
2 changes: 1 addition & 1 deletion app/workers/distribution_worker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit bb511ab

Please sign in to comment.