PERF: Introduce absolute limit of digests per 30 minutes (#10845)
To avoid blocking the sidekiq queue a limit of 10,000 digests per 30 minutes is introduced. This acts as a safety measure that makes sure we don't keep pouring oil on a fire. On multisites it is recommended to set the number way lower so sites do not dominate the backlog. A reasonable default for multisites may be 100-500. This can be controlled with the environment var DISCOURSE_MAX_DIGESTS_ENQUEUED_PER_30_MINS_PER_SITE
This commit is contained in:
parent
6e2be3e60b
commit
120fa8ad2f
|
@ -7,7 +7,13 @@ module Jobs
|
||||||
|
|
||||||
def execute(args)
|
def execute(args)
|
||||||
return if SiteSetting.disable_digest_emails? || SiteSetting.private_email?
|
return if SiteSetting.disable_digest_emails? || SiteSetting.private_email?
|
||||||
target_user_ids.each do |user_id|
|
users = target_user_ids
|
||||||
|
|
||||||
|
if users.length > GlobalSetting.max_digests_enqueued_per_30_mins_per_site
|
||||||
|
users = users.shuffle[0...GlobalSetting.max_digests_enqueued_per_30_mins_per_site]
|
||||||
|
end
|
||||||
|
|
||||||
|
users.each do |user_id|
|
||||||
::Jobs.enqueue(:user_email, type: :digest, user_id: user_id)
|
::Jobs.enqueue(:user_email, type: :digest, user_id: user_id)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -309,3 +309,8 @@ allowed_theme_repos =
|
||||||
# We want this off by default so the process is not started when it does not
|
# We want this off by default so the process is not started when it does not
|
||||||
# need to be (e.g. development, test, certain hosting tiers)
|
# need to be (e.g. development, test, certain hosting tiers)
|
||||||
enable_email_sync_demon = false
|
enable_email_sync_demon = false
|
||||||
|
|
||||||
|
# we never want to queue more than 10000 digests per 30 minute block
|
||||||
|
# this can easily lead to blocking sidekiq
|
||||||
|
# on multisites we recommend a far lower number
|
||||||
|
max_digests_enqueued_per_30_mins_per_site = 10000
|
||||||
|
|
|
@ -121,6 +121,20 @@ describe Jobs::EnqueueDigestEmails do
|
||||||
|
|
||||||
let(:user) { Fabricate(:user) }
|
let(:user) { Fabricate(:user) }
|
||||||
|
|
||||||
|
it "limits jobs enqueued per max_digests_enqueued_per_30_mins_per_site" do
|
||||||
|
Fabricate(:user, last_seen_at: 2.months.ago, last_emailed_at: 2.months.ago)
|
||||||
|
Fabricate(:user, last_seen_at: 2.months.ago, last_emailed_at: 2.months.ago)
|
||||||
|
|
||||||
|
global_setting :max_digests_enqueued_per_30_mins_per_site, 1
|
||||||
|
|
||||||
|
# I don't love fakes, but no point sending this fake email
|
||||||
|
Sidekiq::Testing.fake! do
|
||||||
|
expect do
|
||||||
|
Jobs::EnqueueDigestEmails.new.execute(nil)
|
||||||
|
end.to change(Jobs::UserEmail.jobs, :size).by (1)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
context "digest emails are enabled" do
|
context "digest emails are enabled" do
|
||||||
before do
|
before do
|
||||||
Jobs::EnqueueDigestEmails.any_instance.expects(:target_user_ids).returns([user.id])
|
Jobs::EnqueueDigestEmails.any_instance.expects(:target_user_ids).returns([user.id])
|
||||||
|
|
Loading…
Reference in New Issue