From ab8ac78ccb46446f6bed23c8fff0623bf9c1141e Mon Sep 17 00:00:00 2001 From: mfo Date: Thu, 3 Oct 2024 11:13:33 +0200 Subject: [PATCH 1/3] tech(sidekiq): use sidekiq by default, clean transition code --- Procfile.dev | 2 +- app/jobs/sidekiq_again_job.rb | 14 -------- config/env.example.optional | 3 ++ config/environments/production.rb | 2 +- config/initializers/transition_to_sidekiq.rb | 35 -------------------- 5 files changed, 5 insertions(+), 51 deletions(-) delete mode 100644 app/jobs/sidekiq_again_job.rb delete mode 100644 config/initializers/transition_to_sidekiq.rb diff --git a/Procfile.dev b/Procfile.dev index 953087cec..1459bc6df 100644 --- a/Procfile.dev +++ b/Procfile.dev @@ -1,3 +1,3 @@ -web: RAILS_QUEUE_ADAPTER=delayed_job bin/rails server -p 3000 +web: bin/rails server -p 3000 jobs: bin/rake jobs:work vite: bin/vite dev diff --git a/app/jobs/sidekiq_again_job.rb b/app/jobs/sidekiq_again_job.rb deleted file mode 100644 index dcb7b4059..000000000 --- a/app/jobs/sidekiq_again_job.rb +++ /dev/null @@ -1,14 +0,0 @@ -# frozen_string_literal: true - -class SidekiqAgainJob < ApplicationJob - self.queue_adapter = :sidekiq - queue_as :default - - def perform(user, with_exception: false) - if with_exception - raise 'Nop' - end - Sentry.capture_message('this is a message from sidekiq') - UserMailer.new_account_warning(user).deliver_now - end -end diff --git a/config/env.example.optional b/config/env.example.optional index 74451810a..050e5d49b 100644 --- a/config/env.example.optional +++ b/config/env.example.optional @@ -282,3 +282,6 @@ VITE_LIGHTGALLERY_LICENSE_KEY = "" # This email will be visible to users whom dossier had been fixed by our maintenance_tasks # By default we use CONTACT_EMAIL, but you can customize it MAINTENANCE_INSTRUCTEUR_EMAIL="" + +# want to stay on delayed job ? set as 'delayed_job' +RAILS_QUEUE_ADAPTER=" \ No newline at end of file diff --git a/config/environments/production.rb b/config/environments/production.rb index eb1d964eb..cf942cd6c 100644 --- a/config/environments/production.rb +++ b/config/environments/production.rb @@ -83,7 +83,7 @@ Rails.application.configure do end # Use a real queuing backend for Active Job (and separate queues per environment). - config.active_job.queue_adapter = :delayed_job + config.active_job.queue_adapter = ENV.fetch('RAILS_QUEUE_ADAPTER') { :sidekiq } # config.active_job.queue_name_prefix = "tps_production" config.action_mailer.perform_caching = false diff --git a/config/initializers/transition_to_sidekiq.rb b/config/initializers/transition_to_sidekiq.rb deleted file mode 100644 index a60869ac1..000000000 --- a/config/initializers/transition_to_sidekiq.rb +++ /dev/null @@ -1,35 +0,0 @@ -# frozen_string_literal: true - -if Rails.env.production? && SIDEKIQ_ENABLED - ActiveSupport.on_load(:after_initialize) do - [ - ActiveStorage::AnalyzeJob, - ActiveStorage::PurgeJob, - AdminUpdateDefaultZonesJob, - APIEntreprise::Job, - AdminUpdateDefaultZonesJob, - BatchOperationEnqueueAllJob, - BatchOperationProcessOneJob, - ChampFetchExternalDataJob, - Cron::CronJob, - DestroyRecordLaterJob, - DossierIndexSearchTermsJob, - DossierOperationLogMoveToColdStorageBatchJob, - DossierRebaseJob, - HelpscoutCreateConversationJob, - ImageProcessorJob, - MaintenanceTasks::TaskJob, - Migrations::BackfillStableIdJob, - PriorizedMailDeliveryJob, - ProcedureExternalURLCheckJob, - ProcedureSVASVRProcessDossierJob, - ProcessStalledDeclarativeDossierJob, - ResetExpiringDossiersJob, - SendClosingNotificationJob, - VirusScannerJob, - WebHookJob - ].each do |job_class| - job_class.queue_adapter = :sidekiq - end - end -end From 5eec93bc8c7209abcf47e5addb71449bdb702d20 Mon Sep 17 00:00:00 2001 From: mfo Date: Thu, 3 Oct 2024 11:34:28 +0200 Subject: [PATCH 2/3] clean(delayed_jobs): remove dependencies and all occurences --- Gemfile | 4 -- Gemfile.lock | 27 --------- README.md | 3 +- app/jobs/cron/cron_job.rb | 6 +- app/jobs/cron/release_crashed_export_job.rb | 46 ---------------- .../manager/application/_navigation.html.erb | 1 - bin/delayed_job | 5 -- config/deploy.rb | 13 ----- config/environments/development.rb | 4 +- config/initializers/delayed_job.rb | 6 -- config/initializers/sentry.rb | 25 ++------- config/routes.rb | 1 - lib/tasks/deploy.rake | 2 +- ...e_old_cron_job_from_delayed_job_table.rake | 16 ------ ...bious_proc_job_from_delayed_job_table.rake | 16 ------ ...estroy_dossier_transfer_without_email.rake | 27 --------- .../cron/release_crashed_export_job_spec.rb | 55 ------------------- 17 files changed, 10 insertions(+), 247 deletions(-) delete mode 100644 app/jobs/cron/release_crashed_export_job.rb delete mode 100755 bin/delayed_job delete mode 100644 config/initializers/delayed_job.rb delete mode 100644 lib/tasks/deployment/20220517040643_remove_old_cron_job_from_delayed_job_table.rake delete mode 100644 lib/tasks/deployment/20220517120321_remove_old_dubious_proc_job_from_delayed_job_table.rake delete mode 100644 lib/tasks/deployment/20220802133502_destroy_dossier_transfer_without_email.rake delete mode 100644 spec/jobs/cron/release_crashed_export_job_spec.rb diff --git a/Gemfile b/Gemfile index 3903e5db1..b6ad31cfe 100644 --- a/Gemfile +++ b/Gemfile @@ -26,9 +26,6 @@ gem 'chunky_png' gem 'clamav-client', require: 'clamav/client' gem 'daemons' gem 'deep_cloneable' # Enable deep clone of active record models -gem 'delayed_cron_job', require: false # Cron jobs -gem 'delayed_job_active_record' -gem 'delayed_job_web' gem 'devise' gem 'devise-i18n' gem 'devise-two-factor' @@ -89,7 +86,6 @@ gem 'rexml' # add missing gem due to ruby3 (https://github.com/Shopify/bootsnap/ gem 'rqrcode' gem 'saml_idp' gem 'sassc-rails' # Use SCSS for stylesheets -gem 'sentry-delayed_job' gem 'sentry-rails' gem 'sentry-ruby' gem 'sentry-sidekiq' diff --git a/Gemfile.lock b/Gemfile.lock index 394298047..c27054274 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -191,18 +191,6 @@ GEM date (3.3.4) deep_cloneable (3.2.0) activerecord (>= 3.1.0, < 8) - delayed_cron_job (0.9.0) - fugit (>= 1.5) - delayed_job (4.1.11) - activesupport (>= 3.0, < 8.0) - delayed_job_active_record (4.1.8) - activerecord (>= 3.0, < 8.0) - delayed_job (>= 3.0, < 5) - delayed_job_web (1.4.4) - activerecord (> 3.0.0) - delayed_job (> 2.0.3) - rack-protection (>= 1.5.5) - sinatra (>= 1.4.4) descendants_tracker (0.0.4) thread_safe (~> 0.3, >= 0.3.1) devise (4.9.4) @@ -448,8 +436,6 @@ GEM minitest (5.25.1) msgpack (1.7.2) multi_json (1.15.0) - mustermann (3.0.0) - ruby2_keywords (~> 0.0.1) net-http (0.4.1) uri net-imap (0.4.12) @@ -679,7 +665,6 @@ GEM ruby-progressbar (1.13.0) ruby-vips (2.2.0) ffi (~> 1.12) - ruby2_keywords (0.0.5) rubyzip (2.3.2) saml_idp (0.16.0) activesupport (>= 5.2) @@ -714,9 +699,6 @@ GEM rexml (~> 3.2, >= 3.2.5) rubyzip (>= 1.2.2, < 3.0) websocket (~> 1.0) - sentry-delayed_job (5.17.3) - delayed_job (>= 4.0) - sentry-ruby (~> 5.17.3) sentry-rails (5.17.3) railties (>= 5.0) sentry-ruby (~> 5.17.3) @@ -755,11 +737,6 @@ GEM simplecov_json_formatter (0.1.4) simpleidn (0.2.1) unf (~> 0.1.4) - sinatra (3.2.0) - mustermann (~> 3.0) - rack (~> 2.2, >= 2.2.4) - rack-protection (= 3.2.0) - tilt (~> 2.0) skylight (6.0.4) activesupport (>= 5.2.0) smart_properties (1.17.0) @@ -914,9 +891,6 @@ DEPENDENCIES clamav-client daemons deep_cloneable - delayed_cron_job - delayed_job_active_record - delayed_job_web devise devise-i18n devise-two-factor @@ -1000,7 +974,6 @@ DEPENDENCIES scss_lint selenium-devtools selenium-webdriver - sentry-delayed_job sentry-rails sentry-ruby sentry-sidekiq diff --git a/README.md b/README.md index 0261e2109..88fa5fd48 100644 --- a/README.md +++ b/README.md @@ -33,11 +33,12 @@ Vous souhaitez y apporter des changements ou des améliorations ? Lisez notre [ ``` -Nous sommes en cours de migration de `delayed_job` vers `sidekiq` pour le traitement des jobs asynchrones. Pour faire tourner sidekiq, vous aurez besoin de : - redis +#### Crédits et licences + - lightgallery : une license a été souscrite pour soutenir le projet, mais elle n'est pas obligatoire si la librairie est utilisée dans le cadre d'une application open source. #### Développement diff --git a/app/jobs/cron/cron_job.rb b/app/jobs/cron/cron_job.rb index 2124474a4..521d24ab0 100644 --- a/app/jobs/cron/cron_job.rb +++ b/app/jobs/cron/cron_job.rb @@ -38,11 +38,7 @@ class Cron::CronJob < ApplicationJob end def enqueued_cron_job - if queue_adapter_name == "sidekiq" - sidekiq_cron_job - else - delayed_job - end + sidekiq_cron_job end def sidekiq_cron_job diff --git a/app/jobs/cron/release_crashed_export_job.rb b/app/jobs/cron/release_crashed_export_job.rb deleted file mode 100644 index 23adf0d2f..000000000 --- a/app/jobs/cron/release_crashed_export_job.rb +++ /dev/null @@ -1,46 +0,0 @@ -# frozen_string_literal: true - -class Cron::ReleaseCrashedExportJob < Cron::CronJob - self.schedule_expression = "every 10 minute" - SECSCAN_LIMIT = 20_000 - - def perform(*args) - return if !performable? - export_jobs = jobs_for_current_host - - return if export_jobs.empty? - - host_pids = Sys::ProcTable.ps.map(&:pid) - export_jobs.each do |job| - _, pid = hostname_and_pid(job.locked_by) - - reset(job:) if host_pids.exclude?(pid.to_i) - end - end - - def reset(job:) - job.locked_by = nil - job.locked_at = nil - job.attempts += 1 - job.save! - end - - def hostname_and_pid(worker_name) - matches = /host:(?.*) pid:(?\d+)/.match(worker_name) - [matches[:host], matches[:pid]] - end - - def jobs_for_current_host - Delayed::Job.where("locked_by like ?", "%#{whoami}%") - .where(queue: ExportJob.queue_name) - end - - def whoami - me, _ = hostname_and_pid(Delayed::Worker.new.name) - me - end - - def performable? - Delayed::Job.count < SECSCAN_LIMIT - end -end diff --git a/app/views/manager/application/_navigation.html.erb b/app/views/manager/application/_navigation.html.erb index 83dc6da3e..d04f2f72f 100644 --- a/app/views/manager/application/_navigation.html.erb +++ b/app/views/manager/application/_navigation.html.erb @@ -23,7 +23,6 @@ as defined by the routes in the `admin/` namespace
- <%= link_to "Delayed Jobs", manager_delayed_job_path, class: "navigation__link" %> <%= link_to "Sidekiq", manager_sidekiq_web_path, class: "navigation__link" %> <%= link_to "Maintenance Tasks", manager_maintenance_tasks_path, class: "navigation__link" %> <%= link_to "Features", manager_flipper_path, class: "navigation__link" %> diff --git a/bin/delayed_job b/bin/delayed_job deleted file mode 100755 index edf195985..000000000 --- a/bin/delayed_job +++ /dev/null @@ -1,5 +0,0 @@ -#!/usr/bin/env ruby - -require File.expand_path(File.join(File.dirname(__FILE__), '..', 'config', 'environment')) -require 'delayed/command' -Delayed::Command.new(ARGV).daemonize diff --git a/config/deploy.rb b/config/deploy.rb index bd6694041..3eeaab8a2 100644 --- a/config/deploy.rb +++ b/config/deploy.rb @@ -100,18 +100,6 @@ namespace :service do #{echo_cmd %[test -f #{webserver_file_path} && sudo systemctl reload nginx]} } end - - desc "Restart delayed_job" - task :restart_delayed_job do - worker_file_path = File.join(deploy_to, 'shared', SHARED_WORKER_FILE_NAME) - - command %{ - echo "-----> Restarting delayed_job service" - #{echo_cmd %[test -f #{worker_file_path} && echo 'it is a worker marchine, restarting delayed_job']} - #{echo_cmd %[test -f #{worker_file_path} && sudo systemctl restart delayed_job]} - #{echo_cmd %[test -f #{worker_file_path} || echo "it is not a worker marchine, #{worker_file_path} is absent"]} - } - end end desc "Deploys the current version to the server." @@ -135,7 +123,6 @@ task :deploy do on :launch do invoke :'service:restart_puma' invoke :'service:reload_nginx' - invoke :'service:restart_delayed_job' invoke :'deploy:cleanup' end end diff --git a/config/environments/development.rb b/config/environments/development.rb index e6fdffbb7..b179e79e4 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -111,8 +111,8 @@ Rails.application.configure do # Annotate rendered view with file names. # config.action_view.annotate_rendered_view_with_filenames = true - # We use the async adapter by default, but delayed_job can be set using - # RAILS_QUEUE_ADAPTER=delayed_job bin/rails server + # We use the async adapter by default, but sidekiq can be set using + # RAILS_QUEUE_ADAPTER=sidekiq bin/rails server config.active_job.queue_adapter = ENV.fetch('RAILS_QUEUE_ADAPTER', 'async').to_sym # Use an evented file watcher to asynchronously detect changes in source code, diff --git a/config/initializers/delayed_job.rb b/config/initializers/delayed_job.rb deleted file mode 100644 index af7f691d6..000000000 --- a/config/initializers/delayed_job.rb +++ /dev/null @@ -1,6 +0,0 @@ -# frozen_string_literal: true - -# Set max_run_time at the highest job duration we want, -# then at job level we'll decrease this value to a lower value -# except for ExportJob. -Delayed::Worker.max_run_time = 16.hours # same as Export::MAX_DUREE_GENERATION but we can't yet use this constant here diff --git a/config/initializers/sentry.rb b/config/initializers/sentry.rb index c45a69db8..914e5d5a2 100644 --- a/config/initializers/sentry.rb +++ b/config/initializers/sentry.rb @@ -22,27 +22,10 @@ Sentry.init do |config| # transaction_context is the transaction object in hash form # keep in mind that sampling happens right after the transaction is initialized # for example, at the beginning of the request - transaction_context = sampling_context[:transaction_context] - - # transaction_context helps you sample transactions with more sophistication - # for example, you can provide different sample rates based on the operation or name - case transaction_context[:op] - when /delayed_job/ - contexts = Sentry.get_current_scope.contexts - job_class = contexts.dig(:"Active-Job", :job_class) - attempts = contexts.dig(:"Delayed-Job", :attempts) - max_attempts = job_class.safe_constantize&.new&.max_attempts rescue 25 - - # Don't trace on all attempts - [0, 2, 5, 10, 20, max_attempts].include?(attempts) - else # rails requests - if sampling_context.dig(:env, "REQUEST_METHOD") == "GET" - 0.001 - else - 0.01 - end + if sampling_context[:transaction_context].dig(:env, "REQUEST_METHOD") == "GET" + 0.001 + else + 0.01 end end - - config.delayed_job.report_after_job_retries = false # don't wait for all attempts before reporting end diff --git a/config/routes.rb b/config/routes.rb index e1f7cc8c2..99862a416 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -107,7 +107,6 @@ Rails.application.routes.draw do authenticate :super_admin do mount Flipper::UI.app(-> { Flipper.instance }) => "/features", as: :flipper - match "/delayed_job" => DelayedJobWeb, :anchor => false, :via => [:get, :post] mount MaintenanceTasks::Engine => "/maintenance_tasks" mount Sidekiq::Web => "/sidekiq" end diff --git a/lib/tasks/deploy.rake b/lib/tasks/deploy.rake index f7edf1cc5..db5fab695 100644 --- a/lib/tasks/deploy.rake +++ b/lib/tasks/deploy.rake @@ -37,6 +37,6 @@ task :rollback do branch = ENV.fetch('BRANCH') domains.each do |domain| - sh "mina rollback service:restart_puma service:reload_nginx service:restart_delayed_job domain=#{domain} branch=#{branch}" + sh "mina rollback service:restart_puma service:reload_nginx domain=#{domain} branch=#{branch}" end end diff --git a/lib/tasks/deployment/20220517040643_remove_old_cron_job_from_delayed_job_table.rake b/lib/tasks/deployment/20220517040643_remove_old_cron_job_from_delayed_job_table.rake deleted file mode 100644 index 6095daef7..000000000 --- a/lib/tasks/deployment/20220517040643_remove_old_cron_job_from_delayed_job_table.rake +++ /dev/null @@ -1,16 +0,0 @@ -# frozen_string_literal: true - -namespace :after_party do - desc 'Deployment task: remove_old_cron_job_from_delayed_job_table' - task remove_old_cron_job_from_delayed_job_table: :environment do - puts "Running deploy task 'remove_old_cron_job_from_delayed_job_table'" - - cron = Delayed::Job.where.not(cron: nil) - .where("handler LIKE ?", "%UpdateAdministrateurUsageStatisticsJob%") - .first - cron.destroy if cron - - AfterParty::TaskRecord - .create version: AfterParty::TaskRecorder.new(__FILE__).timestamp - end -end diff --git a/lib/tasks/deployment/20220517120321_remove_old_dubious_proc_job_from_delayed_job_table.rake b/lib/tasks/deployment/20220517120321_remove_old_dubious_proc_job_from_delayed_job_table.rake deleted file mode 100644 index ab6c32c12..000000000 --- a/lib/tasks/deployment/20220517120321_remove_old_dubious_proc_job_from_delayed_job_table.rake +++ /dev/null @@ -1,16 +0,0 @@ -# frozen_string_literal: true - -namespace :after_party do - desc 'Deployment task: remove_old_find_dubious_procedures_job_from_delayed_job_table' - task remove_old_dubious_proc_job_from_delayed_job_table: :environment do - puts "Running deploy task 'remove_old_dubious_proc_job_from_delayed_job_table'" - - cron = Delayed::Job.where.not(cron: nil) - .where("handler LIKE ?", "%FindDubiousProceduresJob%") - .first - cron.destroy if cron - - AfterParty::TaskRecord - .create version: AfterParty::TaskRecorder.new(__FILE__).timestamp - end -end diff --git a/lib/tasks/deployment/20220802133502_destroy_dossier_transfer_without_email.rake b/lib/tasks/deployment/20220802133502_destroy_dossier_transfer_without_email.rake deleted file mode 100644 index 7461f27e9..000000000 --- a/lib/tasks/deployment/20220802133502_destroy_dossier_transfer_without_email.rake +++ /dev/null @@ -1,27 +0,0 @@ -# frozen_string_literal: true - -namespace :after_party do - desc 'Deployment task: destroy_dossier_transfer_without_email' - task destroy_dossier_transfer_without_email: :environment do - puts "Running deploy task 'destroy_dossier_transfer_without_email'" - - invalid_dossiers = DossierTransfer.where(email: "") - - progress = ProgressReport.new(invalid_dossiers.count) - - invalid_dossiers.find_each do |dossier_transfer| - puts "Destroy dossier transfer #{dossier_transfer.id}" - dossier_transfer.destroy_and_nullify - - job = Delayed::Job.where("handler LIKE ALL(ARRAY[?, ?])", "%ActionMailer::MailDeliveryJob%", "%aj_globalid: gid://tps/DossierTransfer/#{dossier_transfer.id}\n%").first - job.destroy if job - - progress.inc - end - - # Update task as completed. If you remove the line below, the task will - # run with every deploy (or every time you call after_party:run). - AfterParty::TaskRecord - .create version: AfterParty::TaskRecorder.new(__FILE__).timestamp - end -end diff --git a/spec/jobs/cron/release_crashed_export_job_spec.rb b/spec/jobs/cron/release_crashed_export_job_spec.rb deleted file mode 100644 index f51454372..000000000 --- a/spec/jobs/cron/release_crashed_export_job_spec.rb +++ /dev/null @@ -1,55 +0,0 @@ -# frozen_string_literal: true - -describe Cron::ReleaseCrashedExportJob do - let(:handler) { "whocares" } - - def locked_by(hostname) - "delayed_job.33 host:#{hostname} pid:1252488" - end - - describe '.perform' do - subject { described_class.new.perform } - let!(:job) { Delayed::Job.create!(handler:, queue: ExportJob.queue_name, locked_by: locked_by(Socket.gethostname)) } - - it 'releases lock' do - expect { subject }.to change { job.reload.locked_by }.from(anything).to(nil) - end - it 'increases attempts' do - expect { subject }.to change { job.reload.attempts }.by(1) - end - end - - describe '.hostname_and_pid' do - subject { described_class.new.hostname_and_pid(Delayed::Worker.new.name) } - it 'extract hostname and pid from worker.name' do - hostname, pid = subject - - expect(hostname).to eq(Socket.gethostname) - expect(pid).to eq(Process.pid.to_s) - end - end - - describe 'whoami' do - subject { described_class.new.whoami } - it { is_expected.to eq(Socket.gethostname) } - end - - describe 'jobs_for_current_host' do - subject { described_class.new.jobs_for_current_host } - - context 'when jobs run an another host' do - let!(:job) { Delayed::Job.create!(handler:, queue: :default, locked_by: locked_by('spec1.prod')) } - it { is_expected.to be_empty } - end - - context 'when jobs run an same host with default queue' do - let!(:job) { Delayed::Job.create!(handler:, queue: :default, locked_by: locked_by(Socket.gethostname)) } - it { is_expected.to be_empty } - end - - context 'when jobs run an same host with exports queue' do - let!(:job) { Delayed::Job.create!(handler:, queue: ExportJob.queue_name, locked_by: locked_by(Socket.gethostname)) } - it { is_expected.to include(job) } - end - end -end From 02590b3a73bac7e116b613d7fff5ac5de981becd Mon Sep 17 00:00:00 2001 From: mfo Date: Tue, 8 Oct 2024 16:37:44 +0200 Subject: [PATCH 3/3] Revert "clean(delayed_jobs): remove dependencies and all occurences" This reverts commit 90ca937b7131575816d72e35e2c48f9d82e9a5e6. ReRevert me in order to remove all delayed job occurences. But we keep this dependencie for our instances at least for a year --- Gemfile | 4 ++ Gemfile.lock | 27 +++++++++ README.md | 3 +- app/jobs/cron/cron_job.rb | 6 +- app/jobs/cron/release_crashed_export_job.rb | 46 ++++++++++++++++ .../manager/application/_navigation.html.erb | 1 + bin/delayed_job | 5 ++ config/deploy.rb | 13 +++++ config/environments/development.rb | 4 +- config/initializers/delayed_job.rb | 6 ++ config/initializers/sentry.rb | 25 +++++++-- config/routes.rb | 1 + lib/tasks/deploy.rake | 2 +- ...e_old_cron_job_from_delayed_job_table.rake | 16 ++++++ ...bious_proc_job_from_delayed_job_table.rake | 16 ++++++ ...estroy_dossier_transfer_without_email.rake | 27 +++++++++ .../cron/release_crashed_export_job_spec.rb | 55 +++++++++++++++++++ 17 files changed, 247 insertions(+), 10 deletions(-) create mode 100644 app/jobs/cron/release_crashed_export_job.rb create mode 100755 bin/delayed_job create mode 100644 config/initializers/delayed_job.rb create mode 100644 lib/tasks/deployment/20220517040643_remove_old_cron_job_from_delayed_job_table.rake create mode 100644 lib/tasks/deployment/20220517120321_remove_old_dubious_proc_job_from_delayed_job_table.rake create mode 100644 lib/tasks/deployment/20220802133502_destroy_dossier_transfer_without_email.rake create mode 100644 spec/jobs/cron/release_crashed_export_job_spec.rb diff --git a/Gemfile b/Gemfile index b6ad31cfe..3903e5db1 100644 --- a/Gemfile +++ b/Gemfile @@ -26,6 +26,9 @@ gem 'chunky_png' gem 'clamav-client', require: 'clamav/client' gem 'daemons' gem 'deep_cloneable' # Enable deep clone of active record models +gem 'delayed_cron_job', require: false # Cron jobs +gem 'delayed_job_active_record' +gem 'delayed_job_web' gem 'devise' gem 'devise-i18n' gem 'devise-two-factor' @@ -86,6 +89,7 @@ gem 'rexml' # add missing gem due to ruby3 (https://github.com/Shopify/bootsnap/ gem 'rqrcode' gem 'saml_idp' gem 'sassc-rails' # Use SCSS for stylesheets +gem 'sentry-delayed_job' gem 'sentry-rails' gem 'sentry-ruby' gem 'sentry-sidekiq' diff --git a/Gemfile.lock b/Gemfile.lock index c27054274..394298047 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -191,6 +191,18 @@ GEM date (3.3.4) deep_cloneable (3.2.0) activerecord (>= 3.1.0, < 8) + delayed_cron_job (0.9.0) + fugit (>= 1.5) + delayed_job (4.1.11) + activesupport (>= 3.0, < 8.0) + delayed_job_active_record (4.1.8) + activerecord (>= 3.0, < 8.0) + delayed_job (>= 3.0, < 5) + delayed_job_web (1.4.4) + activerecord (> 3.0.0) + delayed_job (> 2.0.3) + rack-protection (>= 1.5.5) + sinatra (>= 1.4.4) descendants_tracker (0.0.4) thread_safe (~> 0.3, >= 0.3.1) devise (4.9.4) @@ -436,6 +448,8 @@ GEM minitest (5.25.1) msgpack (1.7.2) multi_json (1.15.0) + mustermann (3.0.0) + ruby2_keywords (~> 0.0.1) net-http (0.4.1) uri net-imap (0.4.12) @@ -665,6 +679,7 @@ GEM ruby-progressbar (1.13.0) ruby-vips (2.2.0) ffi (~> 1.12) + ruby2_keywords (0.0.5) rubyzip (2.3.2) saml_idp (0.16.0) activesupport (>= 5.2) @@ -699,6 +714,9 @@ GEM rexml (~> 3.2, >= 3.2.5) rubyzip (>= 1.2.2, < 3.0) websocket (~> 1.0) + sentry-delayed_job (5.17.3) + delayed_job (>= 4.0) + sentry-ruby (~> 5.17.3) sentry-rails (5.17.3) railties (>= 5.0) sentry-ruby (~> 5.17.3) @@ -737,6 +755,11 @@ GEM simplecov_json_formatter (0.1.4) simpleidn (0.2.1) unf (~> 0.1.4) + sinatra (3.2.0) + mustermann (~> 3.0) + rack (~> 2.2, >= 2.2.4) + rack-protection (= 3.2.0) + tilt (~> 2.0) skylight (6.0.4) activesupport (>= 5.2.0) smart_properties (1.17.0) @@ -891,6 +914,9 @@ DEPENDENCIES clamav-client daemons deep_cloneable + delayed_cron_job + delayed_job_active_record + delayed_job_web devise devise-i18n devise-two-factor @@ -974,6 +1000,7 @@ DEPENDENCIES scss_lint selenium-devtools selenium-webdriver + sentry-delayed_job sentry-rails sentry-ruby sentry-sidekiq diff --git a/README.md b/README.md index 88fa5fd48..0261e2109 100644 --- a/README.md +++ b/README.md @@ -33,12 +33,11 @@ Vous souhaitez y apporter des changements ou des améliorations ? Lisez notre [ ``` +Nous sommes en cours de migration de `delayed_job` vers `sidekiq` pour le traitement des jobs asynchrones. Pour faire tourner sidekiq, vous aurez besoin de : - redis -#### Crédits et licences - - lightgallery : une license a été souscrite pour soutenir le projet, mais elle n'est pas obligatoire si la librairie est utilisée dans le cadre d'une application open source. #### Développement diff --git a/app/jobs/cron/cron_job.rb b/app/jobs/cron/cron_job.rb index 521d24ab0..2124474a4 100644 --- a/app/jobs/cron/cron_job.rb +++ b/app/jobs/cron/cron_job.rb @@ -38,7 +38,11 @@ class Cron::CronJob < ApplicationJob end def enqueued_cron_job - sidekiq_cron_job + if queue_adapter_name == "sidekiq" + sidekiq_cron_job + else + delayed_job + end end def sidekiq_cron_job diff --git a/app/jobs/cron/release_crashed_export_job.rb b/app/jobs/cron/release_crashed_export_job.rb new file mode 100644 index 000000000..23adf0d2f --- /dev/null +++ b/app/jobs/cron/release_crashed_export_job.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +class Cron::ReleaseCrashedExportJob < Cron::CronJob + self.schedule_expression = "every 10 minute" + SECSCAN_LIMIT = 20_000 + + def perform(*args) + return if !performable? + export_jobs = jobs_for_current_host + + return if export_jobs.empty? + + host_pids = Sys::ProcTable.ps.map(&:pid) + export_jobs.each do |job| + _, pid = hostname_and_pid(job.locked_by) + + reset(job:) if host_pids.exclude?(pid.to_i) + end + end + + def reset(job:) + job.locked_by = nil + job.locked_at = nil + job.attempts += 1 + job.save! + end + + def hostname_and_pid(worker_name) + matches = /host:(?.*) pid:(?\d+)/.match(worker_name) + [matches[:host], matches[:pid]] + end + + def jobs_for_current_host + Delayed::Job.where("locked_by like ?", "%#{whoami}%") + .where(queue: ExportJob.queue_name) + end + + def whoami + me, _ = hostname_and_pid(Delayed::Worker.new.name) + me + end + + def performable? + Delayed::Job.count < SECSCAN_LIMIT + end +end diff --git a/app/views/manager/application/_navigation.html.erb b/app/views/manager/application/_navigation.html.erb index d04f2f72f..83dc6da3e 100644 --- a/app/views/manager/application/_navigation.html.erb +++ b/app/views/manager/application/_navigation.html.erb @@ -23,6 +23,7 @@ as defined by the routes in the `admin/` namespace
+ <%= link_to "Delayed Jobs", manager_delayed_job_path, class: "navigation__link" %> <%= link_to "Sidekiq", manager_sidekiq_web_path, class: "navigation__link" %> <%= link_to "Maintenance Tasks", manager_maintenance_tasks_path, class: "navigation__link" %> <%= link_to "Features", manager_flipper_path, class: "navigation__link" %> diff --git a/bin/delayed_job b/bin/delayed_job new file mode 100755 index 000000000..edf195985 --- /dev/null +++ b/bin/delayed_job @@ -0,0 +1,5 @@ +#!/usr/bin/env ruby + +require File.expand_path(File.join(File.dirname(__FILE__), '..', 'config', 'environment')) +require 'delayed/command' +Delayed::Command.new(ARGV).daemonize diff --git a/config/deploy.rb b/config/deploy.rb index 3eeaab8a2..bd6694041 100644 --- a/config/deploy.rb +++ b/config/deploy.rb @@ -100,6 +100,18 @@ namespace :service do #{echo_cmd %[test -f #{webserver_file_path} && sudo systemctl reload nginx]} } end + + desc "Restart delayed_job" + task :restart_delayed_job do + worker_file_path = File.join(deploy_to, 'shared', SHARED_WORKER_FILE_NAME) + + command %{ + echo "-----> Restarting delayed_job service" + #{echo_cmd %[test -f #{worker_file_path} && echo 'it is a worker marchine, restarting delayed_job']} + #{echo_cmd %[test -f #{worker_file_path} && sudo systemctl restart delayed_job]} + #{echo_cmd %[test -f #{worker_file_path} || echo "it is not a worker marchine, #{worker_file_path} is absent"]} + } + end end desc "Deploys the current version to the server." @@ -123,6 +135,7 @@ task :deploy do on :launch do invoke :'service:restart_puma' invoke :'service:reload_nginx' + invoke :'service:restart_delayed_job' invoke :'deploy:cleanup' end end diff --git a/config/environments/development.rb b/config/environments/development.rb index b179e79e4..e6fdffbb7 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -111,8 +111,8 @@ Rails.application.configure do # Annotate rendered view with file names. # config.action_view.annotate_rendered_view_with_filenames = true - # We use the async adapter by default, but sidekiq can be set using - # RAILS_QUEUE_ADAPTER=sidekiq bin/rails server + # We use the async adapter by default, but delayed_job can be set using + # RAILS_QUEUE_ADAPTER=delayed_job bin/rails server config.active_job.queue_adapter = ENV.fetch('RAILS_QUEUE_ADAPTER', 'async').to_sym # Use an evented file watcher to asynchronously detect changes in source code, diff --git a/config/initializers/delayed_job.rb b/config/initializers/delayed_job.rb new file mode 100644 index 000000000..af7f691d6 --- /dev/null +++ b/config/initializers/delayed_job.rb @@ -0,0 +1,6 @@ +# frozen_string_literal: true + +# Set max_run_time at the highest job duration we want, +# then at job level we'll decrease this value to a lower value +# except for ExportJob. +Delayed::Worker.max_run_time = 16.hours # same as Export::MAX_DUREE_GENERATION but we can't yet use this constant here diff --git a/config/initializers/sentry.rb b/config/initializers/sentry.rb index 914e5d5a2..c45a69db8 100644 --- a/config/initializers/sentry.rb +++ b/config/initializers/sentry.rb @@ -22,10 +22,27 @@ Sentry.init do |config| # transaction_context is the transaction object in hash form # keep in mind that sampling happens right after the transaction is initialized # for example, at the beginning of the request - if sampling_context[:transaction_context].dig(:env, "REQUEST_METHOD") == "GET" - 0.001 - else - 0.01 + transaction_context = sampling_context[:transaction_context] + + # transaction_context helps you sample transactions with more sophistication + # for example, you can provide different sample rates based on the operation or name + case transaction_context[:op] + when /delayed_job/ + contexts = Sentry.get_current_scope.contexts + job_class = contexts.dig(:"Active-Job", :job_class) + attempts = contexts.dig(:"Delayed-Job", :attempts) + max_attempts = job_class.safe_constantize&.new&.max_attempts rescue 25 + + # Don't trace on all attempts + [0, 2, 5, 10, 20, max_attempts].include?(attempts) + else # rails requests + if sampling_context.dig(:env, "REQUEST_METHOD") == "GET" + 0.001 + else + 0.01 + end end end + + config.delayed_job.report_after_job_retries = false # don't wait for all attempts before reporting end diff --git a/config/routes.rb b/config/routes.rb index 99862a416..e1f7cc8c2 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -107,6 +107,7 @@ Rails.application.routes.draw do authenticate :super_admin do mount Flipper::UI.app(-> { Flipper.instance }) => "/features", as: :flipper + match "/delayed_job" => DelayedJobWeb, :anchor => false, :via => [:get, :post] mount MaintenanceTasks::Engine => "/maintenance_tasks" mount Sidekiq::Web => "/sidekiq" end diff --git a/lib/tasks/deploy.rake b/lib/tasks/deploy.rake index db5fab695..f7edf1cc5 100644 --- a/lib/tasks/deploy.rake +++ b/lib/tasks/deploy.rake @@ -37,6 +37,6 @@ task :rollback do branch = ENV.fetch('BRANCH') domains.each do |domain| - sh "mina rollback service:restart_puma service:reload_nginx domain=#{domain} branch=#{branch}" + sh "mina rollback service:restart_puma service:reload_nginx service:restart_delayed_job domain=#{domain} branch=#{branch}" end end diff --git a/lib/tasks/deployment/20220517040643_remove_old_cron_job_from_delayed_job_table.rake b/lib/tasks/deployment/20220517040643_remove_old_cron_job_from_delayed_job_table.rake new file mode 100644 index 000000000..6095daef7 --- /dev/null +++ b/lib/tasks/deployment/20220517040643_remove_old_cron_job_from_delayed_job_table.rake @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +namespace :after_party do + desc 'Deployment task: remove_old_cron_job_from_delayed_job_table' + task remove_old_cron_job_from_delayed_job_table: :environment do + puts "Running deploy task 'remove_old_cron_job_from_delayed_job_table'" + + cron = Delayed::Job.where.not(cron: nil) + .where("handler LIKE ?", "%UpdateAdministrateurUsageStatisticsJob%") + .first + cron.destroy if cron + + AfterParty::TaskRecord + .create version: AfterParty::TaskRecorder.new(__FILE__).timestamp + end +end diff --git a/lib/tasks/deployment/20220517120321_remove_old_dubious_proc_job_from_delayed_job_table.rake b/lib/tasks/deployment/20220517120321_remove_old_dubious_proc_job_from_delayed_job_table.rake new file mode 100644 index 000000000..ab6c32c12 --- /dev/null +++ b/lib/tasks/deployment/20220517120321_remove_old_dubious_proc_job_from_delayed_job_table.rake @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +namespace :after_party do + desc 'Deployment task: remove_old_find_dubious_procedures_job_from_delayed_job_table' + task remove_old_dubious_proc_job_from_delayed_job_table: :environment do + puts "Running deploy task 'remove_old_dubious_proc_job_from_delayed_job_table'" + + cron = Delayed::Job.where.not(cron: nil) + .where("handler LIKE ?", "%FindDubiousProceduresJob%") + .first + cron.destroy if cron + + AfterParty::TaskRecord + .create version: AfterParty::TaskRecorder.new(__FILE__).timestamp + end +end diff --git a/lib/tasks/deployment/20220802133502_destroy_dossier_transfer_without_email.rake b/lib/tasks/deployment/20220802133502_destroy_dossier_transfer_without_email.rake new file mode 100644 index 000000000..7461f27e9 --- /dev/null +++ b/lib/tasks/deployment/20220802133502_destroy_dossier_transfer_without_email.rake @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +namespace :after_party do + desc 'Deployment task: destroy_dossier_transfer_without_email' + task destroy_dossier_transfer_without_email: :environment do + puts "Running deploy task 'destroy_dossier_transfer_without_email'" + + invalid_dossiers = DossierTransfer.where(email: "") + + progress = ProgressReport.new(invalid_dossiers.count) + + invalid_dossiers.find_each do |dossier_transfer| + puts "Destroy dossier transfer #{dossier_transfer.id}" + dossier_transfer.destroy_and_nullify + + job = Delayed::Job.where("handler LIKE ALL(ARRAY[?, ?])", "%ActionMailer::MailDeliveryJob%", "%aj_globalid: gid://tps/DossierTransfer/#{dossier_transfer.id}\n%").first + job.destroy if job + + progress.inc + end + + # Update task as completed. If you remove the line below, the task will + # run with every deploy (or every time you call after_party:run). + AfterParty::TaskRecord + .create version: AfterParty::TaskRecorder.new(__FILE__).timestamp + end +end diff --git a/spec/jobs/cron/release_crashed_export_job_spec.rb b/spec/jobs/cron/release_crashed_export_job_spec.rb new file mode 100644 index 000000000..f51454372 --- /dev/null +++ b/spec/jobs/cron/release_crashed_export_job_spec.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +describe Cron::ReleaseCrashedExportJob do + let(:handler) { "whocares" } + + def locked_by(hostname) + "delayed_job.33 host:#{hostname} pid:1252488" + end + + describe '.perform' do + subject { described_class.new.perform } + let!(:job) { Delayed::Job.create!(handler:, queue: ExportJob.queue_name, locked_by: locked_by(Socket.gethostname)) } + + it 'releases lock' do + expect { subject }.to change { job.reload.locked_by }.from(anything).to(nil) + end + it 'increases attempts' do + expect { subject }.to change { job.reload.attempts }.by(1) + end + end + + describe '.hostname_and_pid' do + subject { described_class.new.hostname_and_pid(Delayed::Worker.new.name) } + it 'extract hostname and pid from worker.name' do + hostname, pid = subject + + expect(hostname).to eq(Socket.gethostname) + expect(pid).to eq(Process.pid.to_s) + end + end + + describe 'whoami' do + subject { described_class.new.whoami } + it { is_expected.to eq(Socket.gethostname) } + end + + describe 'jobs_for_current_host' do + subject { described_class.new.jobs_for_current_host } + + context 'when jobs run an another host' do + let!(:job) { Delayed::Job.create!(handler:, queue: :default, locked_by: locked_by('spec1.prod')) } + it { is_expected.to be_empty } + end + + context 'when jobs run an same host with default queue' do + let!(:job) { Delayed::Job.create!(handler:, queue: :default, locked_by: locked_by(Socket.gethostname)) } + it { is_expected.to be_empty } + end + + context 'when jobs run an same host with exports queue' do + let!(:job) { Delayed::Job.create!(handler:, queue: ExportJob.queue_name, locked_by: locked_by(Socket.gethostname)) } + it { is_expected.to include(job) } + end + end +end