Skip to content

Commit

Permalink
Prevent even one iteration in the race condition case
Browse files Browse the repository at this point in the history
  • Loading branch information
etiennebarrie committed Nov 3, 2021
1 parent 3c425d7 commit a71a3db
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 1 deletion.
2 changes: 2 additions & 0 deletions app/models/maintenance_tasks/run.rb
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,8 @@ def running
if updated
self.status = :running
clear_attribute_changes([:status])
else
reload_status
end
end

Expand Down
6 changes: 6 additions & 0 deletions test/dummy/app/jobs/custom_task_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,10 @@ class CustomTaskJob < MaintenanceTasks::TaskJob
raise "Error enqueuing" if run.task_name == "Maintenance::EnqueueErrorTask"
throw :abort if run.task_name == "Maintenance::CancelledEnqueueTask"
end

class_attribute :race_condition_hook, instance_accessor: false

before_perform(prepend: true) do
CustomTaskJob.race_condition_hook&.call
end
end
17 changes: 16 additions & 1 deletion test/jobs/maintenance_tasks/task_job_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ class TaskJobTest < ActiveJob::TestCase
assert_no_enqueued_jobs
end

test ".perform_now skips iterations when Run is cancelled" do
test ".perform_now avoids iterating when Run is cancelled" do
@run.cancelling!

Maintenance::TestTask.any_instance.expects(:process).never
Expand All @@ -64,6 +64,21 @@ class TaskJobTest < ActiveJob::TestCase
assert_no_enqueued_jobs
end

test ".perform_now avoids iterating and cancels Run when a race occurs between starting and cancelling" do
CustomTaskJob.race_condition_hook = -> do
Run.find(@run.id).update(status: :cancelling)
end

Maintenance::TestTask.any_instance.expects(:process).never

CustomTaskJob.perform_now(@run)

assert_predicate(@run.reload, :cancelled?)
assert_no_enqueued_jobs
ensure
CustomTaskJob.race_condition_hook = nil
end

test ".perform_now updates tick_count" do
Maintenance::TestTask.any_instance.expects(:process).twice

Expand Down
12 changes: 12 additions & 0 deletions test/models/maintenance_tasks/run_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,18 @@ class RunTest < ActiveSupport::TestCase
end
end

test "#running doesn't set a stopping run to running and reloads the status" do
[:cancelling, :pausing].each do |status|
run = Run.create!(
task_name: "Maintenance::UpdatePostsTask",
)
Run.find(run.id).update(status: status) # race condition
run.running

assert_equal status.to_s, run.status
end
end

test "#cancel transitions the Run to cancelling if not paused" do
[:enqueued, :running, :pausing, :interrupted].each do |status|
run = Run.create!(
Expand Down

0 comments on commit a71a3db

Please sign in to comment.