From 08ec45443df285726df23311f8b4b074ff430240 Mon Sep 17 00:00:00 2001 From: mfo Date: Tue, 24 Sep 2024 20:58:26 +0200 Subject: [PATCH 1/3] BREAKING(sidekiq.queues): rationalize queues. now have critical, default, low Dear instances, make sure to update your sidekiq processes. Indead, we are adopting a new strategy for daemons that process our background jobs. Now their is only 3 queues on sidekiq (still, we keep archive/export on delayed job for now). The big idea : allow any worker to deal with any queue (capacity mgmt) : - critical: must be procesed now - default: will be processed asap - low: do it after everything else FYI: we had 50% of our server capacity on reserved queues. Leading to bottle neck processing (reserved queues/worker not processing other queues). We HAD TO fix it. Sorry for the inconvenience TBH, this is an idea of Colin Darie . I'm just pushing it forward. Co-Authored-By: Colin Darie --- app/jobs/batch_operation_enqueue_all_job.rb | 2 +- app/jobs/batch_operation_process_one_job.rb | 2 +- app/jobs/cron/cron_job.rb | 2 +- app/jobs/dossier_index_search_terms_job.rb | 2 +- .../dossier_operation_log_move_to_cold_storage_batch_job.rb | 2 +- app/jobs/dossier_rebase_job.rb | 2 +- app/jobs/helpscout_create_conversation_job.rb | 2 -- app/jobs/procedure_sva_svr_process_dossier_job.rb | 2 +- app/jobs/web_hook_job.rb | 2 +- 9 files changed, 8 insertions(+), 10 deletions(-) diff --git a/app/jobs/batch_operation_enqueue_all_job.rb b/app/jobs/batch_operation_enqueue_all_job.rb index 3b54879f8..f99d7a79d 100644 --- a/app/jobs/batch_operation_enqueue_all_job.rb +++ b/app/jobs/batch_operation_enqueue_all_job.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class BatchOperationEnqueueAllJob < ApplicationJob - queue_as :mailers # hotfix + queue_as :critical def perform(batch_operation) batch_operation.enqueue_all diff --git a/app/jobs/batch_operation_process_one_job.rb b/app/jobs/batch_operation_process_one_job.rb index 04c18353a..102b27a76 100644 --- a/app/jobs/batch_operation_process_one_job.rb +++ b/app/jobs/batch_operation_process_one_job.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class BatchOperationProcessOneJob < ApplicationJob - queue_as :mailers # hotfix + queue_as :critical retry_on StandardError, attempts: 1 # default 5, for now no retryable behavior def perform(batch_operation, dossier) diff --git a/app/jobs/cron/cron_job.rb b/app/jobs/cron/cron_job.rb index 2124474a4..7ca5e04e6 100644 --- a/app/jobs/cron/cron_job.rb +++ b/app/jobs/cron/cron_job.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class Cron::CronJob < ApplicationJob - queue_as :cron + queue_as :default class_attribute :schedule_expression class << self diff --git a/app/jobs/dossier_index_search_terms_job.rb b/app/jobs/dossier_index_search_terms_job.rb index 884506eae..dc0079a5a 100644 --- a/app/jobs/dossier_index_search_terms_job.rb +++ b/app/jobs/dossier_index_search_terms_job.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class DossierIndexSearchTermsJob < ApplicationJob - queue_as :low_priority + queue_as :low discard_on ActiveRecord::RecordNotFound diff --git a/app/jobs/dossier_operation_log_move_to_cold_storage_batch_job.rb b/app/jobs/dossier_operation_log_move_to_cold_storage_batch_job.rb index f9c48faa2..d2fb60224 100644 --- a/app/jobs/dossier_operation_log_move_to_cold_storage_batch_job.rb +++ b/app/jobs/dossier_operation_log_move_to_cold_storage_batch_job.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class DossierOperationLogMoveToColdStorageBatchJob < ApplicationJob - queue_as :low_priority + queue_as :low def perform(ids) DossierOperationLog.where(id: ids) diff --git a/app/jobs/dossier_rebase_job.rb b/app/jobs/dossier_rebase_job.rb index a6e8a3286..2ca0eb8c9 100644 --- a/app/jobs/dossier_rebase_job.rb +++ b/app/jobs/dossier_rebase_job.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class DossierRebaseJob < ApplicationJob - queue_as :low_priority # they are massively enqueued, so don't interfere with others especially antivirus + queue_as :low # they are massively enqueued, so don't interfere with others especially antivirus # If by the time the job runs the Dossier has been deleted, ignore the rebase discard_on ActiveRecord::RecordNotFound diff --git a/app/jobs/helpscout_create_conversation_job.rb b/app/jobs/helpscout_create_conversation_job.rb index 675bfa23d..61b55bcaa 100644 --- a/app/jobs/helpscout_create_conversation_job.rb +++ b/app/jobs/helpscout_create_conversation_job.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true class HelpscoutCreateConversationJob < ApplicationJob - queue_as :default - class FileNotScannedYetError < StandardError end diff --git a/app/jobs/procedure_sva_svr_process_dossier_job.rb b/app/jobs/procedure_sva_svr_process_dossier_job.rb index 619ebc568..48a8d7634 100644 --- a/app/jobs/procedure_sva_svr_process_dossier_job.rb +++ b/app/jobs/procedure_sva_svr_process_dossier_job.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class ProcedureSVASVRProcessDossierJob < ApplicationJob - queue_as :sva + queue_as :critical def perform(dossier) dossier.process_sva_svr! diff --git a/app/jobs/web_hook_job.rb b/app/jobs/web_hook_job.rb index 32e8ab3bb..40b37c837 100644 --- a/app/jobs/web_hook_job.rb +++ b/app/jobs/web_hook_job.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class WebHookJob < ApplicationJob - queue_as :webhooks_v1 + queue_as :default TIMEOUT = 10 From afe98704e904d9a4144ae468d06a3d12618471d3 Mon Sep 17 00:00:00 2001 From: mfo Date: Tue, 24 Sep 2024 21:24:22 +0200 Subject: [PATCH 2/3] feat(async): re-order priority jobs --- app/jobs/champ_fetch_external_data_job.rb | 1 + app/jobs/destroy_record_later_job.rb | 1 + app/jobs/dolist_report_job.rb | 1 + app/jobs/etablissement_update_job.rb | 1 + app/jobs/helpscout_create_conversation_job.rb | 1 + app/jobs/priorized_mail_delivery_job.rb | 2 +- app/jobs/process_stalled_declarative_dossier_job.rb | 1 + app/jobs/reset_expiring_dossiers_job.rb | 1 + app/jobs/send_closing_notification_job.rb | 2 ++ 9 files changed, 10 insertions(+), 1 deletion(-) diff --git a/app/jobs/champ_fetch_external_data_job.rb b/app/jobs/champ_fetch_external_data_job.rb index 99cd96a6e..134d579e0 100644 --- a/app/jobs/champ_fetch_external_data_job.rb +++ b/app/jobs/champ_fetch_external_data_job.rb @@ -2,6 +2,7 @@ class ChampFetchExternalDataJob < ApplicationJob discard_on ActiveJob::DeserializationError + queue_as :critical # ui feedback, asap include Dry::Monads[:result] diff --git a/app/jobs/destroy_record_later_job.rb b/app/jobs/destroy_record_later_job.rb index 8b39e11ac..426c967a7 100644 --- a/app/jobs/destroy_record_later_job.rb +++ b/app/jobs/destroy_record_later_job.rb @@ -2,6 +2,7 @@ class DestroyRecordLaterJob < ApplicationJob discard_on ActiveRecord::RecordNotFound + queue_as :low # destroy later, will be done when possible def perform(record) record.destroy diff --git a/app/jobs/dolist_report_job.rb b/app/jobs/dolist_report_job.rb index a27348270..60e1be5bf 100644 --- a/app/jobs/dolist_report_job.rb +++ b/app/jobs/dolist_report_job.rb @@ -3,6 +3,7 @@ class DolistReportJob < ApplicationJob # Consolidate random recent emails dispatched to Dolist with their statuses # and send a report by email. + queue_as :low # reporting will be done asap def perform(report_to, sample_size = 1000) events = EmailEvent.dolist.dispatched.where(processed_at: 2.weeks.ago..).order("RANDOM()").limit(sample_size) diff --git a/app/jobs/etablissement_update_job.rb b/app/jobs/etablissement_update_job.rb index cff98d98e..f5eaacba0 100644 --- a/app/jobs/etablissement_update_job.rb +++ b/app/jobs/etablissement_update_job.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true class EtablissementUpdateJob < ApplicationJob + queue_as :critical # reporting will be done asap, but no occurence found. maube dead? def perform(dossier, siret) begin etablissement_attributes = APIEntrepriseService.get_etablissement_params_for_siret(siret, dossier.procedure.id) diff --git a/app/jobs/helpscout_create_conversation_job.rb b/app/jobs/helpscout_create_conversation_job.rb index 61b55bcaa..f18781d53 100644 --- a/app/jobs/helpscout_create_conversation_job.rb +++ b/app/jobs/helpscout_create_conversation_job.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true class HelpscoutCreateConversationJob < ApplicationJob + queue_as :critical # user feedback is critical class FileNotScannedYetError < StandardError end diff --git a/app/jobs/priorized_mail_delivery_job.rb b/app/jobs/priorized_mail_delivery_job.rb index ac308b180..db35c9bd5 100644 --- a/app/jobs/priorized_mail_delivery_job.rb +++ b/app/jobs/priorized_mail_delivery_job.rb @@ -12,7 +12,7 @@ class PriorizedMailDeliveryJob < ActionMailer::MailDeliveryJob end end - def custom_queue + def custom_queue # should be low ENV.fetch('BULK_EMAIL_QUEUE') { Rails.application.config.action_mailer.deliver_later_queue_name.to_s } end end diff --git a/app/jobs/process_stalled_declarative_dossier_job.rb b/app/jobs/process_stalled_declarative_dossier_job.rb index 6449c73af..aaa49fb70 100644 --- a/app/jobs/process_stalled_declarative_dossier_job.rb +++ b/app/jobs/process_stalled_declarative_dossier_job.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true class ProcessStalledDeclarativeDossierJob < ApplicationJob + queue_as :low def perform(dossier) return if dossier.declarative_triggered_at.present? diff --git a/app/jobs/reset_expiring_dossiers_job.rb b/app/jobs/reset_expiring_dossiers_job.rb index 5cf818ee1..95a87f571 100644 --- a/app/jobs/reset_expiring_dossiers_job.rb +++ b/app/jobs/reset_expiring_dossiers_job.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true class ResetExpiringDossiersJob < ApplicationJob + queue_as :low def perform(procedure) procedure .dossiers diff --git a/app/jobs/send_closing_notification_job.rb b/app/jobs/send_closing_notification_job.rb index f5d9f7167..f4850b870 100644 --- a/app/jobs/send_closing_notification_job.rb +++ b/app/jobs/send_closing_notification_job.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class SendClosingNotificationJob < ApplicationJob + queue_as :low # no rush on this one + def perform(user_ids, content, procedure) User.where(id: user_ids).find_each do |user| Expired::MailRateLimiter.new().send_with_delay(UserMailer.notify_after_closing(user, content, @procedure)) From b8b727f06ba0e67a8dfab06355dfa5b9d6724954 Mon Sep 17 00:00:00 2001 From: mfo Date: Tue, 24 Sep 2024 21:25:10 +0200 Subject: [PATCH 3/3] feat(default.queues): mailers that are not critical are low, otherwise critical. analysis is default, purge is low --- app/jobs/priorized_mail_delivery_job.rb | 4 ++-- config/application.rb | 6 +++--- spec/mailers/administrateur_mailer_spec.rb | 6 ++---- spec/mailers/instructeur_mailer_spec.rb | 7 ++----- spec/mailers/user_mailer_spec.rb | 9 +++------ 5 files changed, 12 insertions(+), 20 deletions(-) diff --git a/app/jobs/priorized_mail_delivery_job.rb b/app/jobs/priorized_mail_delivery_job.rb index db35c9bd5..2404e3912 100644 --- a/app/jobs/priorized_mail_delivery_job.rb +++ b/app/jobs/priorized_mail_delivery_job.rb @@ -12,7 +12,7 @@ class PriorizedMailDeliveryJob < ActionMailer::MailDeliveryJob end end - def custom_queue # should be low - ENV.fetch('BULK_EMAIL_QUEUE') { Rails.application.config.action_mailer.deliver_later_queue_name.to_s } + def custom_queue + 'low' end end diff --git a/config/application.rb b/config/application.rb index b57986388..dc185a729 100644 --- a/config/application.rb +++ b/config/application.rb @@ -53,14 +53,14 @@ module TPS config.action_dispatch.ip_spoofing_check = false # Set the queue name for the mail delivery jobs to 'mailers' - config.action_mailer.deliver_later_queue_name = 'mailers' + config.action_mailer.deliver_later_queue_name = 'critical' # otherwise, :low # Allow the error messages format to be customized config.active_model.i18n_customize_full_message = true # Set the queue name for the analysis jobs to 'active_storage_analysis' - config.active_storage.queues.analysis = :active_storage_analysis - config.active_storage.queues.purge = :purge + config.active_storage.queues.analysis = :default + config.active_storage.queues.purge = :low config.active_support.cache_format_version = 7.0 diff --git a/spec/mailers/administrateur_mailer_spec.rb b/spec/mailers/administrateur_mailer_spec.rb index c038975f2..180877eb1 100644 --- a/spec/mailers/administrateur_mailer_spec.rb +++ b/spec/mailers/administrateur_mailer_spec.rb @@ -11,8 +11,7 @@ RSpec.describe AdministrateurMailer, type: :mailer do it { expect(subject.subject).to include("La suppression automatique des dossiers a été activée sur la démarche") } context 'when perform_later is called' do - let(:custom_queue) { 'low_priority' } - before { ENV['BULK_EMAIL_QUEUE'] = custom_queue } + let(:custom_queue) { 'low' } it 'enqueues email is custom queue for low priority delivery' do expect { subject.deliver_later }.to have_enqueued_job.on_queue(custom_queue) end @@ -52,8 +51,7 @@ end it { expect(subject.body).to include("un de vos services n'a pas son siret renseigné") } context 'when perform_later is called' do - let(:custom_queue) { 'low_priority' } - before { ENV['BULK_EMAIL_QUEUE'] = custom_queue } + let(:custom_queue) { 'low' } it 'enqueues email is custom queue for low priority delivery' do expect { subject.deliver_later }.to have_enqueued_job.on_queue(custom_queue) end diff --git a/spec/mailers/instructeur_mailer_spec.rb b/spec/mailers/instructeur_mailer_spec.rb index db53e6282..28667228e 100644 --- a/spec/mailers/instructeur_mailer_spec.rb +++ b/spec/mailers/instructeur_mailer_spec.rb @@ -10,8 +10,7 @@ RSpec.describe InstructeurMailer, type: :mailer do it { expect(subject.body).to include('Bonjour') } context 'when perform_later is called' do - let(:custom_queue) { 'low_priority' } - before { ENV['BULK_EMAIL_QUEUE'] = custom_queue } + let(:custom_queue) { 'low' } it 'enqueues email is custom queue for low priority delivery' do expect { subject.deliver_later }.to have_enqueued_job(PriorizedMailDeliveryJob).on_queue(custom_queue) @@ -81,9 +80,7 @@ RSpec.describe InstructeurMailer, type: :mailer do end context 'when perform_later is called' do - let(:custom_queue) { 'low_priority' } - before { ENV['BULK_EMAIL_QUEUE'] = custom_queue } - + let(:custom_queue) { 'low' } it 'enqueues email is custom queue for low priority delivery' do expect { subject.deliver_later }.to have_enqueued_job.on_queue(custom_queue) end diff --git a/spec/mailers/user_mailer_spec.rb b/spec/mailers/user_mailer_spec.rb index c187c27d7..aa2b1812a 100644 --- a/spec/mailers/user_mailer_spec.rb +++ b/spec/mailers/user_mailer_spec.rb @@ -151,8 +151,7 @@ RSpec.describe UserMailer, type: :mailer do context 'when perform_later is called' do let(:role) { administrateurs(:default_admin) } - let(:custom_queue) { 'low_priority' } - before { ENV['BULK_EMAIL_QUEUE'] = custom_queue } + let(:custom_queue) { 'low' } it 'enqueues email is custom queue for low priority delivery' do expect { subject.deliver_later }.to have_enqueued_job.on_queue(custom_queue) end @@ -168,8 +167,7 @@ RSpec.describe UserMailer, type: :mailer do end context 'when perform_later is called' do - let(:custom_queue) { 'low_priority' } - before { ENV['BULK_EMAIL_QUEUE'] = custom_queue } + let(:custom_queue) { 'low' } it 'enqueues email is custom queue for low priority delivery' do expect { subject.deliver_later }.to have_enqueued_job.on_queue(custom_queue) end @@ -188,8 +186,7 @@ RSpec.describe UserMailer, type: :mailer do end context 'when perform_later is called' do - let(:custom_queue) { 'low_priority' } - before { ENV['BULK_EMAIL_QUEUE'] = custom_queue } + let(:custom_queue) { 'low' } it 'enqueues email is custom queue for low priority delivery' do expect { subject.deliver_later }.to have_enqueued_job.on_queue(custom_queue) end