Skip to content

Commit

Permalink
Change auto-deletion throttling constants to better scale with server…
Browse files Browse the repository at this point in the history
… size (#23320)
  • Loading branch information
ClearlyClaire authored Feb 23, 2023
1 parent f4f91bc commit 20b80c6
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 26 deletions.
24 changes: 8 additions & 16 deletions app/workers/scheduler/accounts_statuses_cleanup_scheduler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,36 +7,30 @@ class Scheduler::AccountsStatusesCleanupScheduler
# This limit is mostly to be nice to the fediverse at large and not
# generate too much traffic.
# This also helps limiting the running time of the scheduler itself.
MAX_BUDGET = 50
MAX_BUDGET = 150

# This is an attempt to spread the load across instances, as various
# accounts are likely to have various followers.
PER_ACCOUNT_BUDGET = 5

# This is an attempt to limit the workload generated by status removal
# jobs to something the particular instance can handle.
PER_THREAD_BUDGET = 5
PER_THREAD_BUDGET = 6

# Those avoid loading an instance that is already under load
MAX_DEFAULT_SIZE = 2
MAX_DEFAULT_SIZE = 200
MAX_DEFAULT_LATENCY = 5
MAX_PUSH_SIZE = 5
MAX_PUSH_SIZE = 500
MAX_PUSH_LATENCY = 10

# 'pull' queue has lower priority jobs, and it's unlikely that pushing
# deletes would cause much issues with this queue if it didn't cause issues
# with default and push. Yet, do not enqueue deletes if the instance is
# lagging behind too much.
MAX_PULL_SIZE = 500
MAX_PULL_LATENCY = 300

# This is less of an issue in general, but deleting old statuses is likely
# to cause delivery errors, and thus increase the number of jobs to be retried.
# This doesn't directly translate to load, but connection errors and a high
# number of dead instances may lead to this spiraling out of control if
# unchecked.
MAX_RETRY_SIZE = 50_000
MAX_PULL_SIZE = 10_000
MAX_PULL_LATENCY = 5.minutes.to_i

sidekiq_options retry: 0, lock: :until_executed
sidekiq_options retry: 0, lock: :until_executed, lock_ttl: 1.day.to_i

def perform
return if under_load?
Expand Down Expand Up @@ -73,8 +67,6 @@ def compute_budget
end

def under_load?
return true if Sidekiq::Stats.new.retry_size > MAX_RETRY_SIZE

queue_under_load?('default', MAX_DEFAULT_SIZE, MAX_DEFAULT_LATENCY) || queue_under_load?('push', MAX_PUSH_SIZE, MAX_PUSH_LATENCY) || queue_under_load?('pull', MAX_PULL_SIZE, MAX_PULL_LATENCY)
end

Expand Down
10 changes: 0 additions & 10 deletions spec/workers/scheduler/accounts_statuses_cleanup_scheduler_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
},
]
end
let(:retry_size) { 0 }

before do
queue_stub = double
Expand All @@ -35,7 +34,6 @@
allow(Sidekiq::ProcessSet).to receive(:new).and_return(process_set_stub)

sidekiq_stats_stub = double
allow(sidekiq_stats_stub).to receive(:retry_size).and_return(retry_size)
allow(Sidekiq::Stats).to receive(:new).and_return(sidekiq_stats_stub)

# Create a bunch of old statuses
Expand Down Expand Up @@ -72,14 +70,6 @@
expect(subject.under_load?).to be true
end
end

context 'when there is a huge amount of jobs to retry' do
let(:retry_size) { 1_000_000 }

it 'returns true' do
expect(subject.under_load?).to be true
end
end
end

describe '#get_budget' do
Expand Down

0 comments on commit 20b80c6

Please sign in to comment.