Skip to content

Commit

Permalink
Migrate 'in queue' jobs to good_jobs table.
Browse files Browse the repository at this point in the history 
Update good_job version as well.
  • Loading branch information
@ba1ash
ba1ash committed Mar 5, 2024
1 parent ee88c89 commit c69d5eb
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 4 deletions.
3 changes: 2 additions & 1 deletion Gemfile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,8 @@ gem 'multi_json', '~> 1.15.0'
gem 'oj', '~> 3.16.0'

gem 'daemons'
gem 'good_job'
gem 'delayed_job', require: false # only needed for ActiveJob::QueueAdapters::DelayedJobAdapter::JobWrapper to be available in db/migrate/20240227154544_remove_delayed_jobs.rb

This comment has been minimized.

Sign in to view

Copy link
@machisuji

machisuji Mar 5, 2024 •
edited
Loading

Member

I would prefer it we could make this work without keeping around the delayed_job dependency.
I would propose just defining the class directly in the migration itself as follows.

module ActiveJob
  module QueueAdapters
    class DelayedJobAdapter
      class JobWrapper
        attr_accessor :job_data

        def initialize(job_data)
          @job_data = job_data
        end
      end
    end
  end
end
gem 'good_job', '~> 3.26.1' # update should be done manually in sync with saas-openproject version.

gem 'rack-protection', '~> 3.2.0'

Expand Down
9 changes: 6 additions & 3 deletions Gemfile.lock
View file
Original file line number Diff line number Diff line change
Expand Up @@ -445,6 +445,8 @@ 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)
Expand Down Expand Up @@ -543,7 +545,7 @@ GEM
friendly_id (5.5.1)
activerecord (>= 4.0.0)
front_matter_parser (1.0.1)
fugit (1.9.0)
fugit (1.10.1)
et-orbi (~> 1, >= 1.2.7)
raabro (~> 1.4)
fuubar (2.5.1)
Expand All @@ -557,7 +559,7 @@ GEM
i18n (>= 0.7)
multi_json
request_store (>= 1.0)
good_job (3.21.5)
good_job (3.26.1)
activejob (>= 6.0.0)
activerecord (>= 6.0.0)
concurrent-ruby (>= 1.0.2)
Expand Down Expand Up @@ -1160,6 +1162,7 @@ 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
Expand All @@ -1177,7 +1180,7 @@ DEPENDENCIES
friendly_id (~> 5.5.0)
fuubar (~> 2.5.0)
gon (~> 6.4.0)
good_job
good_job (~> 3.26.1)
google-apis-gmail_v1
googleauth
grape (~> 2.0.0)
Expand Down
29 changes: 29 additions & 0 deletions db/migrate/20240227154544_remove_delayed_jobs.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,35 @@

class RemoveDelayedJobs < ActiveRecord::Migration[7.1]
def change
reversible do |direction|
direction.up do

This comment has been minimized.

Sign in to view

Copy link
@machisuji

machisuji Mar 5, 2024 •
edited
Loading

Member

The workers shouldn't be running during the migration but just in case they are I would propose locking all existing jobs before the migration. Since each migration runs in a transaction this probably can't be in the same migration. Contrary to the new advice on not performing data migrations, in this case I would make an exception and add a new migration before this one that simply locks all existing delayed jobs so they are not picked up by a worker during the following migration. Otherwise jobs might be executed twice, once by delayed_job and once by good_job.
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
SQL
tuples.each do |tuple|
job_data = YAML.load(tuple['handler'],
permitted_classes: [ActiveJob::QueueAdapters::DelayedJobAdapter::JobWrapper])
.job_data
new_uuid = SecureRandom.uuid
good_job_record = GoodJob::BaseExecution.new
good_job_record.id = new_uuid
good_job_record.serialized_params = job_data
good_job_record.serialized_params['job_id'] = new_uuid
good_job_record.queue_name = job_data['queue_name']
good_job_record.priority = job_data['priority']
good_job_record.scheduled_at = job_data['scheduled_at']
good_job_record.active_job_id = new_uuid
good_job_record.concurrency_key = nil
good_job_record.job_class = job_data['job_class']
good_job_record.save!
end
end
direction.down {}
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.
Expand Down

0 comments on commit c69d5eb

Please sign in to comment.