Browse Source

ActiveJobLock enhancements (cleanup and Delayed::Job destroy sync)

Thorsten Eckel 5 years ago
parent
commit
4c9919a23e

+ 7 - 0
app/jobs/active_job_lock_cleanup_job.rb

@@ -0,0 +1,7 @@
+class ActiveJobLockCleanupJob < ApplicationJob
+  include HasActiveJobLock
+
+  def perform(diff = 1.day)
+    ::ActiveJobLock.where('created_at < ?', Time.zone.now - diff).destroy_all
+  end
+end

+ 20 - 0
config/initializers/delayed_jobs_ensure_active_job_lock_removal.rb

@@ -0,0 +1,20 @@
+require 'delayed_job'
+
+module Delayed
+  class Job < ::ActiveRecord::Base
+
+    after_destroy :remove_active_job_lock
+
+    def remove_active_job_lock
+      # only ActiveJob Delayed::Jobs can have a lock
+      return if !payload_object.is_a?(::ActiveJob::QueueAdapters::DelayedJobAdapter::JobWrapper)
+
+      # deserialize ActiveJob and load it's arguments to generate the lock_key
+      active_job           = ::ActiveJob::Base.deserialize(payload_object.job_data)
+      active_job.arguments = ::ActiveJob::Arguments.deserialize(active_job.instance_variable_get(:@serialized_arguments))
+
+      # remove possible lock
+      active_job.try(:release_active_job_lock!)
+    end
+  end
+end

+ 15 - 0
db/migrate/20191129102720_active_job_lock_cleanup_job_scheduler.rb

@@ -0,0 +1,15 @@
+class ActiveJobLockCleanupJobScheduler < ActiveRecord::Migration[5.2]
+  def up
+    return if !Setting.find_by(name: 'system_init_done')
+
+    Scheduler.create_or_update(
+      name:          'Cleanup ActiveJob locks.',
+      method:        'ActiveJobLockCleanupJob.perform_now',
+      period:        1.day,
+      prio:          2,
+      active:        true,
+      updated_by_id: 1,
+      created_by_id: 1,
+    )
+  end
+end

+ 9 - 0
db/seeds/schedulers.rb

@@ -127,6 +127,15 @@ Scheduler.create_or_update(
   updated_by_id: 1,
   created_by_id: 1,
 )
+Scheduler.create_or_update(
+  name:          'Cleanup ActiveJob locks.',
+  method:        'ActiveJobLockCleanupJob.perform_now',
+  period:        1.day,
+  prio:          2,
+  active:        true,
+  updated_by_id: 1,
+  created_by_id: 1,
+)
 Scheduler.create_or_update(
   name:          'Sync calendars with ical feeds.',
   method:        'Calendar.sync',

+ 23 - 0
spec/db/migrate/active_job_lock_cleanup_job_scheduler_spec.rb

@@ -0,0 +1,23 @@
+require 'rails_helper'
+
+RSpec.describe ActiveJobLockCleanupJobScheduler, type: :db_migration do
+
+  let(:scheduler_method) { 'ActiveJobLockCleanupJob.perform_now' }
+
+  context 'New system', system_init_done: false do
+    it 'has no work to do' do
+      expect { migrate }.not_to change { Scheduler.exists?(method: scheduler_method) }.from(true)
+    end
+  end
+
+  context 'System that is already set up' do
+
+    before do
+      Scheduler.find_by(method: scheduler_method).destroy!
+    end
+
+    it 'creates Scheduler' do
+      expect { migrate }.to change { Scheduler.exists?(method: scheduler_method) }
+    end
+  end
+end

+ 27 - 0
spec/jobs/active_job_lock_cleanup_job_spec.rb

@@ -0,0 +1,27 @@
+require 'rails_helper'
+
+RSpec.describe ActiveJobLockCleanupJob, type: :job do
+
+  context 'when ActiveJobLock records older than a day are present' do
+
+    before do
+      create(:active_job_lock, created_at: 1.day.ago)
+      travel 1.minute
+    end
+
+    it 'cleans up those jobs' do
+      expect { described_class.perform_now }.to change(ActiveJobLock, :count).by(-1)
+    end
+  end
+
+  context 'when recent ActiveJobLock records are present' do
+
+    before do
+      create(:active_job_lock, created_at: 1.minute.ago)
+    end
+
+    it 'keeps those jobs' do
+      expect { described_class.perform_now }.not_to change(ActiveJobLock, :count)
+    end
+  end
+end

+ 17 - 0
spec/jobs/concerns/has_active_job_lock_spec.rb

@@ -80,6 +80,23 @@ RSpec.describe HasActiveJobLock, type: :job do
       it 'allows execution of perform_now jobs' do
         expect { job_class.perform_now }.to change(job_class, :perform_counter).by(1)
       end
+
+      context 'when Delayed::Job gets destroyed' do
+
+        before do
+          ::ActiveJob::Base.queue_adapter = :delayed_job
+        end
+
+        it 'is ensured that ActiveJobLock gets removed' do
+          job = job_class.perform_later
+
+          expect do
+            Delayed::Job.find(job.provider_job_id).destroy!
+          end.to change {
+            ActiveJobLock.exists?(lock_key: job.lock_key, active_job_id: job.job_id)
+          }.to(false)
+        end
+      end
     end
 
     context 'dynamic lock key' do

+ 1 - 0
spec/support/active_job.rb

@@ -33,6 +33,7 @@ RSpec.configure do |config|
 
       example.run
 
+    ensure
       ::ActiveJob::Base.queue_adapter = default_queue_adapter
     end
   end