From dbab61ad3f637ee79241a9af737d4628b8f41e4c Mon Sep 17 00:00:00 2001 From: Pavel Balashou Date: Tue, 5 Mar 2024 17:39:27 +0100 Subject: [PATCH] Remove delayed_job dependecy. Lock jobs. - Remove delayed_job dependecy completely from Gemfile. - Lock jobs during migration from delayed_jobs to good_jobs using SELECT FOR UPDATE. It means that potentially not shut down DJ worker will not be able to pick them up for performing during the migration process. After migration has been finished the jobs in question will be in good_jobs table and delayed_jobs table will be removed. So, duplicate execution should not happen. --- Gemfile | 1 - Gemfile.lock | 3 -- .../20240227154544_remove_delayed_jobs.rb | 41 ++++++++++++------- 3 files changed, 27 insertions(+), 18 deletions(-) diff --git a/Gemfile b/Gemfile index 6174cbae7754..0a102b3e3236 100644 --- a/Gemfile +++ b/Gemfile @@ -124,7 +124,6 @@ gem 'multi_json', '~> 1.15.0' gem 'oj', '~> 3.16.0' gem 'daemons' -gem 'delayed_job', require: false # only needed for ActiveJob::QueueAdapters::DelayedJobAdapter::JobWrapper to be available in db/migrate/20240227154544_remove_delayed_jobs.rb gem 'good_job', '~> 3.26.1' # update should be done manually in sync with saas-openproject version. gem 'rack-protection', '~> 3.2.0' diff --git a/Gemfile.lock b/Gemfile.lock index ae94811ef538..8be2ae2c4ea2 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -445,8 +445,6 @@ GEM deckar01-task_list (2.3.4) html-pipeline (~> 2.0) declarative (0.0.20) - delayed_job (4.1.11) - activesupport (>= 3.0, < 8.0) descendants_tracker (0.0.4) thread_safe (~> 0.3, >= 0.3.1) diff-lcs (1.5.1) @@ -1162,7 +1160,6 @@ DEPENDENCIES date_validator (~> 0.12.0) debug deckar01-task_list (~> 2.3.1) - delayed_job disposable (~> 0.6.2) doorkeeper (~> 5.6.6) dotenv-rails diff --git a/db/migrate/20240227154544_remove_delayed_jobs.rb b/db/migrate/20240227154544_remove_delayed_jobs.rb index bff7e2421e82..e5bfb6413df1 100644 --- a/db/migrate/20240227154544_remove_delayed_jobs.rb +++ b/db/migrate/20240227154544_remove_delayed_jobs.rb @@ -27,19 +27,32 @@ #++ class RemoveDelayedJobs < ActiveRecord::Migration[7.1] + # it is needed, because ActiveJob::QueueAdapters::DelayedJobAdapter::JobWrapper + # can not be used without required delayed_job + # See https://github.com/rails/rails/blob/6f0d1ad14b92b9f5906e44740fce8b4f1c7075dc/activejob/lib/active_job/queue_adapters/delayed_job_adapter.rb + class JobWrapperDeserializationMock + attr_accessor :job_data + + def initialize(job_data) + @job_data = job_data + end + end + def change reversible do |direction| direction.up do tuples = execute <<~SQL select * from delayed_jobs - where locked_by is null -- not in progress - and handler NOT LIKE '%job_class: Storages::ManageNextcloudIntegrationEventsJob%' -- no need to migrate. It will be run later with cron. - and cron is null; -- not cron scheduled + where locked_by is null -- not in progress + and handler NOT LIKE '%job_class: Storages::ManageNextcloudIntegrationEventsJob%' -- no need to migrate. It will be run later with cron. + and cron is null -- not cron schedule + FOR UPDATE; -- to prevent potentialy running delayed_job process working on these jobs(delayed_job uses SELECT FOR UPDATE to get workable jobs) SQL tuples.each do |tuple| - job_data = YAML.load(tuple['handler'], - permitted_classes: [ActiveJob::QueueAdapters::DelayedJobAdapter::JobWrapper]) - .job_data + handler = tuple['handler'].gsub('ruby/object:ActiveJob::QueueAdapters::DelayedJobAdapter::JobWrapper', + "ruby/object:#{RemoveDelayedJobs::JobWrapperDeserializationMock.name}") + job_data = YAML.load(handler, permitted_classes: [RemoveDelayedJobs::JobWrapperDeserializationMock]) + .job_data new_uuid = SecureRandom.uuid good_job_record = GoodJob::BaseExecution.new good_job_record.id = new_uuid @@ -58,14 +71,14 @@ def change end drop_table :delayed_jobs do |t| - t.integer :priority, default: 0 # Allows some jobs to jump to the front of the queue - t.integer :attempts, default: 0 # Provides for retries, but still fail eventually. - t.text :handler # YAML-encoded string of the object that will do work - t.text :last_error # reason for last failure (See Note below) - t.datetime :run_at # When to run. Could be Time.zone.now for immediately, or sometime in the future. - t.datetime :locked_at # Set when a client is working on this object - t.datetime :failed_at # Set when all retries have failed (actually, by default, the record is deleted instead) - t.string :locked_by # Who is working on this object (if locked) + t.integer :priority, default: 0 + t.integer :attempts, default: 0 + t.text :handler + t.text :last_error + t.datetime :run_at + t.datetime :locked_at + t.datetime :failed_at + t.string :locked_by t.timestamps null: true t.string :queue t.string :cron