Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Automatically restart timed tasks with delay in the past #10028

Merged
merged 3 commits into from
Mar 18, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions app/jobs/task_timer_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,14 @@ def process(task_timer)
task_timer.with_lock do
return if task_timer.processed?

task_timer.attempted!
task_timer.task.when_timer_ends
task_timer.processed!
end
rescue StandardError => e
# Ensure errors are sent to Sentry, but don't block the job from continuing.
# The next time the job runs, we'll process the unprocessed task timers again.
task_timer.update_error!(e.inspect)
Raven.capture_exception(e)
end
end
5 changes: 4 additions & 1 deletion app/models/concerns/timeable_task.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@ def create!(args)
fail Caseflow::Error::MissingTimerMethod unless method_defined?(:timer_ends_at)

super(args).tap do |task|
TaskTimer.new(task: task).submit_for_processing!(delay: task.timer_ends_at)
timer = TaskTimer.new(task: task)
timer.submit_for_processing!(delay: task.timer_ends_at)
# if timer_ends_at is in the past, we automatically trigger processing now.
timer.restart! if timer.expired_without_processing?
end
end
end
Expand Down
7 changes: 5 additions & 2 deletions spec/jobs/task_timer_job_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,10 @@ def timer_ends_at

TaskTimerJob.perform_now

expect(timer.reload.processed_at).not_to eq(nil)
expect(error_timer.reload.processed_at).to eq(nil)
expect(timer.reload.processed_at).not_to be_nil
expect(timer.reload.attempted_at).not_to be_nil
expect(error_timer.reload.processed_at).to be_nil
expect(error_timer.error).to eq("RuntimeError")
expect(error_timer.attempted_at).to be_nil # because it was in a failed transaction
end
end
24 changes: 23 additions & 1 deletion spec/models/concerns/timeable_task_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,16 @@ class AnotherTimedTask < GenericTask
def timer_ends_at; end
end

class OldTimedTask < GenericTask
include TimeableTask

def when_timer_ends; end

def timer_ends_at
NOW - 5.days
end
end

before do
Timecop.freeze(NOW)
end
Expand All @@ -29,7 +39,19 @@ def timer_ends_at; end
task = SomeTimedTask.create!(appeal: appeal, assigned_to: Bva.singleton)
timers = TaskTimer.where(task: task)
expect(timers.length).to eq(1)
expect(timers.first.last_submitted_at).to eq(Time.zone.now + 5.days - 3.hours + 1.minute)
delayed_start = Time.zone.now + 5.days - 3.hours + 1.minute
expect(timers.first.last_submitted_at).to eq(delayed_start)
expect(timers.first.submitted_at).to eq(delayed_start)
end

it "queues itself when the delay is in the past" do
task = OldTimedTask.create!(appeal: appeal, assigned_to: Bva.singleton)
timers = TaskTimer.where(task: task)
expect(timers.length).to eq(1)
expect(timers.first.submitted_and_ready?).to eq(true)
expect(timers.first.last_submitted_at).to eq(NOW)
delay_in_the_past = Time.zone.now - 5.days - 3.hours + 1.minute
expect(timers.first.submitted_at).to eq(delay_in_the_past)
end

context "when not correctly configured" do
Expand Down