diff --git a/app/models/concerns/good_job/reportable.rb b/app/models/concerns/good_job/reportable.rb index 5217c251..18623d6b 100644 --- a/app/models/concerns/good_job/reportable.rb +++ b/app/models/concerns/good_job/reportable.rb @@ -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 diff --git a/app/models/good_job/batch_record.rb b/app/models/good_job/batch_record.rb index ac7e263d..89839500 100644 --- a/app/models/good_job/batch_record.rb +++ b/app/models/good_job/batch_record.rb @@ -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 diff --git a/app/models/good_job/job.rb b/app/models/good_job/job.rb index cc21233f..3325b723 100644 --- a/app/models/good_job/job.rb +++ b/app/models/good_job/job.rb @@ -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 @@ -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 @@ -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. @@ -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 diff --git a/spec/app/models/good_job/job_spec.rb b/spec/app/models/good_job/job_spec.rb index 4220466d..a9e636c4 100644 --- a/spec/app/models/good_job/job_spec.rb +++ b/spec/app/models/good_job/job_spec.rb @@ -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 diff --git a/spec/system/jobs_spec.rb b/spec/system/jobs_spec.rb index 6dda74d9..f7b5d44c 100644 --- a/spec/system/jobs_spec.rb +++ b/spec/system/jobs_spec.rb @@ -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" @@ -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