Skip to content

Commit

Permalink
Remove references to and ignore good_jobs.retried_good_job_id column (
Browse files Browse the repository at this point in the history
  • Loading branch information
bensheldon authored Jul 16, 2024
1 parent 327cb92 commit 76450c5
Show file tree
Hide file tree
Showing 5 changed files with 15 additions and 41 deletions.
20 changes: 8 additions & 12 deletions app/models/concerns/good_job/reportable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,27 +16,23 @@ module Reportable
# @return [Symbol]
def status
if finished_at.present?
if error.present? && retried_good_job_id.present?
:retried
elsif error.present? && retried_good_job_id.nil?
if error.present?
:discarded
else
:succeeded
end
elsif (scheduled_at || created_at) > DateTime.current
if serialized_params.fetch('executions', 0) > 1
:retried
else
:scheduled
end
elsif running?
elsif performed_at.present?
:running
else
elsif (scheduled_at || created_at) <= DateTime.current
:queued
elsif error.present?
:retried
else
:scheduled
end
end

# The last relevant timestamp for this execution
# The last relevant timestamp for this job
def last_status_at
finished_at || performed_at || scheduled_at || created_at
end
Expand Down
2 changes: 1 addition & 1 deletion app/models/good_job/batch_record.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ def display_attributes
end

def _continue_discard_or_finish(execution = nil, lock: true)
execution_discarded = execution && execution.error.present? && execution.finished_at && execution.retried_good_job_id.nil?
execution_discarded = execution && execution.finished_at.present? && execution.error.present?
take_advisory_lock(lock) do
Batch.within_thread(batch_id: nil, batch_callback_id: id) do
reload
Expand Down
8 changes: 4 additions & 4 deletions app/models/good_job/job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ class Job < BaseRecord
self.table_name = 'good_jobs'
self.advisory_lockable_column = 'id'
self.implicit_order_column = 'created_at'
self.ignored_columns += ["is_discrete"]
self.ignored_columns += %w[is_discrete retried_good_job_id]

define_model_callbacks :perform
define_model_callbacks :perform_unlocked, only: :after
Expand Down Expand Up @@ -61,7 +61,7 @@ class Job < BaseRecord
# Advisory locked and executing
scope :running, -> { where(finished_at: nil).joins_advisory_locks.where.not(pg_locks: { locktype: nil }) }
# Finished executing (succeeded or discarded)
scope :finished, -> { where.not(finished_at: nil).where(retried_good_job_id: nil) }
scope :finished, -> { where.not(finished_at: nil) }
# Completed executing successfully
scope :succeeded, -> { finished.where(error: nil) }
# Errored but will not be retried
Expand Down Expand Up @@ -461,7 +461,7 @@ def running?
# Tests whether the job has finished (succeeded or discarded).
# @return [Boolean]
def finished?
finished_at.present? && retried_good_job_id.nil?
finished_at.present?
end

# Tests whether the job has finished but with an error.
Expand Down Expand Up @@ -687,7 +687,7 @@ def perform(lock_id:)
execution.duration = monotonic_duration

retry_unhandled_error = result.unhandled_error && GoodJob.retry_on_unhandled_error
reenqueued = result.retried? || retried_good_job_id.present? || retry_unhandled_error
reenqueued = result.retried? || retry_unhandled_error
if reenqueued
job_attributes[:performed_at] = nil
job_attributes[:finished_at] = nil
Expand Down
6 changes: 1 addition & 5 deletions spec/app/models/good_job/job_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -94,12 +94,8 @@ def perform(*)
describe '#finished?' do
it 'is true if the job has finished' do
expect do
job.update(finished_at: Time.current, retried_good_job_id: nil)
job.update(finished_at: Time.current)
end.to change(job, :finished?).from(false).to(true)

expect do
job.update(finished_at: Time.current, retried_good_job_id: SecureRandom.uuid)
end.to change(job, :finished?).from(true).to(false)
end
end

Expand Down
20 changes: 1 addition & 19 deletions spec/system/jobs_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -140,21 +140,7 @@
end

it 'can force discard jobs' do
unfinished_job.update scheduled_at: 1.hour.ago

locked_event = Concurrent::Event.new
done_event = Concurrent::Event.new

promise = Concurrent::Promises.future do
rails_promise do
# pretend the job is running
unfinished_job.with_advisory_lock do
locked_event.set
done_event.wait(20)
end
end
end
locked_event.wait(10)
unfinished_job.update performed_at: 1.hour.ago, finished_at: nil

visit '/good_job'
click_link "Jobs"
Expand All @@ -166,10 +152,6 @@
end
expect(page).to have_content "Job has been force discarded"
end.to change { unfinished_job.reload.finished_at }.from(nil).to within(1.second).of(Time.current)
ensure
locked_event.set
done_event.set
promise.value!
end

it 'can destroy jobs' do
Expand Down

0 comments on commit 76450c5

Please sign in to comment.