Passer de delayed_job à good_job (#3338)

* Switch from delayed_job to good_job * Set default retry behaviour (same as DelayedJob for now) * Complete a test * Fix typo * Use latest migration script from good_job gem * Fix rubocop offences * Add comment * Finely tune settings * Small fixes * Last retry actually raises exception, just check that Sentry gets 1 occurrence * Update good_job gem to latest patch version * Fix name of Scalingo container (see Procfile)
betagouv · Feb 23, 2023 · a55705c · a55705c
1 parent b95b286
commit a55705c
Show file tree

Hide file tree

Showing 32 changed files with 291 additions and 180 deletions.
diff --git a/Gemfile b/Gemfile
@@ -48,9 +48,7 @@ gem "pundit"
 gem "devise_token_auth", github: "lynndylanhurley/devise_token_auth"
 
 # Jobs
-gem "delayed_job_active_record"
-gem "delayed_job_web"
-gem "delayed_cron_job"
+gem "good_job"
 gem "daemons"
 
 # JSON serialization and queries

diff --git a/Gemfile.lock b/Gemfile.lock
@@ -148,7 +148,7 @@ GEM
     coderay (1.1.3)
     coercible (1.0.0)
       descendants_tracker (~> 0.0.1)
-    concurrent-ruby (1.1.10)
+    concurrent-ruby (1.2.0)
     crack (0.4.5)
       rexml
     crass (1.0.6)
@@ -163,18 +163,6 @@ GEM
     database_cleaner-core (2.0.1)
     date (3.3.3)
     debug_inspector (1.1.0)
-    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.7)
-      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.8.1)
@@ -212,11 +200,19 @@ GEM
       faraday (>= 1, < 3)
     faraday-net_http (3.0.2)
     ffi (1.15.5)
-    fugit (1.7.2)
+    fugit (1.8.1)
       et-orbi (~> 1, >= 1.2.7)
       raabro (~> 1.4)
-    globalid (1.0.1)
+    globalid (1.1.0)
       activesupport (>= 5.0)
+    good_job (3.12.3)
+      activejob (>= 6.0.0)
+      activerecord (>= 6.0.0)
+      concurrent-ruby (>= 1.0.2)
+      fugit (>= 1.1)
+      railties (>= 6.0.0)
+      thor (>= 0.14.1)
+      webrick (>= 1.3)
     groupdate (6.1.0)
       activesupport (>= 5.2)
     hashdiff (1.0.1)
@@ -296,8 +292,6 @@ GEM
       activesupport (>= 5.2, < 7.1)
     msgpack (1.6.0)
     multi_xml (0.6.0)
-    mustermann (3.0.0)
-      ruby2_keywords (~> 0.0.1)
     net-imap (0.3.4)
       date
       net-protocol
@@ -308,7 +302,7 @@ GEM
     net-smtp (0.3.3)
       net-protocol
     nio4r (2.5.8)
-    nokogiri (1.14.0)
+    nokogiri (1.14.2)
       mini_portile2 (~> 2.8.0)
       racc (~> 1.4)
     oauth2 (2.0.9)
@@ -421,7 +415,7 @@ GEM
       activesupport (>= 4.2)
       choice (~> 0.2.0)
       ruby-graphviz (~> 1.2)
-    rails-html-sanitizer (1.4.4)
+    rails-html-sanitizer (1.5.0)
       loofah (~> 2.19, >= 2.19.1)
     rails_autolink (1.1.7)
       rails (> 3.1)
@@ -532,11 +526,6 @@ GEM
       simplecov_json_formatter (~> 0.1)
     simplecov-html (0.12.3)
     simplecov_json_formatter (0.1.4)
-    sinatra (3.0.4)
-      mustermann (~> 3.0)
-      rack (~> 2.2, >= 2.2.4)
-      rack-protection (= 3.0.4)
-      tilt (~> 2.0)
     skylight (5.3.4)
       activesupport (>= 5.2.0)
     slim (4.1.0)
@@ -575,7 +564,7 @@ GEM
     turbolinks-source (5.2.0)
     typhoeus (1.4.0)
       ethon (>= 0.9.0)
-    tzinfo (2.0.5)
+    tzinfo (2.0.6)
       concurrent-ruby (~> 1.0)
     unicode-display_width (2.3.0)
     uniform_notifier (1.16.0)
@@ -604,13 +593,14 @@ GEM
       addressable (>= 2.8.0)
       crack (>= 0.3.2)
       hashdiff (>= 0.4.0, < 2.0.0)
+    webrick (1.8.1)
     websocket (1.2.9)
     websocket-driver (0.7.5)
       websocket-extensions (>= 0.1.0)
     websocket-extensions (0.1.5)
     xpath (3.2.0)
       nokogiri (~> 1.8)
-    zeitwerk (2.6.6)
+    zeitwerk (2.6.7)
 
 PLATFORMS
   ruby
@@ -636,16 +626,14 @@ DEPENDENCIES
   chartkick (~> 3.4.0)
   daemons
   database_cleaner
-  delayed_cron_job
-  delayed_job_active_record
-  delayed_job_web
   devise
   devise-async
   devise_invitable
   devise_token_auth!
   dotenv-rails
   factory_bot
   faker
+  good_job
   groupdate (~> 6.1)
   hiredis
   icalendar (~> 2.5)

diff --git a/Procfile b/Procfile
@@ -1,3 +1,3 @@
 web: ./bin/start_web_server
-jobs: bundle exec bin/delayed_job run
+jobs: bundle exec good_job start
 postdeploy: bundle exec rake db:migrate
diff --git a/Procfile.dev b/Procfile.dev
@@ -1,3 +1,3 @@
 web: ./bin/start_web_server
-worker: ./bin/rake jobs:work
+jobs: bundle exec good_job start
 js: yarn build --mode=development --watch --progress
diff --git a/app/jobs/application_job.rb b/app/jobs/application_job.rb
@@ -1,4 +1,7 @@
 # frozen_string_literal: true
 
 class ApplicationJob < ActiveJob::Base
+  include DefaultJobBehaviour
+
+  queue_as :default
 end
diff --git a/app/jobs/concerns/default_job_behaviour.rb b/app/jobs/concerns/default_job_behaviour.rb
@@ -0,0 +1,24 @@
+# frozen_string_literal: true
+
+module DefaultJobBehaviour
+  extend ActiveSupport::Concern
+
+  included do
+    # This retry_on means:
+    # "retry 20 times with an exponential backoff, then mark job as discarded"
+    #
+    # Exponential backoff is n^4, so wait times between retries will be as follows:
+    # attempt:  1   2    3    4   5    6    7    8    9     10    11  12  13  14   15   16   17   18   19   20
+    # backoff:  1s, 16s, 81s, 4m, 10m, 21m, 40m, 68m, 109m, 166m, 4h, 6h, 8h, 11h, 14h, 18h, 23h, 29h, 36h, 44h
+    retry_on(StandardError, wait: :exponentially_longer, attempts: 20)
+
+    # Makes sure every failed attempt is logged to Sentry
+    # (see: https://github.com/bensheldon/good_job#retries)
+    around_perform do |_job, block|
+      block.call
+    rescue StandardError => e
+      Sentry.capture_exception(e)
+      raise # will be caught by the retry mechanism
+    end
+  end
+end
diff --git a/app/jobs/cron_job.rb b/app/jobs/cron_job.rb
@@ -1,53 +1,15 @@
 # frozen_string_literal: true
 
 class CronJob < ApplicationJob
-  # Cron jobs superclass
-  # See https://github.com/codez/delayed_cron_job#custom-cronjob-superclass
-  # It mostly ensures there is only one single scheduled job of each cron job class.
   queue_as :cron
 
-  class_attribute :cron_expression
-
-  class << self
-    def schedule
-      set(cron: cron_expression).perform_later unless scheduled?
-      update_cron_expression!
-    end
-
-    def remove
-      delayed_job.destroy if scheduled?
-    end
-
-    def scheduled?
-      delayed_job.present?
-    end
-
-    def update_cron_expression!
-      delayed_job.update!(cron: cron_expression)
-    end
-
-    def delayed_job
-      Delayed::Job
-        .where("handler LIKE ?", "%job_class: #{name}%")
-        .first
-    end
-  end
-
-  ## Actual Cron Jobs
-  #
   class FileAttenteJob < CronJob
-    # Every 10 minutes, from 9:00 to 18:00
-    self.cron_expression = "0/10 9,10,11,12,13,14,15,16,17,18 * * *"
-
     def perform
       FileAttente.send_notifications
     end
   end
 
   class ReminderJob < CronJob
-    # At 3:00 every day
-    self.cron_expression = "0 3 * * *"
-
     def perform
       Rdv.not_cancelled.day_after_tomorrow.find_each do |rdv|
         run_at = rdv.starts_at - 48.hours
@@ -57,9 +19,6 @@ def perform
   end
 
   class UpdateExpirationsJob < CronJob
-    # At 1:00 every day
-    self.cron_expression = "0 1 * * *"
-
     def perform
       [PlageOuverture, Absence].each do |klass|
         klass.not_expired.find_each(&:refresh_expired_cached)
@@ -68,9 +27,6 @@ def perform
   end
 
   class WarmUpOccurrencesCache < CronJob
-    # At 23:00 every day
-    self.cron_expression = "0 23 * * *"
-
     def perform
       [PlageOuverture, Absence].each do |klass|
         klass.regulieres.not_expired.find_each do |model|
@@ -81,9 +37,6 @@ def perform
   end
 
   class DestroyOldRdvsJob < CronJob
-    # At 2:00 every day
-    self.cron_expression = "0 2 * * *"
-
     def perform
       Rdv.unscoped.where(starts_at: ..2.years.ago).each do |rdv|
         rdv.skip_webhooks = true
@@ -93,9 +46,6 @@ def perform
   end
 
   class DestroyOldPlageOuvertureJob < CronJob
-    # At 01:00 every day
-    self.cron_expression = "0 1 * * *"
-
     def perform
       po_exceptionnelle_closed_since_1_year = PlageOuverture.where(recurrence: nil).where(first_day: ..1.year.ago)
       po_reccurent_closed_since_1_year = PlageOuverture.where(recurrence_ends_at: ..1.year.ago)
@@ -107,9 +57,6 @@ def perform
   end
 
   class DestroyRedisWaitingRoomKeys < CronJob
-    # At 03:00 every day
-    self.cron_expression = "0 3 * * *"
-
     def perform
       Rdv.reset_user_in_waiting_room!
     end

diff --git a/app/jobs/outlook/create_event_job.rb b/app/jobs/outlook/create_event_job.rb
@@ -2,6 +2,8 @@
 
 module Outlook
   class CreateEventJob < ApplicationJob
+    queue_as :outlook_sync
+
     def perform(agents_rdv)
       outlook_event = Outlook::Event.new(agents_rdv: agents_rdv).create
       agents_rdv.update(outlook_id: outlook_event["id"], skip_outlook_update: true, outlook_create_in_progress: false)

diff --git a/app/jobs/outlook/destroy_event_job.rb b/app/jobs/outlook/destroy_event_job.rb
@@ -2,6 +2,8 @@
 
 module Outlook
   class DestroyEventJob < ApplicationJob
+    queue_as :outlook_sync
+
     def perform(outlook_id, agent)
       outlook_event = Outlook::Event.new(outlook_id: outlook_id, agent: agent).destroy
 

diff --git a/app/jobs/outlook/mass_create_event_job.rb b/app/jobs/outlook/mass_create_event_job.rb
@@ -2,6 +2,8 @@
 
 module Outlook
   class MassCreateEventJob < ApplicationJob
+    queue_as :outlook_sync
+
     def perform(agent)
       agent.agents_rdvs.future.each(&:sync_create_in_outlook_asynchronously)
     end

diff --git a/app/jobs/outlook/mass_destroy_event_job.rb b/app/jobs/outlook/mass_destroy_event_job.rb
@@ -2,6 +2,8 @@
 
 module Outlook
   class MassDestroyEventJob < ApplicationJob
+    queue_as :outlook_sync
+
     def perform(agent)
       agent.agents_rdvs.exists_in_outlook.each do |agents_rdv|
         Outlook::DestroyEventJob.perform_now(agents_rdv.outlook_id, agents_rdv.agent)

diff --git a/app/jobs/outlook/update_event_job.rb b/app/jobs/outlook/update_event_job.rb
@@ -2,6 +2,8 @@
 
 module Outlook
   class UpdateEventJob < ApplicationJob
+    queue_as :outlook_sync
+
     def perform(agents_rdv)
       Outlook::Event.new(agents_rdv: agents_rdv).update
     end

diff --git a/app/jobs/rdv_upcoming_reminder_job.rb b/app/jobs/rdv_upcoming_reminder_job.rb
@@ -1,6 +1,8 @@
 # frozen_string_literal: true
 
 class RdvUpcomingReminderJob < ApplicationJob
+  queue_as :reminders
+
   def perform(rdv)
     Notifiers::RdvUpcomingReminder.perform_with(rdv, nil)
   end

diff --git a/app/jobs/sms_job.rb b/app/jobs/sms_job.rb
@@ -1,6 +1,8 @@
 # frozen_string_literal: true
 
 class SmsJob < ApplicationJob
+  queue_as :sms
+
   class InvalidMobilePhoneNumberError < StandardError; end
 
   def perform(sender_name:, phone_number:, content:, provider:, api_key:, receipt_params:) # rubocop:disable Metrics/ParameterLists

diff --git a/app/jobs/webhook_job.rb b/app/jobs/webhook_job.rb
@@ -7,6 +7,8 @@ class WebhookJob < ApplicationJob
 
   queue_as :webhook
 
+  retry_on(OutgoingWebhookError, wait: :exponentially_longer, attempts: 10, queue: :webhook_retries)
+
   def perform(payload, webhook_endpoint_id)
     webhook_endpoint = WebhookEndpoint.find(webhook_endpoint_id)
 
@@ -41,7 +43,7 @@ def perform(payload, webhook_endpoint_id)
   # c'est en général lié à une mise à jour
   # ou une suppression qui ne fonctionne pas
   #
-  # Ce petit paliatif est là en attendant qu'ils
+  # Ce petit palliatif est là en attendant qu'ils
   # fassent évoluer leur système.
   def self.false_negative_from_drome?(body)
     body = JSON.parse(body)

diff --git a/app/mailers/custom_mailer_delivery_job.rb b/app/mailers/custom_mailer_delivery_job.rb
@@ -2,6 +2,8 @@
 
 # See https://www.bigbinary.com/blog/rails-5-2-allows-mailers-to-use-custom-active-job-class
 class CustomMailerDeliveryJob < ActionMailer::MailDeliveryJob
+  include DefaultJobBehaviour
+
   # Only discard DeserializationError if it is caused by a ActiveRecord::RecordNotFound.
   # We don't want to discard a job when deserialization failed because of a DB failure for example.
   rescue_from ActiveJob::DeserializationError do |exception|

diff --git a/app/services/notifiers/rdv_upcoming_reminder.rb b/app/services/notifiers/rdv_upcoming_reminder.rb
@@ -8,10 +8,10 @@ def rdvs_users_to_notify
   end
 
   def notify_user_by_mail(user)
-    user_mailer(user).rdv_upcoming_reminder.deliver_later(queue: :mailers_low)
+    user_mailer(user).rdv_upcoming_reminder.deliver_later(queue: :mailers_low, priority: -10)
   end
 
   def notify_user_by_sms(user)
-    Users::RdvSms.rdv_upcoming_reminder(@rdv, user, @rdv_users_tokens_by_user_id[user.id]).deliver_later(queue: :sms_low)
+    Users::RdvSms.rdv_upcoming_reminder(@rdv, user, @rdv_users_tokens_by_user_id[user.id]).deliver_later(queue: :sms_low, priority: -10)
   end
 end