diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index b802b1e1d..16a5a1565 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -92,8 +92,8 @@ jobs: strategy: fail-fast: false matrix: - ruby: [2.6, 2.7, "3.0", 3.1, 3.2, 3.3] - pg: [15] + ruby: ["3.0", 3.1, 3.2, 3.3] + pg: [16] include: - ruby: 3.3 pg: 11 @@ -103,6 +103,8 @@ jobs: pg: 13 - ruby: 3.3 pg: 14 + - ruby: 3.3 + pg: 15 env: PGHOST: localhost PGUSER: good_job diff --git a/.rubocop.yml b/.rubocop.yml index 62ce405bd..d80ecf5d2 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -12,7 +12,7 @@ inherit_mode: - Include AllCops: - TargetRubyVersion: 2.6 + TargetRubyVersion: 3.0 TargetRailsVersion: 6.0 DisplayCopNames: true DisplayStyleGuide: true diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 46657425d..08ae0107d 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1,7 +1,27 @@ # This configuration was generated by # `rubocop --auto-gen-config` -# on 2020-07-16 00:15:42 UTC using RuboCop version 0.88.0. +# on 2024-07-07 01:28:10 UTC using RuboCop version 1.57.2. # The point is for the user to remove these configuration records # one by one as the offenses are removed from the code base. # Note that changes in the inspected code, or installation of new # versions of RuboCop, may require this file to be generated again. + +# Offense count: 1 +# This cop supports unsafe autocorrection (--autocorrect-all). +Lint/RedundantDirGlobSort: + Exclude: + - 'spec/rails_helper.rb' + +# Offense count: 3 +# This cop supports unsafe autocorrection (--autocorrect-all). +Performance/MapCompact: + Exclude: + - 'app/controllers/good_job/jobs_controller.rb' + - 'lib/good_job/multi_scheduler.rb' + +# Offense count: 2 +# This cop supports safe autocorrection (--autocorrect). +# Configuration parameters: AllowOnlyRestArgument, UseAnonymousForwarding. +Style/ArgumentsForwarding: + Exclude: + - 'app/models/good_job/batch.rb' diff --git a/.ruby-version b/.ruby-version index bea438e9a..619b53766 100644 --- a/.ruby-version +++ b/.ruby-version @@ -1 +1 @@ -3.3.1 +3.3.3 diff --git a/Appraisals b/Appraisals index b65dd787c..5621e8c48 100644 --- a/Appraisals +++ b/Appraisals @@ -1,16 +1,10 @@ # frozen_string_literal: true -ruby_27_or_higher = Gem::Version.new(RUBY_VERSION) >= Gem::Version.new('2.7') +ruby_30_or_higher = Gem::Version.new(RUBY_VERSION) >= Gem::Version.new('3.0') ruby_31_or_higher = Gem::Version.new(RUBY_VERSION) >= Gem::Version.new('3.1') jruby = RUBY_PLATFORM.include?('java') unless ruby_31_or_higher # https://github.com/rails/rails/issues/44090#issuecomment-1007686519 - appraise "rails-6.0" do - gem "rails", "~> 6.0.0" - gem "traces", "~> 0.9.1" - gem "puma", "~> 5.6" - end - appraise "rails-6.1" do gem "rails", "~> 6.1.0" gem "traces", "~> 0.9.1" @@ -18,10 +12,7 @@ unless ruby_31_or_higher # https://github.com/rails/rails/issues/44090#issuecomm end end -if ruby_27_or_higher && !ruby_31_or_higher && !jruby - # Rails HEAD requires MRI 2.7+ - # activerecord-jdbcpostgresql-adapter does not have a compatible version - +if ruby_30_or_higher && !ruby_31_or_higher && !jruby appraise "rails-7.0" do gem "rails", "~> 7.0.0" gem "selenium-webdriver", "~> 4.0" # https://github.com/rails/rails/pull/43498 diff --git a/Gemfile.lock b/Gemfile.lock index d645d35fa..e765689d8 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -2,12 +2,12 @@ PATH remote: . specs: good_job (3.99.0) - activejob (>= 6.0.0) - activerecord (>= 6.0.0) - concurrent-ruby (>= 1.0.2) - fugit (>= 1.1) - railties (>= 6.0.0) - thor (>= 0.14.1) + activejob (>= 6.1.0) + activerecord (>= 6.1.0) + concurrent-ruby (>= 1.3.1) + fugit (>= 1.11.0) + railties (>= 6.1.0) + thor (>= 1.0.0) GEM remote: https://rubygems.org/ @@ -146,7 +146,7 @@ GEM xpath (~> 3.2) chef-utils (18.3.0) concurrent-ruby - concurrent-ruby (1.2.3) + concurrent-ruby (1.3.3) connection_pool (2.4.1) console (1.23.2) fiber-annotation @@ -175,7 +175,7 @@ GEM rubocop smart_properties erubi (1.12.0) - et-orbi (1.2.7) + et-orbi (1.2.11) tzinfo faraday (2.7.11) base64 @@ -189,8 +189,8 @@ GEM fiber-annotation (0.2.0) fiber-local (1.0.0) foreman (0.87.2) - fugit (1.9.0) - et-orbi (~> 1, >= 1.2.7) + fugit (1.11.0) + et-orbi (~> 1, >= 1.2.11) raabro (~> 1.4) gem-release (2.2.2) github_changelog_generator (1.16.4) @@ -541,7 +541,7 @@ DEPENDENCIES yard-activesupport-concern RUBY VERSION - ruby 3.3.1p55 + ruby 3.3.3p89 BUNDLED WITH 2.5.9 diff --git a/app/charts/good_job/scheduled_by_queue_chart.rb b/app/charts/good_job/scheduled_by_queue_chart.rb index 18ba15508..d6df51273 100644 --- a/app/charts/good_job/scheduled_by_queue_chart.rb +++ b/app/charts/good_job/scheduled_by_queue_chart.rb @@ -11,7 +11,7 @@ def data start_time = end_time - 1.day table_name = GoodJob::Job.table_name - count_query = Arel.sql(GoodJob::Execution.pg_or_jdbc_query(<<~SQL.squish)) + count_query = Arel.sql(GoodJob::Job.pg_or_jdbc_query(<<~SQL.squish)) SELECT * FROM generate_series( date_trunc('hour', $1::timestamp), @@ -35,7 +35,7 @@ def data ActiveRecord::Relation::QueryAttribute.new('start_time', start_time, ActiveRecord::Type::DateTime.new), ActiveRecord::Relation::QueryAttribute.new('end_time', end_time, ActiveRecord::Type::DateTime.new), ] - executions_data = GoodJob::Execution.connection.exec_query(GoodJob::Execution.pg_or_jdbc_query(count_query), "GoodJob Dashboard Chart", binds) + executions_data = GoodJob::Job.connection.exec_query(GoodJob::Job.pg_or_jdbc_query(count_query), "GoodJob Dashboard Chart", binds) queue_names = executions_data.reject { |d| d['count'].nil? }.map { |d| d['queue_name'] || BaseFilter::EMPTY }.uniq labels = [] diff --git a/app/controllers/good_job/cron_entries_controller.rb b/app/controllers/good_job/cron_entries_controller.rb index 3b31eb2ea..8971238a1 100644 --- a/app/controllers/good_job/cron_entries_controller.rb +++ b/app/controllers/good_job/cron_entries_controller.rb @@ -2,8 +2,6 @@ module GoodJob class CronEntriesController < GoodJob::ApplicationController - before_action :check_settings_migration!, only: [:enable, :disable] - def index @cron_entries = CronEntry.all end @@ -30,11 +28,5 @@ def disable @cron_entry.disable redirect_back(fallback_location: cron_entries_path, notice: t(".notice")) end - - private - - def check_settings_migration! - redirect_back(fallback_location: cron_entries_path, alert: t("good_job.cron_entries.pending_migrations")) unless GoodJob::Setting.migrated? - end end end diff --git a/app/controllers/good_job/metrics_controller.rb b/app/controllers/good_job/metrics_controller.rb index 38278f616..80e626e14 100644 --- a/app/controllers/good_job/metrics_controller.rb +++ b/app/controllers/good_job/metrics_controller.rb @@ -4,7 +4,7 @@ module GoodJob class MetricsController < ApplicationController def primary_nav jobs_count = GoodJob::Job.count - batches_count = GoodJob::BatchRecord.migrated? ? GoodJob::BatchRecord.all.size : 0 + batches_count = GoodJob::BatchRecord.all.size cron_entries_count = GoodJob::CronEntry.all.size processes_count = GoodJob::Process.active.count diff --git a/app/controllers/good_job/performances_controller.rb b/app/controllers/good_job/performances_controller.rb index 1becd93d0..06254cbc9 100644 --- a/app/controllers/good_job/performances_controller.rb +++ b/app/controllers/good_job/performances_controller.rb @@ -3,21 +3,17 @@ module GoodJob class PerformancesController < ApplicationController def show - if GoodJob::DiscreteExecution.duration_interval_migrated? - @performances = GoodJob::DiscreteExecution - .where.not(job_class: nil) - .group(:job_class) - .select(" + @performances = GoodJob::DiscreteExecution + .where.not(job_class: nil) + .group(:job_class) + .select(" job_class, COUNT(*) AS executions_count, AVG(duration) AS avg_duration, MIN(duration) AS min_duration, MAX(duration) AS max_duration ") - .order("job_class") - else - @needs_upgrade = true - end + .order("job_class") end end end diff --git a/app/models/concerns/good_job/filterable.rb b/app/models/concerns/good_job/filterable.rb index b773e3017..bd2599c5f 100644 --- a/app/models/concerns/good_job/filterable.rb +++ b/app/models/concerns/good_job/filterable.rb @@ -35,7 +35,7 @@ module Filterable next if query.blank? # TODO: turn this into proper bind parameters in Arel - tsvector = "(to_tsvector('english', id::text) || to_tsvector('english', COALESCE(active_job_id::text, '')) || to_tsvector('english', serialized_params) || to_tsvector('english', COALESCE(error, ''))#{" || to_tsvector('english', COALESCE(array_to_string(labels, ' '), ''))" if labels_migrated?})" + tsvector = "(to_tsvector('english', id::text) || to_tsvector('english', COALESCE(active_job_id::text, '')) || to_tsvector('english', serialized_params) || to_tsvector('english', COALESCE(error, '')) || to_tsvector('english', COALESCE(array_to_string(labels, ' '), '')))" to_tsquery_function = database_supports_websearch_to_tsquery? ? 'websearch_to_tsquery' : 'plainto_tsquery' where("#{tsvector} @@ #{to_tsquery_function}(?)", query) .order(sanitize_sql_for_order([Arel.sql("ts_rank(#{tsvector}, #{to_tsquery_function}(?))"), query]) => 'DESC') diff --git a/app/models/good_job/base_execution.rb b/app/models/good_job/base_execution.rb index 4dc6d6c3a..62cb6f48d 100644 --- a/app/models/good_job/base_execution.rb +++ b/app/models/good_job/base_execution.rb @@ -74,9 +74,6 @@ def self.queue_parser(string) end end - belongs_to :batch, class_name: 'GoodJob::BatchRecord', optional: true, inverse_of: :executions - has_many :discrete_executions, class_name: 'GoodJob::DiscreteExecution', foreign_key: 'active_job_id', primary_key: 'active_job_id', inverse_of: :execution # rubocop:disable Rails/HasManyOrHasOneDependent - # With a given class name # @!method job_class(name) # @!scope class @@ -84,11 +81,6 @@ def self.queue_parser(string) # @return [ActiveRecord::Relation] scope :job_class, ->(name) { where(params_job_class.eq(name)) } - after_destroy lambda { - GoodJob::DiscreteExecution.where(active_job_id: active_job_id).delete_all if discrete? # TODO: move into association `dependent: :delete_all` after v4 - self.class.active_job_id(active_job_id).delete_all - }, if: -> { @_destroy_job } - # Get jobs with given ActiveJob ID # @!method active_job_id(active_job_id) # @!scope class @@ -226,67 +218,12 @@ def coalesce_scheduled_at_created_at end def discrete_support? - GoodJob::DiscreteExecution.migrated? - end - - def error_event_migrated? - return true if columns_hash["error_event"].present? - - migration_pending_warning! - false - end - - def cron_indices_migrated? - return true if connection.index_name_exists?(:good_jobs, :index_good_jobs_on_cron_key_and_created_at_cond) - - migration_pending_warning! - false - end - - def labels_migrated? - return true if columns_hash["labels"].present? - - migration_pending_warning! - false - end - - def labels_indices_migrated? - return true if connection.index_name_exists?(:good_jobs, :index_good_jobs_on_labels) - - migration_pending_warning! - false + true end - - def active_job_id_index_removal_migrated? - return true unless connection.index_name_exists?(:good_jobs, :index_good_jobs_on_active_job_id) - - migration_pending_warning! - false - end - - def candidate_lookup_index_migrated? - return true if connection.index_name_exists?(:good_jobs, :index_good_job_jobs_for_candidate_lookup) - - migration_pending_warning! - false - end - - def process_lock_migrated? - return true if connection.index_name_exists?(:good_job_executions, :index_good_job_executions_on_process_id_and_created_at) - - migration_pending_warning! - false - end - end - - # The ActiveJob job class, as a string - # @return [String] - def job_class - discrete? ? attributes['job_class'] : serialized_params['job_class'] end def discrete? - self.class.discrete_support? && is_discrete? + is_discrete? end # Build an ActiveJob instance and deserialize the arguments, using `#active_job_data`. @@ -304,54 +241,54 @@ def active_job(ignore_deserialization_errors: false) raise unless ignore_deserialization_errors end - def self.build_for_enqueue(active_job, overrides = {}) - new(**enqueue_args(active_job, overrides)) + def self.build_for_enqueue(active_job, scheduled_at: nil) + new(**enqueue_args(active_job, scheduled_at: scheduled_at)) end # Construct arguments for GoodJob::Execution from an ActiveJob instance. - def self.enqueue_args(active_job, overrides = {}) - if active_job.priority && GoodJob.configuration.smaller_number_is_higher_priority.in?([nil, false]) - GoodJob.deprecator.warn(<<~DEPRECATION) - The next major version of GoodJob (v4.0) will change job `priority` to give smaller numbers higher priority (default: `0`), in accordance with Active Job's definition of priority. - To opt-in to this behavior now, set `config.good_job.smaller_number_is_higher_priority = true` in your GoodJob initializer or application.rb. - DEPRECATION - end - + def self.enqueue_args(active_job, scheduled_at: nil) execution_args = { + id: active_job.job_id, active_job_id: active_job.job_id, + job_class: active_job.class.name, queue_name: active_job.queue_name.presence || DEFAULT_QUEUE_NAME, priority: active_job.priority || DEFAULT_PRIORITY, serialized_params: active_job.serialize, + created_at: Time.current, } - execution_args[:scheduled_at] = Time.zone.at(active_job.scheduled_at) if active_job.scheduled_at + + execution_args[:scheduled_at] = if scheduled_at + scheduled_at + elsif active_job.scheduled_at + Time.zone.at(active_job.scheduled_at) + else + execution_args[:created_at] + end + execution_args[:concurrency_key] = active_job.good_job_concurrency_key if active_job.respond_to?(:good_job_concurrency_key) - if active_job.respond_to?(:good_job_labels) && active_job.good_job_labels.any? && labels_migrated? + if active_job.respond_to?(:good_job_labels) && active_job.good_job_labels.any? labels = active_job.good_job_labels.dup labels.map! { |label| label.to_s.strip.presence } labels.tap(&:compact!).tap(&:uniq!) execution_args[:labels] = labels end - reenqueued_current_execution = CurrentThread.active_job_id && CurrentThread.active_job_id == active_job.job_id - current_execution = CurrentThread.execution + reenqueued_current_job = CurrentThread.active_job_id && CurrentThread.active_job_id == active_job.job_id + current_job = CurrentThread.job - if reenqueued_current_execution - if GoodJob::BatchRecord.migrated? - execution_args[:batch_id] = current_execution.batch_id - execution_args[:batch_callback_id] = current_execution.batch_callback_id - end - execution_args[:cron_key] = current_execution.cron_key + if reenqueued_current_job + execution_args[:batch_id] = current_job.batch_id + execution_args[:batch_callback_id] = current_job.batch_callback_id + execution_args[:cron_key] = current_job.cron_key else - if GoodJob::BatchRecord.migrated? - execution_args[:batch_id] = GoodJob::Batch.current_batch_id - execution_args[:batch_callback_id] = GoodJob::Batch.current_batch_callback_id - end + execution_args[:batch_id] = GoodJob::Batch.current_batch_id + execution_args[:batch_callback_id] = GoodJob::Batch.current_batch_callback_id execution_args[:cron_key] = CurrentThread.cron_key execution_args[:cron_at] = CurrentThread.cron_at end - execution_args.merge(overrides) + execution_args end # Finds the next eligible Execution, acquire an advisory lock related to it, and @@ -363,21 +300,23 @@ def self.enqueue_args(active_job, overrides = {}) # raised, if any (if the job raised, then the second array entry will be # +nil+). If there were no jobs to execute, returns +nil+. def self.perform_with_advisory_lock(lock_id:, parsed_queues: nil, queue_select_limit: nil) - execution = nil + job = nil result = nil - unfinished.dequeueing_ordered(parsed_queues).only_scheduled.limit(1).with_advisory_lock(select_limit: queue_select_limit) do |executions| - execution = executions.first - if execution&.executable? - yield(execution) if block_given? - result = execution.perform(lock_id: lock_id) + unfinished.dequeueing_ordered(parsed_queues).only_scheduled.limit(1).with_advisory_lock(select_limit: queue_select_limit) do |jobs| + job = jobs.first + + if job&.executable? + yield(job) if block_given? + + result = job.perform(lock_id: lock_id) else - execution = nil + job = nil yield(nil) if block_given? end end - execution&.run_callbacks(:perform_unlocked) + job&.run_callbacks(:perform_unlocked) result end @@ -413,46 +352,38 @@ def self.next_scheduled_at(after: nil, limit: 100, now_limit: nil) # The new {Execution} instance representing the queued ActiveJob job. def self.enqueue(active_job, scheduled_at: nil, create_with_advisory_lock: false) ActiveSupport::Notifications.instrument("enqueue_job.good_job", { active_job: active_job, scheduled_at: scheduled_at, create_with_advisory_lock: create_with_advisory_lock }) do |instrument_payload| - current_execution = CurrentThread.execution + current_job = CurrentThread.job - retried = current_execution && current_execution.active_job_id == active_job.job_id + retried = current_job && current_job.active_job_id == active_job.job_id if retried - if current_execution.discrete? - execution = current_execution - execution.assign_attributes(enqueue_args(active_job, { scheduled_at: scheduled_at })) - execution.scheduled_at ||= Time.current - # TODO: these values ideally shouldn't be persisted until the current_execution is finished - # which will require handling `retry_job` being called from outside the execution context. - execution.performed_at = nil - execution.finished_at = nil - else - execution = build_for_enqueue(active_job, { scheduled_at: scheduled_at }) - end + job = current_job + job.assign_attributes(enqueue_args(active_job, scheduled_at: scheduled_at)) + job.scheduled_at ||= Time.current + # TODO: these values ideally shouldn't be persisted until the current_job is finished + # which will require handling `retry_job` being called from outside the job context. + job.performed_at = nil + job.finished_at = nil else - execution = build_for_enqueue(active_job, { scheduled_at: scheduled_at }) - execution.make_discrete if discrete_support? + job = build_for_enqueue(active_job, scheduled_at: scheduled_at) end if create_with_advisory_lock - if execution.persisted? - execution.advisory_lock + if job.persisted? + job.advisory_lock else - execution.create_with_advisory_lock = true + job.create_with_advisory_lock = true end end - instrument_payload[:execution] = execution - execution.save! + instrument_payload[:job] = job + job.save! - if retried - CurrentThread.execution_retried = execution - CurrentThread.execution.retried_good_job_id = execution.id unless current_execution.discrete? - else - CurrentThread.execution_retried = nil - end + CurrentThread.execution_retried = (job if retried) - active_job.provider_job_id = execution.id - execution + active_job.provider_job_id = job.id + raise "These should be equal" if active_job.provider_job_id != active_job.job_id + + job end end @@ -476,64 +407,47 @@ def perform(lock_id:) discrete_execution = nil result = GoodJob::CurrentThread.within do |current_thread| current_thread.reset - current_thread.execution = self + current_thread.job = self existing_performed_at = performed_at if existing_performed_at current_thread.execution_interrupted = existing_performed_at - if discrete? - interrupt_error_string = self.class.format_error(GoodJob::InterruptError.new("Interrupted after starting perform at '#{existing_performed_at}'")) - self.error = interrupt_error_string - self.error_event = ERROR_EVENT_INTERRUPTED if self.class.error_event_migrated? - monotonic_duration = (::Process.clock_gettime(::Process::CLOCK_MONOTONIC) - monotonic_start).seconds - - discrete_execution_attrs = { - error: interrupt_error_string, - finished_at: job_performed_at, - } - discrete_execution_attrs[:error_event] = GoodJob::ErrorEvents::ERROR_EVENT_ENUMS[GoodJob::ErrorEvents::ERROR_EVENT_INTERRUPTED] if self.class.error_event_migrated? - discrete_execution_attrs[:duration] = monotonic_duration if GoodJob::DiscreteExecution.duration_interval_usable? - discrete_executions.where(finished_at: nil).where.not(performed_at: nil).update_all(discrete_execution_attrs) # rubocop:disable Rails/SkipsModelValidations - end + interrupt_error_string = self.class.format_error(GoodJob::InterruptError.new("Interrupted after starting perform at '#{existing_performed_at}'")) + self.error = interrupt_error_string + self.error_event = ERROR_EVENT_INTERRUPTED + monotonic_duration = (::Process.clock_gettime(::Process::CLOCK_MONOTONIC) - monotonic_start).seconds + + discrete_execution_attrs = { + error: interrupt_error_string, + finished_at: job_performed_at, + } + discrete_execution_attrs[:error_event] = GoodJob::ErrorEvents::ERROR_EVENT_ENUMS[GoodJob::ErrorEvents::ERROR_EVENT_INTERRUPTED] + discrete_execution_attrs[:duration] = monotonic_duration + discrete_executions.where(finished_at: nil).where.not(performed_at: nil).update_all(discrete_execution_attrs) # rubocop:disable Rails/SkipsModelValidations end - if discrete? - transaction do - discrete_execution_attrs = { - job_class: job_class, - queue_name: queue_name, - serialized_params: serialized_params, - scheduled_at: (scheduled_at || created_at), - created_at: job_performed_at, - } - discrete_execution_attrs[:process_id] = lock_id if GoodJob::DiscreteExecution.columns_hash.key?("process_id") - - execution_attrs = { - performed_at: job_performed_at, - executions_count: ((executions_count || 0) + 1), - } - if GoodJob::Execution.columns_hash.key?("locked_by_id") - execution_attrs[:locked_by_id] = lock_id - execution_attrs[:locked_at] = Time.current - end - - discrete_execution = discrete_executions.create!(discrete_execution_attrs) - update!(execution_attrs) - end - else - execution_attrs = { + transaction do + discrete_execution_attrs = { + job_class: job_class, + queue_name: queue_name, + serialized_params: serialized_params, + scheduled_at: (scheduled_at || created_at), + created_at: job_performed_at, + process_id: lock_id, + } + job_attrs = { performed_at: job_performed_at, + executions_count: ((executions_count || 0) + 1), + locked_by_id: lock_id, + locked_at: Time.current, } - if GoodJob::Execution.columns_hash.key?("locked_by_id") - execution_attrs[:locked_by_id] = lock_id - execution_attrs[:locked_at] = Time.current - end - update!(execution_attrs) + discrete_execution = discrete_executions.create!(discrete_execution_attrs) + update!(job_attrs) end - ActiveSupport::Notifications.instrument("perform_job.good_job", { execution: self, process_id: current_thread.process_id, thread_name: current_thread.thread_name }) do |instrument_payload| + ActiveSupport::Notifications.instrument("perform_job.good_job", { job: self, execution: discrete_execution, process_id: current_thread.process_id, thread_name: current_thread.thread_name }) do |instrument_payload| value = ActiveJob::Base.execute(active_job_data) if value.is_a?(Exception) @@ -584,50 +498,39 @@ def perform(lock_id:) error_string = self.class.format_error(job_error) job_attributes[:error] = error_string - job_attributes[:error_event] = result.error_event if self.class.error_event_migrated? - if discrete_execution - discrete_execution.error = error_string - discrete_execution.error_event = result.error_event - discrete_execution.error_backtrace = job_error.backtrace if discrete_execution.class.backtrace_migrated? - end + job_attributes[:error_event] = result.error_event + + discrete_execution.error = error_string + discrete_execution.error_event = result.error_event + discrete_execution.error_backtrace = job_error.backtrace else job_attributes[:error] = nil job_attributes[:error_event] = nil end - job_attributes.delete(:error_event) unless self.class.error_event_migrated? job_finished_at = Time.current monotonic_duration = (::Process.clock_gettime(::Process::CLOCK_MONOTONIC) - monotonic_start).seconds job_attributes[:finished_at] = job_finished_at - if discrete_execution - discrete_execution.finished_at = job_finished_at - discrete_execution.duration = monotonic_duration if GoodJob::DiscreteExecution.duration_interval_usable? - end + + discrete_execution.finished_at = job_finished_at + discrete_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 if reenqueued - if discrete_execution - job_attributes[:performed_at] = nil - job_attributes[:finished_at] = nil - else - job_attributes[:retried_good_job_id] = retried_good_job_id - job_attributes[:finished_at] = nil if retry_unhandled_error - end + job_attributes[:performed_at] = nil + job_attributes[:finished_at] = nil end + assign_attributes(job_attributes) preserve_unhandled = (result.unhandled_error && (GoodJob.retry_on_unhandled_error || GoodJob.preserve_job_records == :on_unhandled_error)) - if GoodJob.preserve_job_records == true || reenqueued || preserve_unhandled || cron_key.present? - if discrete_execution - transaction do - discrete_execution.save! - update!(job_attributes) - end - else - update!(job_attributes) + if finished_at.blank? || GoodJob.preserve_job_records == true || reenqueued || preserve_unhandled || cron_key.present? + transaction do + discrete_execution.save! + save! end else - destroy_job + destroy! end result @@ -708,7 +611,7 @@ def reset_batch_values(&block) end def continue_discard_or_finish_batch - batch._continue_discard_or_finish(self) if GoodJob::BatchRecord.migrated? && batch.present? + batch._continue_discard_or_finish(self) if batch.present? end def active_job_data @@ -716,7 +619,7 @@ def active_job_data .tap do |job_data| job_data["provider_job_id"] = id job_data["good_job_concurrency_key"] = concurrency_key if concurrency_key - job_data["good_job_labels"] = Array(labels) if self.class.labels_migrated? && labels.present? + job_data["good_job_labels"] = Array(labels) if labels.present? end end end diff --git a/app/models/good_job/cron_entry.rb b/app/models/good_job/cron_entry.rb index f8db6bc6c..013026744 100644 --- a/app/models/good_job/cron_entry.rb +++ b/app/models/good_job/cron_entry.rb @@ -72,8 +72,6 @@ def next_at(previously_at: nil) end def enabled? - return true unless GoodJob::Setting.migrated? - GoodJob::Setting.cron_key_enabled?(key, default: enabled_by_default?) end diff --git a/app/models/good_job/discrete_execution.rb b/app/models/good_job/discrete_execution.rb index f9c913bb3..adc54825f 100644 --- a/app/models/good_job/discrete_execution.rb +++ b/app/models/good_job/discrete_execution.rb @@ -14,31 +14,6 @@ class DiscreteExecution < BaseRecord alias_attribute :performed_at, :created_at - def self.error_event_migrated? - return true if columns_hash["error_event"].present? - - migration_pending_warning! - false - end - - def self.backtrace_migrated? - return true if columns_hash["error_backtrace"].present? - - migration_pending_warning! - false - end - - def self.duration_interval_migrated? - return true if columns_hash["duration"].present? - - migration_pending_warning! - false - end - - def self.duration_interval_usable? - duration_interval_migrated? && Gem::Version.new(Rails.version) >= Gem::Version.new('6.1.0.a') - end - def number serialized_params.fetch('executions', 0) + 1 end @@ -50,12 +25,7 @@ def queue_latency # Monotonic time between when this job started and finished def runtime_latency - # migrated and Rails greater than 6.1 - if self.class.duration_interval_usable? - duration - elsif performed_at - (finished_at || Time.current) - performed_at - end + duration end def last_status_at diff --git a/app/models/good_job/execution.rb b/app/models/good_job/execution.rb index 0bc9564b5..aa2142c57 100644 --- a/app/models/good_job/execution.rb +++ b/app/models/good_job/execution.rb @@ -1,12 +1,8 @@ # frozen_string_literal: true module GoodJob - # Active Record model that represents an +ActiveJob+ job. - # Most behavior is currently in BaseExecution in anticipation of - # moving behavior into +GoodJob::Job+. - class Execution < BaseExecution - self.table_name = 'good_jobs' - - belongs_to :job, class_name: 'GoodJob::Job', foreign_key: 'active_job_id', primary_key: 'active_job_id', optional: true, inverse_of: :executions + # Created at the time a Job begins executing. + # Behavior from +DiscreteExecution+ will be merged into this class. + class Execution < DiscreteExecution end end diff --git a/app/models/good_job/job.rb b/app/models/good_job/job.rb index b8a0325e9..f34788699 100644 --- a/app/models/good_job/job.rb +++ b/app/models/good_job/job.rb @@ -2,10 +2,6 @@ module GoodJob # Active Record model that represents an +ActiveJob+ job. - # There is not a table in the database whose discrete rows represents "Jobs". - # The +good_jobs+ table is a table of individual {GoodJob::Execution}s that share the same +active_job_id+. - # A single row from the +good_jobs+ table of executions is fetched to represent a Job. - # class Job < BaseExecution # Raised when an inappropriate action is applied to a Job based on its state. ActionForStateMismatchError = Class.new(StandardError) @@ -16,29 +12,17 @@ class Job < BaseExecution # Raised when Active Job data cannot be deserialized ActiveJobDeserializationError = Class.new(StandardError) - class << self - delegate :table_name, to: GoodJob::Execution - - def table_name=(_value) - raise NotImplementedError, 'Assign GoodJob::Execution.table_name directly' - end - end - - self.primary_key = 'active_job_id' - self.advisory_lockable_column = 'active_job_id' + self.table_name = 'good_jobs' + self.advisory_lockable_column = 'id' self.implicit_order_column = 'created_at' belongs_to :batch, class_name: 'GoodJob::BatchRecord', inverse_of: :jobs, optional: true belongs_to :locked_by_process, class_name: "GoodJob::Process", foreign_key: :locked_by_id, inverse_of: :locked_jobs, optional: true - has_many :executions, -> { order(created_at: :asc) }, class_name: 'GoodJob::Execution', foreign_key: 'active_job_id', inverse_of: :job # rubocop:disable Rails/HasManyOrHasOneDependent - has_many :discrete_executions, -> { order(created_at: :asc) }, class_name: 'GoodJob::DiscreteExecution', foreign_key: 'active_job_id', primary_key: :active_job_id, inverse_of: :job # rubocop:disable Rails/HasManyOrHasOneDependent + has_many :executions, -> { order(created_at: :asc) }, class_name: 'GoodJob::Execution', foreign_key: 'active_job_id', primary_key: :active_job_id, inverse_of: :job, dependent: :delete_all + # TODO: rename callers of discrete_execution to executions, but after v4 has some time to bake for cleaner diffs/patches + has_many :discrete_executions, -> { order(created_at: :asc) }, class_name: 'GoodJob::Execution', foreign_key: 'active_job_id', primary_key: :active_job_id, inverse_of: :job, dependent: :delete_all - after_destroy lambda { - GoodJob::DiscreteExecution.where(active_job_id: active_job_id).delete_all if discrete? # TODO: move into association `dependent: :delete_all` after v4 - } - - # Only the most-recent unretried execution represents a "Job" - default_scope { where(retried_good_job_id: nil) } + before_create -> { self.id = active_job_id }, if: -> { active_job_id.present? } # Get Jobs finished before the given timestamp. # @!method finished_before(timestamp) @@ -64,65 +48,11 @@ def table_name=(_value) scope :unfinished_undiscrete, -> { where(finished_at: nil, retried_good_job_id: nil, is_discrete: [nil, false]) } - # The job's Active Job UUID - # @return [String] - def id - active_job_id - end - - # Override #reload to add a custom scope to ensure the reloaded record is the head execution - # @return [Job] - def reload(options = nil) - self.class.connection.clear_query_cache - - # override with the `where(retried_good_job_id: nil)` scope - override_query = self.class.where(retried_good_job_id: nil) - fresh_object = - if options && options[:lock] - self.class.unscoped { override_query.lock(options[:lock]).find(id) } - else - self.class.unscoped { override_query.find(id) } - end - - @attributes = fresh_object.instance_variable_get(:@attributes) - @new_record = false - @previously_new_record = false - self - end - - # This job's most recent {Execution} - # @param reload [Booelan] whether to reload executions - # @return [Execution] - def head_execution(reload: false) - executions.reload if reload - executions.load # memoize the results - executions.last - end - - # This job's initial/oldest {Execution} - # @return [Execution] - def tail_execution - executions.first - end - - # The number of times this job has been executed, according to Active Job's serialized state. - # @return [Numeric] - def executions_count - aj_count = serialized_params.fetch('executions', 0) - # The execution count within serialized_params is not updated - # once the underlying execution has been executed. - if status.in? [:discarded, :succeeded, :running] - aj_count + 1 - else - aj_count - end - end - - # The number of times this job has been executed, according to the number of GoodJob {Execution} records. - # @return [Numeric] - def preserved_executions_count - executions.size - end + # TODO: it would be nice to enforce these values at the model + # validates :active_job_id, presence: true + # validates :scheduled_at, presence: true + # validates :job_class, presence: true + # validates :error_event, presence: true, if: -> { error.present? } # The most recent error message. # If the job has been retried, the error will be fetched from the previous {Execution} record. @@ -155,6 +85,10 @@ def display_name job_class end + def executions_count + super || 0 + end + # Tests whether the job is being executed right now. # @return [Boolean] def running? @@ -189,33 +123,33 @@ def succeeded? # @return [ActiveJob::Base] def retry_job with_advisory_lock do - execution = head_execution(reload: true) - active_job = execution.active_job(ignore_deserialization_errors: true) + reload + active_job = self.active_job(ignore_deserialization_errors: true) raise ActiveJobDeserializationError if active_job.nil? raise AdapterNotGoodJobError unless active_job.class.queue_adapter.is_a? GoodJob::Adapter - raise ActionForStateMismatchError if execution.finished_at.blank? || execution.error.blank? + raise ActionForStateMismatchError if finished_at.blank? || error.blank? # Update the executions count because the previous execution will not have been preserved # Do not update `exception_executions` because that comes from rescue_from's arguments active_job.executions = (active_job.executions || 0) + 1 begin - error_class, error_message = execution.error.split(ERROR_MESSAGE_SEPARATOR).map(&:strip) + error_class, error_message = error.split(ERROR_MESSAGE_SEPARATOR).map(&:strip) error = error_class.constantize.new(error_message) rescue StandardError - error = StandardError.new(execution.error) + error = StandardError.new(error) end new_active_job = nil GoodJob::CurrentThread.within do |current_thread| - current_thread.execution = execution + current_thread.job = self current_thread.retry_now = true - execution.class.transaction(joinable: false, requires_new: true) do + self.class.transaction(joinable: false, requires_new: true) do new_active_job = active_job.retry_job(wait: 0, error: error) - execution.error_event = ERROR_EVENT_RETRIED if execution.error && execution.class.error_event_migrated? - execution.save! + self.error_event = ERROR_EVENT_RETRIED if error + save! end end @@ -244,11 +178,10 @@ def force_discard_job(message) # @return [void] def reschedule_job(scheduled_at = Time.current) with_advisory_lock do - execution = head_execution(reload: true) - - raise ActionForStateMismatchError if execution.finished_at.present? + reload + raise ActionForStateMismatchError if finished_at.present? - execution.update(scheduled_at: scheduled_at) + update(scheduled_at: scheduled_at) end end @@ -256,9 +189,7 @@ def reschedule_job(scheduled_at = Time.current) # @return [void] def destroy_job with_advisory_lock do - execution = head_execution(reload: true) - - raise ActionForStateMismatchError if execution.finished_at.blank? + raise ActionForStateMismatchError if finished_at.blank? destroy end @@ -270,37 +201,27 @@ def _execution_id attributes['id'] end - # Utility method to test whether this job's underlying attributes represents its most recent execution. - # @return [Boolean] - def _head? - _execution_id == head_execution(reload: true).id - end - private def _discard_job(message) - execution = head_execution(reload: true) - active_job = execution.active_job(ignore_deserialization_errors: true) + active_job = self.active_job(ignore_deserialization_errors: true) - raise ActionForStateMismatchError if execution.finished_at.present? + raise ActionForStateMismatchError if finished_at.present? job_error = GoodJob::Job::DiscardJobError.new(message) - update_execution = proc do - execution.update( - { - finished_at: Time.current, - error: GoodJob::Execution.format_error(job_error), - }.tap do |attrs| - attrs[:error_event] = ERROR_EVENT_DISCARDED if self.class.error_event_migrated? - end + update_record = proc do + update( + finished_at: Time.current, + error: self.class.format_error(job_error), + error_event: ERROR_EVENT_DISCARDED ) end if active_job.respond_to?(:instrument) - active_job.send :instrument, :discard, error: job_error, &update_execution + active_job.send :instrument, :discard, error: job_error, &update_record else - update_execution.call + update_record.call end end end diff --git a/app/models/good_job/process.rb b/app/models/good_job/process.rb index a158dde1b..5340a9ce1 100644 --- a/app/models/good_job/process.rb +++ b/app/models/good_job/process.rb @@ -33,13 +33,9 @@ class Process < BaseRecord # @!scope class # @return [ActiveRecord::Relation] scope :active, (lambda do - if lock_type_migrated? - query = joins_advisory_locks - query.where(lock_type: LOCK_TYPE_ENUMS[LOCK_TYPE_ADVISORY]).advisory_locked - .or(query.where(lock_type: nil).where(arel_table[:updated_at].gt(EXPIRED_INTERVAL.ago))) - else - advisory_locked - end + query = joins_advisory_locks + query.where(lock_type: LOCK_TYPE_ENUMS[LOCK_TYPE_ADVISORY]).advisory_locked + .or(query.where(lock_type: nil).where(arel_table[:updated_at].gt(EXPIRED_INTERVAL.ago))) end) # Processes that are inactive and unlocked (e.g. SIGKILLed) @@ -47,13 +43,9 @@ class Process < BaseRecord # @!scope class # @return [ActiveRecord::Relation] scope :inactive, (lambda do - if lock_type_migrated? - query = joins_advisory_locks - query.where(lock_type: LOCK_TYPE_ENUMS[LOCK_TYPE_ADVISORY]).advisory_unlocked - .or(query.where(lock_type: nil).where(arel_table[:updated_at].lt(EXPIRED_INTERVAL.ago))) - else - advisory_unlocked - end + query = joins_advisory_locks + query.where(lock_type: LOCK_TYPE_ENUMS[LOCK_TYPE_ADVISORY]).advisory_unlocked + .or(query.where(lock_type: nil).where(arel_table[:updated_at].lt(EXPIRED_INTERVAL.ago))) end) # Deletes all inactive process records. @@ -64,11 +56,6 @@ def self.cleanup end end - # @return [Boolean] - def self.lock_type_migrated? - columns_hash["lock_type"].present? - end - def self.create_record(id:, with_advisory_lock: false) attributes = { id: id, @@ -76,7 +63,7 @@ def self.create_record(id:, with_advisory_lock: false) } if with_advisory_lock attributes[:create_with_advisory_lock] = true - attributes[:lock_type] = LOCK_TYPE_ADVISORY if lock_type_migrated? + attributes[:lock_type] = LOCK_TYPE_ADVISORY end create!(attributes) end diff --git a/app/views/good_job/batches/index.html.erb b/app/views/good_job/batches/index.html.erb index 390ec9cbf..bf0f57867 100644 --- a/app/views/good_job/batches/index.html.erb +++ b/app/views/good_job/batches/index.html.erb @@ -2,19 +2,15 @@

<%= t ".title" %>

-<% if GoodJob::BatchRecord.migrated? %> - <%= render 'good_job/batches/table', batches: @filter.records, filter: @filter %> - <% if @filter.records.present? %> - - <% end %> -<% else %> -

<%= t ".pending_migrations" %>

+<%= render 'good_job/batches/table', batches: @filter.records, filter: @filter %> +<% if @filter.records.present? %> + <% end %> diff --git a/app/views/good_job/jobs/_executions.erb b/app/views/good_job/jobs/_executions.erb index 5279dd428..14a26b1c5 100644 --- a/app/views/good_job/jobs/_executions.erb +++ b/app/views/good_job/jobs/_executions.erb @@ -38,7 +38,7 @@ <%= t "good_job.shared.error" %>: <%= execution.error %> - <% if GoodJob::DiscreteExecution.backtrace_migrated? && execution.error_backtrace&.any? %> + <% if execution.error_backtrace&.any? %> <%= tag.ul class: "nav nav-tabs small w-fit-content", id: dom_id(execution, :tab), role: "tablist" do %> @@ -87,8 +84,4 @@ <%= tag.pre JSON.pretty_generate(@job.display_serialized_params) %> <% end %> -<% if @job.discrete? %> - <%= render 'executions', executions: @job.discrete_executions.reverse %> -<% else %> - <%= render 'executions', executions: @job.executions.includes_advisory_locks.reverse %> -<% end %> +<%= render 'executions', executions: @job.executions.reverse %> diff --git a/app/views/good_job/performances/show.html.erb b/app/views/good_job/performances/show.html.erb index 31f7b553d..3b8ee3424 100644 --- a/app/views/good_job/performances/show.html.erb +++ b/app/views/good_job/performances/show.html.erb @@ -2,49 +2,42 @@

<%= t ".title" %>

-<% if @needs_upgrade %> -
- <%= t "shared.needs_migration" %> -
-<% else %> - -
-
-
-
-
<%= t ".job_class" %>
-
<%= t ".executions" %>
+
+
+
+
+
<%= t ".job_class" %>
+
<%= t ".executions" %>
-
<%= t ".average_duration" %>
-
<%= t ".minimum_duration" %>
-
<%= t ".maximum_duration" %>
-
-
+
<%= t ".average_duration" %>
+
<%= t ".minimum_duration" %>
+
<%= t ".maximum_duration" %>
+
+
- <% @performances.each do |performance| %> -
-
-
<%= performance.job_class %>
-
-
<%= t ".executions" %>
- <%= performance.executions_count %> -
+ <% @performances.each do |performance| %> +
+
+
<%= performance.job_class %>
+
+
<%= t ".executions" %>
+ <%= performance.executions_count %> +
-
-
<%= t ".average_duration" %>
- <%= format_duration performance.avg_duration %> -
-
-
<%= t ".minimum_duration" %>
- <%= format_duration performance.min_duration %> -
-
-
<%= t ".maximum_duration" %>
- <%= format_duration performance.max_duration %> -
+
+
<%= t ".average_duration" %>
+ <%= format_duration performance.avg_duration %> +
+
+
<%= t ".minimum_duration" %>
+ <%= format_duration performance.min_duration %> +
+
+
<%= t ".maximum_duration" %>
+ <%= format_duration performance.max_duration %>
- <% end %> -
+
+ <% end %>
-<% end %> +
diff --git a/config/locales/de.yml b/config/locales/de.yml index 80669f7d4..693bdd610 100644 --- a/config/locales/de.yml +++ b/config/locales/de.yml @@ -11,7 +11,6 @@ de: batches: index: older_batches: Ältere Chargen - pending_migrations: GoodJob hat ausstehende Datenbankmigrationen. title: Chargen jobs: actions: @@ -48,7 +47,6 @@ de: index: no_cron_schedules_found: Keine Cron-Zeitpläne gefunden. title: Cron-Zeitpläne - pending_migrations: Erfordert ausstehende GoodJob-Datenbankmigration. show: cron_entry_key: Cron-Eingabetaste datetime: @@ -240,6 +238,7 @@ de: dark: Dunkel light: Licht theme: Thema + pending_migrations: GoodJob hat ausstehende Datenbankmigrationen. secondary_navbar: inspiration: Denk daran, auch du machst einen guten Job! last_updated: Zuletzt aktualisiert @@ -250,5 +249,3 @@ de: running: Laufend scheduled: Geplant succeeded: Erfolgreich - shared: - needs_migration: Bitte führen Sie GoodJob-Migrationen aus. diff --git a/config/locales/en.yml b/config/locales/en.yml index b3666352c..84c7b3d56 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -11,7 +11,6 @@ en: batches: index: older_batches: Older batches - pending_migrations: GoodJob has pending database migrations. title: Batches jobs: actions: @@ -48,7 +47,6 @@ en: index: no_cron_schedules_found: No cron schedules found. title: Cron Schedules - pending_migrations: Requires pending GoodJob database migration. show: cron_entry_key: Cron Entry Key datetime: @@ -240,6 +238,7 @@ en: dark: Dark light: Light theme: Theme + pending_migrations: GoodJob has pending database migrations. secondary_navbar: inspiration: Remember, you're doing a Good Job too! last_updated: Last updated @@ -250,5 +249,3 @@ en: running: Running scheduled: Scheduled succeeded: Succeeded - shared: - needs_migration: Please run GoodJob migrations. diff --git a/config/locales/es.yml b/config/locales/es.yml index a295da32e..e269b752b 100644 --- a/config/locales/es.yml +++ b/config/locales/es.yml @@ -11,7 +11,6 @@ es: batches: index: older_batches: Batches anteriores - pending_migrations: GoodJob tiene migraciones pendientes. title: Batches jobs: actions: @@ -48,7 +47,6 @@ es: index: no_cron_schedules_found: No hay tareas programadas. title: Tareas Programadas - pending_migrations: Require las migraciones de GoodJob pendientes. show: cron_entry_key: Cron Entry Key datetime: @@ -240,6 +238,7 @@ es: dark: Oscuro light: Luz theme: Tema + pending_migrations: GoodJob tiene migraciones pendientes. secondary_navbar: inspiration: "¡Recuerda, también tú estás haciendo un buen trabajo!" last_updated: Última actualización @@ -250,5 +249,3 @@ es: running: Ejecutando scheduled: Programado succeeded: Exitoso - shared: - needs_migration: Ejecute las migraciones de GoodJob. diff --git a/config/locales/fr.yml b/config/locales/fr.yml index e932847af..657eb45f3 100644 --- a/config/locales/fr.yml +++ b/config/locales/fr.yml @@ -11,7 +11,6 @@ fr: batches: index: older_batches: Lots plus anciens - pending_migrations: GoodJob a des migrations de bases de données en attente. title: Lots jobs: actions: @@ -48,7 +47,6 @@ fr: index: no_cron_schedules_found: Aucune planification cron trouvée. title: Entrées Cron - pending_migrations: Nécessite une migration de la base de données GoodJob en attente. show: cron_entry_key: Clé d'entrée Cron datetime: @@ -240,6 +238,7 @@ fr: dark: Sombre light: Lumière theme: Thème + pending_migrations: GoodJob a des migrations de bases de données en attente. secondary_navbar: inspiration: N'oublie pas, toi aussi tu fais du bon boulot ! last_updated: Dernière mise à jour @@ -250,5 +249,3 @@ fr: running: En cours scheduled: Planifiés succeeded: Réussis - shared: - needs_migration: Veuillez exécuter des migrations GoodJob. diff --git a/config/locales/it.yml b/config/locales/it.yml index d93cbe7ff..e49845416 100644 --- a/config/locales/it.yml +++ b/config/locales/it.yml @@ -11,7 +11,6 @@ it: batches: index: older_batches: Batch più vecchi - pending_migrations: GoodJob ha migrazioni del database in sospeso. title: Batch jobs: actions: @@ -48,7 +47,6 @@ it: index: no_cron_schedules_found: Nessuna pianificazione cron trovata. title: Pianificazioni cron - pending_migrations: Richiede migrazione del database GoodJob in sospeso. show: cron_entry_key: Chiave voce cron datetime: @@ -240,6 +238,7 @@ it: dark: Scuro light: Chiaro theme: Tema + pending_migrations: GoodJob ha migrazioni del database in sospeso. secondary_navbar: inspiration: Ricorda, stai facendo anche tu un Buon Lavoro! last_updated: Ultimo aggiornamento @@ -250,5 +249,3 @@ it: running: In esecuzione scheduled: Pianificato succeeded: Riuscito - shared: - needs_migration: Esegui le migrazioni GoodJob. diff --git a/config/locales/ja.yml b/config/locales/ja.yml index 47a2189cb..2992af9d3 100644 --- a/config/locales/ja.yml +++ b/config/locales/ja.yml @@ -11,7 +11,6 @@ ja: batches: index: older_batches: より古いバッチ - pending_migrations: GoodJobに保留中のデータベースマイグレーションがあります。 title: バッチ jobs: actions: @@ -48,7 +47,6 @@ ja: index: no_cron_schedules_found: cronスケジュールが見つかりませんでした。 title: cronスケジュール - pending_migrations: GoodJobの保留中のデータベースマイグレーションが必要です。 show: cron_entry_key: cronエントリキー datetime: @@ -240,6 +238,7 @@ ja: dark: 暗い light: ライト theme: テーマ + pending_migrations: GoodJobに保留中のデータベースマイグレーションがあります。 secondary_navbar: inspiration: 覚えておいてください、あなたも良い仕事をしています! last_updated: 最終更新 @@ -250,5 +249,3 @@ ja: running: 実行中 scheduled: スケジュール待ち succeeded: 成功済み - shared: - needs_migration: GoodJob 移行を実行してください。 diff --git a/config/locales/ko.yml b/config/locales/ko.yml index 9a1d28049..43caa2623 100644 --- a/config/locales/ko.yml +++ b/config/locales/ko.yml @@ -11,7 +11,6 @@ ko: batches: index: older_batches: 더 오래된 배치 - pending_migrations: GoodJob에 보류 중인 데이터베이스 마이그레이션 작업이 있습니다. title: 배치 jobs: actions: @@ -48,7 +47,6 @@ ko: index: no_cron_schedules_found: cron 스케줄을 찾을 수 없습니다. title: cron 스케줄 - pending_migrations: 보류 중인 GoodJob 데이터베이스 마이그레이션이 필요합니다. show: cron_entry_key: cron 엔트리 키 datetime: @@ -240,6 +238,7 @@ ko: dark: 어두운 light: 밝은 theme: 테마 + pending_migrations: GoodJob에 보류 중인 데이터베이스 마이그레이션 작업이 있습니다. secondary_navbar: inspiration: 기억하세요, 당신도 좋은 일을 하고 있습니다! last_updated: 최종 업데이트 @@ -250,5 +249,3 @@ ko: running: 실행 중 scheduled: 예정됨 succeeded: 성공함 - shared: - needs_migration: GoodJob 마이그레이션을 실행하세요. diff --git a/config/locales/nl.yml b/config/locales/nl.yml index 39d72fc76..42104bce9 100644 --- a/config/locales/nl.yml +++ b/config/locales/nl.yml @@ -11,7 +11,6 @@ nl: batches: index: older_batches: Oudere partijen - pending_migrations: GoodJob heeft databasemigraties in behandeling. title: Partijen jobs: actions: @@ -48,7 +47,6 @@ nl: index: no_cron_schedules_found: Geen cron-schema's gevonden. title: Cron-schema's - pending_migrations: Vereist GoodJob-databasemigratie in afwachting. show: cron_entry_key: Cron-invoersleutel datetime: @@ -240,6 +238,7 @@ nl: dark: Donker light: Licht theme: Thema + pending_migrations: GoodJob heeft databasemigraties in behandeling. secondary_navbar: inspiration: 'Onthoud: jij levert ook goed werk!' last_updated: Laatst bijgewerkt @@ -250,5 +249,3 @@ nl: running: Rennen scheduled: Gepland succeeded: Geslaagd - shared: - needs_migration: Voer GoodJob-migraties uit. diff --git a/config/locales/pt-BR.yml b/config/locales/pt-BR.yml index a08c997b1..105eee616 100644 --- a/config/locales/pt-BR.yml +++ b/config/locales/pt-BR.yml @@ -11,7 +11,6 @@ pt-BR: batches: index: older_batches: Lotes antigos - pending_migrations: O GoodJob tem migrações pendentes no banco de dados. title: Lotes jobs: actions: @@ -48,7 +47,6 @@ pt-BR: index: no_cron_schedules_found: Nenhuma tarefa programada encontrada. title: Tarefas Programadas - pending_migrations: Requer migração pendente do GoodJob no banco de dados. show: cron_entry_key: Chave da Tarefa Programada datetime: @@ -240,6 +238,7 @@ pt-BR: dark: Escuro light: Claro theme: Tema + pending_migrations: O GoodJob tem migrações pendentes no banco de dados. secondary_navbar: inspiration: Lembre-se, você também está fazendo uma Boa Tarefa! last_updated: Última atualização @@ -250,5 +249,3 @@ pt-BR: running: Em execução scheduled: Agendado succeeded: Concluído com sucesso - shared: - needs_migration: Execute migrações GoodJob. diff --git a/config/locales/ru.yml b/config/locales/ru.yml index c4fc173f0..d7a0d2a24 100644 --- a/config/locales/ru.yml +++ b/config/locales/ru.yml @@ -11,7 +11,6 @@ ru: batches: index: older_batches: Старые группы заданий - pending_migrations: У GoodJob есть ожидающие миграции базы данных. title: Группы заданий jobs: actions: @@ -48,7 +47,6 @@ ru: index: no_cron_schedules_found: Расписания cron не найдены. title: Расписания cron - pending_migrations: У GoodJob есть ожидающие миграции базы данных. show: cron_entry_key: Ключ cron-задания datetime: @@ -266,6 +264,7 @@ ru: dark: Темный light: Светлая theme: Тема офрмдления + pending_migrations: У GoodJob есть ожидающие миграции базы данных. secondary_navbar: inspiration: Благодаря вам Good Job становится лучше! Спасибо! last_updated: Последнее обновление @@ -276,5 +275,3 @@ ru: running: Исполняется scheduled: Запланировано succeeded: Успешно - shared: - needs_migration: Пожалуйста, запустите миграцию GoodJob. diff --git a/config/locales/tr.yml b/config/locales/tr.yml index 29497591f..2c92e8157 100644 --- a/config/locales/tr.yml +++ b/config/locales/tr.yml @@ -11,7 +11,6 @@ tr: batches: index: older_batches: Daha eski toplu işlemler - pending_migrations: GoodJob'ın bekleyen veritabanı güncellemeleri bulunuyor. title: Toplu İşlemler jobs: actions: @@ -48,7 +47,6 @@ tr: index: no_cron_schedules_found: Planlanmış Cron bulunamadı. title: Cron Planları - pending_migrations: Bekleyen GoodJob veritabanı güncellemesi var. show: cron_entry_key: Cron Giriş Anahtarı datetime: @@ -240,6 +238,7 @@ tr: dark: Karanlık light: Işık theme: Tema + pending_migrations: GoodJob'ın bekleyen veritabanı güncellemeleri bulunuyor. secondary_navbar: inspiration: Unutmayın, siz de iyi bir iş yapıyorsunuz! last_updated: Son güncelleme @@ -250,5 +249,3 @@ tr: running: Çalışıyor scheduled: Planlandı succeeded: Başarılı - shared: - needs_migration: Lütfen GoodJob geçişlerini çalıştırın. diff --git a/config/locales/uk.yml b/config/locales/uk.yml index ac0b22f95..3c94ef231 100644 --- a/config/locales/uk.yml +++ b/config/locales/uk.yml @@ -11,7 +11,6 @@ uk: batches: index: older_batches: Старі пакети - pending_migrations: GoodJob має невиконані міграції бази даних. title: Пакети jobs: actions: @@ -48,7 +47,6 @@ uk: index: no_cron_schedules_found: Cron-розклади не знайдені. title: Cron-розклади - pending_migrations: Вимагає невиконаних міграцій бази даних GoodJob. show: cron_entry_key: Ключ cron-запису datetime: @@ -266,6 +264,7 @@ uk: dark: Темний light: світло theme: Тема + pending_migrations: GoodJob має невиконані міграції бази даних. secondary_navbar: inspiration: Пам'ятайте, ви теж робите хорошу роботу! last_updated: Останнє оновлення @@ -276,5 +275,3 @@ uk: running: Виконується scheduled: Заплановано succeeded: Успішно виконано - shared: - needs_migration: Запустіть міграції GoodJob. diff --git a/demo/db/seeds.rb b/demo/db/seeds.rb index db6fea939..b2526f626 100644 --- a/demo/db/seeds.rb +++ b/demo/db/seeds.rb @@ -44,8 +44,8 @@ break if start_date > Time.current end -GoodJob::Execution.insert_all(jobs_data) -puts "Inserted #{jobs_data.size} job records for a total of #{GoodJob::Execution.count} job records." +GoodJob::Job.insert_all(jobs_data) +puts "Inserted #{jobs_data.size} job records for a total of #{GoodJob::Job.count} job records." puts ActiveJob::Base.queue_adapter diff --git a/gemfiles/rails_6.0.gemfile b/gemfiles/rails_6.0.gemfile deleted file mode 100644 index 40a2d3569..000000000 --- a/gemfiles/rails_6.0.gemfile +++ /dev/null @@ -1,53 +0,0 @@ -# This file was generated by Appraisal - -source "https://rubygems.org" - -gem "activerecord-jdbcpostgresql-adapter", platforms: [:jruby] -gem "appraisal" -gem "matrix" -gem "nokogiri" -gem "pg", platforms: [:mri, :mingw, :x64_mingw] -gem "rack", "~> 2.2" -gem "rails", "~> 6.0.0" -gem "traces", "~> 0.9.1" -gem "puma", "~> 5.6" - -platforms :ruby do - gem "bootsnap" - gem "dotenv-rails" - gem "foreman" - gem "gem-release" - gem "github_changelog_generator", require: false - gem "net-imap", require: false - gem "net-pop", require: false - gem "net-smtp", require: false - - group :debug do - gem "activerecord-explain-analyze", require: false - gem "pry-byebug" - gem "rack-mini-profiler" - gem "rbtrace" - gem "stackprof" - end - - group :lint do - gem "easy_translate" - gem "erb_lint" - gem "i18n-tasks" - gem "mdl" - gem "rubocop" - gem "rubocop-performance" - gem "rubocop-rails" - gem "rubocop-rspec" - gem "sorbet" - gem "sorbet-runtime" - gem "spoom", require: false - gem "tapioca", require: false - end - - group :demo, :production do - gem "skylight" - end -end - -gemspec path: "../" diff --git a/good_job.gemspec b/good_job.gemspec index 4bbd571cb..fa68188fb 100644 --- a/good_job.gemspec +++ b/good_job.gemspec @@ -42,14 +42,14 @@ Gem::Specification.new do |spec| "--quiet" ] - spec.required_ruby_version = ">= 2.6.0" + spec.required_ruby_version = ">= 3.0.0" - spec.add_runtime_dependency "activejob", ">= 6.0.0" - spec.add_runtime_dependency "activerecord", ">= 6.0.0" - spec.add_runtime_dependency "concurrent-ruby", ">= 1.0.2" - spec.add_runtime_dependency "fugit", ">= 1.1" - spec.add_runtime_dependency "railties", ">= 6.0.0" - spec.add_runtime_dependency "thor", ">= 0.14.1" + spec.add_runtime_dependency "activejob", ">= 6.1.0" + spec.add_runtime_dependency "activerecord", ">= 6.1.0" + spec.add_runtime_dependency "concurrent-ruby", ">= 1.3.1" + spec.add_runtime_dependency "fugit", ">= 1.11.0" + spec.add_runtime_dependency "railties", ">= 6.1.0" + spec.add_runtime_dependency "thor", ">= 1.0.0" spec.add_development_dependency "capybara" spec.add_development_dependency "kramdown" diff --git a/i18n-tasks.yml b/i18n-tasks.yml index 3f9797ab5..f6a4cd4cf 100644 --- a/i18n-tasks.yml +++ b/i18n-tasks.yml @@ -99,6 +99,7 @@ ignore_unused: - "good_job.error_event.*" - "good_job.number.*" - "good_job.shared.boolean.*" + - "good_job.shared.pending_migrations" ## Exclude these keys from the `i18n-tasks eq-base' report: # ignore_eq_base: diff --git a/lib/generators/good_job/templates/update/migrations/01_create_good_jobs.rb.erb b/lib/generators/good_job/templates/update/migrations/01_create_good_jobs.rb.erb index d7788af77..ab5da0a01 100644 --- a/lib/generators/good_job/templates/update/migrations/01_create_good_jobs.rb.erb +++ b/lib/generators/good_job/templates/update/migrations/01_create_good_jobs.rb.erb @@ -21,20 +21,82 @@ class CreateGoodJobs < ActiveRecord::Migration<%= migration_version %> t.text :cron_key t.uuid :retried_good_job_id t.datetime :cron_at + + t.uuid :batch_id + t.uuid :batch_callback_id + + t.boolean :is_discrete + t.integer :executions_count + t.text :job_class + t.integer :error_event, limit: 2 + t.text :labels, array: true + t.uuid :locked_by_id + t.datetime :locked_at + end + + create_table :good_job_batches, id: :uuid do |t| + t.timestamps + t.text :description + t.jsonb :serialized_properties + t.text :on_finish + t.text :on_success + t.text :on_discard + t.text :callback_queue_name + t.integer :callback_priority + t.datetime :enqueued_at + t.datetime :discarded_at + t.datetime :finished_at + end + + create_table :good_job_executions, id: :uuid do |t| + t.timestamps + + t.uuid :active_job_id, null: false + t.text :job_class + t.text :queue_name + t.jsonb :serialized_params + t.datetime :scheduled_at + t.datetime :finished_at + t.text :error + t.integer :error_event, limit: 2 + t.text :error_backtrace, array: true + t.uuid :process_id + t.interval :duration end create_table :good_job_processes, id: :uuid do |t| t.timestamps t.jsonb :state + t.integer :lock_type, limit: 2 + end + + create_table :good_job_settings, id: :uuid do |t| + t.timestamps + t.text :key + t.jsonb :value + t.index :key, unique: true end add_index :good_jobs, :scheduled_at, where: "(finished_at IS NULL)", name: :index_good_jobs_on_scheduled_at add_index :good_jobs, [:queue_name, :scheduled_at], where: "(finished_at IS NULL)", name: :index_good_jobs_on_queue_name_and_scheduled_at add_index :good_jobs, [:active_job_id, :created_at], name: :index_good_jobs_on_active_job_id_and_created_at add_index :good_jobs, :concurrency_key, where: "(finished_at IS NULL)", name: :index_good_jobs_on_concurrency_key_when_unfinished - add_index :good_jobs, [:cron_key, :created_at], name: :index_good_jobs_on_cron_key_and_created_at - add_index :good_jobs, [:cron_key, :cron_at], name: :index_good_jobs_on_cron_key_and_cron_at, unique: true - add_index :good_jobs, [:active_job_id], name: :index_good_jobs_on_active_job_id + add_index :good_jobs, [:cron_key, :created_at], where: "(cron_key IS NOT NULL)", name: :index_good_jobs_on_cron_key_and_created_at_cond + add_index :good_jobs, [:cron_key, :cron_at], where: "(cron_key IS NOT NULL)", unique: true, name: :index_good_jobs_on_cron_key_and_cron_at_cond add_index :good_jobs, [:finished_at], where: "retried_good_job_id IS NULL AND finished_at IS NOT NULL", name: :index_good_jobs_jobs_on_finished_at + add_index :good_jobs, [:priority, :created_at], order: { priority: "DESC NULLS LAST", created_at: :asc }, + where: "finished_at IS NULL", name: :index_good_jobs_jobs_on_priority_created_at_when_unfinished + add_index :good_jobs, [:priority, :created_at], order: { priority: "ASC NULLS LAST", created_at: :asc }, + where: "finished_at IS NULL", name: :index_good_job_jobs_for_candidate_lookup + add_index :good_jobs, [:batch_id], where: "batch_id IS NOT NULL" + add_index :good_jobs, [:batch_callback_id], where: "batch_callback_id IS NOT NULL" + add_index :good_jobs, :labels, using: :gin, where: "(labels IS NOT NULL)", name: :index_good_jobs_on_labels + + add_index :good_job_executions, [:active_job_id, :created_at], name: :index_good_job_executions_on_active_job_id_and_created_at + add_index :good_jobs, [:priority, :scheduled_at], order: { priority: "ASC NULLS LAST", scheduled_at: :asc }, + where: "finished_at IS NULL AND locked_by_id IS NULL", name: :index_good_jobs_on_priority_scheduled_at_unfinished_unlocked + add_index :good_jobs, :locked_by_id, + where: "locked_by_id IS NOT NULL", name: "index_good_jobs_on_locked_by_id" + add_index :good_job_executions, [:process_id, :created_at], name: :index_good_job_executions_on_process_id_and_created_at end end diff --git a/lib/generators/good_job/templates/update/migrations/02_create_good_job_settings.rb.erb b/lib/generators/good_job/templates/update/migrations/02_create_good_job_settings.rb.erb deleted file mode 100644 index 2988c42f1..000000000 --- a/lib/generators/good_job/templates/update/migrations/02_create_good_job_settings.rb.erb +++ /dev/null @@ -1,20 +0,0 @@ -# frozen_string_literal: true - -class CreateGoodJobSettings < ActiveRecord::Migration<%= migration_version %> - def change - reversible do |dir| - dir.up do - # Ensure this incremental update migration is idempotent - # with monolithic install migration. - return if connection.table_exists?(:good_job_settings) - end - end - - create_table :good_job_settings, id: :uuid do |t| - t.timestamps - t.text :key - t.jsonb :value - t.index :key, unique: true - end - end -end diff --git a/lib/generators/good_job/templates/update/migrations/03_create_index_good_jobs_jobs_on_priority_created_at_when_unfinished.rb.erb b/lib/generators/good_job/templates/update/migrations/03_create_index_good_jobs_jobs_on_priority_created_at_when_unfinished.rb.erb deleted file mode 100644 index 31e7292ee..000000000 --- a/lib/generators/good_job/templates/update/migrations/03_create_index_good_jobs_jobs_on_priority_created_at_when_unfinished.rb.erb +++ /dev/null @@ -1,19 +0,0 @@ -# frozen_string_literal: true - -class CreateIndexGoodJobsJobsOnPriorityCreatedAtWhenUnfinished < ActiveRecord::Migration<%= migration_version %> - disable_ddl_transaction! - - def change - reversible do |dir| - dir.up do - # Ensure this incremental update migration is idempotent - # with monolithic install migration. - return if connection.index_name_exists?(:good_jobs, :index_good_jobs_jobs_on_priority_created_at_when_unfinished) - end - end - - add_index :good_jobs, [:priority, :created_at], order: { priority: "DESC NULLS LAST", created_at: :asc }, - where: "finished_at IS NULL", name: :index_good_jobs_jobs_on_priority_created_at_when_unfinished, - algorithm: :concurrently - end -end diff --git a/lib/generators/good_job/templates/update/migrations/04_create_good_job_batches.rb.erb b/lib/generators/good_job/templates/update/migrations/04_create_good_job_batches.rb.erb deleted file mode 100644 index 801573abe..000000000 --- a/lib/generators/good_job/templates/update/migrations/04_create_good_job_batches.rb.erb +++ /dev/null @@ -1,35 +0,0 @@ -# frozen_string_literal: true - -class CreateGoodJobBatches < ActiveRecord::Migration<%= migration_version %> - def change - reversible do |dir| - dir.up do - # Ensure this incremental update migration is idempotent - # with monolithic install migration. - return if connection.table_exists?(:good_job_batches) - end - end - - create_table :good_job_batches, id: :uuid do |t| - t.timestamps - t.text :description - t.jsonb :serialized_properties - t.text :on_finish - t.text :on_success - t.text :on_discard - t.text :callback_queue_name - t.integer :callback_priority - t.datetime :enqueued_at - t.datetime :discarded_at - t.datetime :finished_at - end - - change_table :good_jobs do |t| - t.uuid :batch_id - t.uuid :batch_callback_id - - t.index :batch_id, where: "batch_id IS NOT NULL" - t.index :batch_callback_id, where: "batch_callback_id IS NOT NULL" - end - end -end diff --git a/lib/generators/good_job/templates/update/migrations/05_create_good_job_executions.rb.erb b/lib/generators/good_job/templates/update/migrations/05_create_good_job_executions.rb.erb deleted file mode 100644 index 051d6f8f7..000000000 --- a/lib/generators/good_job/templates/update/migrations/05_create_good_job_executions.rb.erb +++ /dev/null @@ -1,33 +0,0 @@ -# frozen_string_literal: true - -class CreateGoodJobExecutions < ActiveRecord::Migration<%= migration_version %> - def change - reversible do |dir| - dir.up do - # Ensure this incremental update migration is idempotent - # with monolithic install migration. - return if connection.table_exists?(:good_job_executions) - end - end - - create_table :good_job_executions, id: :uuid do |t| - t.timestamps - - t.uuid :active_job_id, null: false - t.text :job_class - t.text :queue_name - t.jsonb :serialized_params - t.datetime :scheduled_at - t.datetime :finished_at - t.text :error - - t.index [:active_job_id, :created_at], name: :index_good_job_executions_on_active_job_id_and_created_at - end - - change_table :good_jobs do |t| - t.boolean :is_discrete - t.integer :executions_count - t.text :job_class - end - end -end diff --git a/lib/generators/good_job/templates/update/migrations/06_create_good_jobs_error_event.rb.erb b/lib/generators/good_job/templates/update/migrations/06_create_good_jobs_error_event.rb.erb deleted file mode 100644 index 45bd61e53..000000000 --- a/lib/generators/good_job/templates/update/migrations/06_create_good_jobs_error_event.rb.erb +++ /dev/null @@ -1,16 +0,0 @@ -# frozen_string_literal: true - -class CreateGoodJobsErrorEvent < ActiveRecord::Migration<%= migration_version %> - def change - reversible do |dir| - dir.up do - # Ensure this incremental update migration is idempotent - # with monolithic install migration. - return if connection.column_exists?(:good_jobs, :error_event) - end - end - - add_column :good_jobs, :error_event, :integer, limit: 2 - add_column :good_job_executions, :error_event, :integer, limit: 2 - end -end diff --git a/lib/generators/good_job/templates/update/migrations/07_recreate_good_job_cron_indexes_with_conditional.rb.erb b/lib/generators/good_job/templates/update/migrations/07_recreate_good_job_cron_indexes_with_conditional.rb.erb deleted file mode 100644 index 56a6e7ce5..000000000 --- a/lib/generators/good_job/templates/update/migrations/07_recreate_good_job_cron_indexes_with_conditional.rb.erb +++ /dev/null @@ -1,45 +0,0 @@ -# frozen_string_literal: true - -class RecreateGoodJobCronIndexesWithConditional < ActiveRecord::Migration<%= migration_version %> - disable_ddl_transaction! - - def change - reversible do |dir| - dir.up do - unless connection.index_name_exists?(:good_jobs, :index_good_jobs_on_cron_key_and_created_at_cond) - add_index :good_jobs, [:cron_key, :created_at], where: "(cron_key IS NOT NULL)", - name: :index_good_jobs_on_cron_key_and_created_at_cond, algorithm: :concurrently - end - unless connection.index_name_exists?(:good_jobs, :index_good_jobs_on_cron_key_and_cron_at_cond) - add_index :good_jobs, [:cron_key, :cron_at], where: "(cron_key IS NOT NULL)", unique: true, - name: :index_good_jobs_on_cron_key_and_cron_at_cond, algorithm: :concurrently - end - - if connection.index_name_exists?(:good_jobs, :index_good_jobs_on_cron_key_and_created_at) - remove_index :good_jobs, name: :index_good_jobs_on_cron_key_and_created_at - end - if connection.index_name_exists?(:good_jobs, :index_good_jobs_on_cron_key_and_cron_at) - remove_index :good_jobs, name: :index_good_jobs_on_cron_key_and_cron_at - end - end - - dir.down do - unless connection.index_name_exists?(:good_jobs, :index_good_jobs_on_cron_key_and_created_at) - add_index :good_jobs, [:cron_key, :created_at], - name: :index_good_jobs_on_cron_key_and_created_at, algorithm: :concurrently - end - unless connection.index_name_exists?(:good_jobs, :index_good_jobs_on_cron_key_and_cron_at) - add_index :good_jobs, [:cron_key, :cron_at], unique: true, - name: :index_good_jobs_on_cron_key_and_cron_at, algorithm: :concurrently - end - - if connection.index_name_exists?(:good_jobs, :index_good_jobs_on_cron_key_and_created_at_cond) - remove_index :good_jobs, name: :index_good_jobs_on_cron_key_and_created_at_cond - end - if connection.index_name_exists?(:good_jobs, :index_good_jobs_on_cron_key_and_cron_at_cond) - remove_index :good_jobs, name: :index_good_jobs_on_cron_key_and_cron_at_cond - end - end - end - end -end diff --git a/lib/generators/good_job/templates/update/migrations/08_create_good_job_labels.rb.erb b/lib/generators/good_job/templates/update/migrations/08_create_good_job_labels.rb.erb deleted file mode 100644 index 0bc3ec97e..000000000 --- a/lib/generators/good_job/templates/update/migrations/08_create_good_job_labels.rb.erb +++ /dev/null @@ -1,15 +0,0 @@ -# frozen_string_literal: true - -class CreateGoodJobLabels < ActiveRecord::Migration<%= migration_version %> - def change - reversible do |dir| - dir.up do - # Ensure this incremental update migration is idempotent - # with monolithic install migration. - return if connection.column_exists?(:good_jobs, :labels) - end - end - - add_column :good_jobs, :labels, :text, array: true - end -end diff --git a/lib/generators/good_job/templates/update/migrations/09_create_good_job_labels_index.rb.erb b/lib/generators/good_job/templates/update/migrations/09_create_good_job_labels_index.rb.erb deleted file mode 100644 index e069bd9c5..000000000 --- a/lib/generators/good_job/templates/update/migrations/09_create_good_job_labels_index.rb.erb +++ /dev/null @@ -1,22 +0,0 @@ -# frozen_string_literal: true - -class CreateGoodJobLabelsIndex < ActiveRecord::Migration<%= migration_version %> - disable_ddl_transaction! - - def change - reversible do |dir| - dir.up do - unless connection.index_name_exists?(:good_jobs, :index_good_jobs_on_labels) - add_index :good_jobs, :labels, using: :gin, where: "(labels IS NOT NULL)", - name: :index_good_jobs_on_labels, algorithm: :concurrently - end - end - - dir.down do - if connection.index_name_exists?(:good_jobs, :index_good_jobs_on_labels) - remove_index :good_jobs, name: :index_good_jobs_on_labels - end - end - end - end -end diff --git a/lib/generators/good_job/templates/update/migrations/10_remove_good_job_active_id_index.rb.erb b/lib/generators/good_job/templates/update/migrations/10_remove_good_job_active_id_index.rb.erb deleted file mode 100644 index 3163e0a07..000000000 --- a/lib/generators/good_job/templates/update/migrations/10_remove_good_job_active_id_index.rb.erb +++ /dev/null @@ -1,21 +0,0 @@ -# frozen_string_literal: true - -class RemoveGoodJobActiveIdIndex < ActiveRecord::Migration<%= migration_version %> - disable_ddl_transaction! - - def change - reversible do |dir| - dir.up do - if connection.index_name_exists?(:good_jobs, :index_good_jobs_on_active_job_id) - remove_index :good_jobs, name: :index_good_jobs_on_active_job_id - end - end - - dir.down do - unless connection.index_name_exists?(:good_jobs, :index_good_jobs_on_active_job_id) - add_index :good_jobs, :active_job_id, name: :index_good_jobs_on_active_job_id - end - end - end - end -end diff --git a/lib/generators/good_job/templates/update/migrations/11_create_index_good_job_jobs_for_candidate_lookup.rb.erb b/lib/generators/good_job/templates/update/migrations/11_create_index_good_job_jobs_for_candidate_lookup.rb.erb deleted file mode 100644 index 0b65bf031..000000000 --- a/lib/generators/good_job/templates/update/migrations/11_create_index_good_job_jobs_for_candidate_lookup.rb.erb +++ /dev/null @@ -1,19 +0,0 @@ -# frozen_string_literal: true - -class CreateIndexGoodJobJobsForCandidateLookup < ActiveRecord::Migration<%= migration_version %> - disable_ddl_transaction! - - def change - reversible do |dir| - dir.up do - # Ensure this incremental update migration is idempotent - # with monolithic install migration. - return if connection.index_name_exists?(:good_jobs, :index_good_job_jobs_for_candidate_lookup) - end - end - - add_index :good_jobs, [:priority, :created_at], order: { priority: "ASC NULLS LAST", created_at: :asc }, - where: "finished_at IS NULL", name: :index_good_job_jobs_for_candidate_lookup, - algorithm: :concurrently - end -end diff --git a/lib/generators/good_job/templates/update/migrations/12_create_good_job_execution_error_backtrace.rb.erb b/lib/generators/good_job/templates/update/migrations/12_create_good_job_execution_error_backtrace.rb.erb deleted file mode 100644 index ffcde1558..000000000 --- a/lib/generators/good_job/templates/update/migrations/12_create_good_job_execution_error_backtrace.rb.erb +++ /dev/null @@ -1,15 +0,0 @@ -# frozen_string_literal: true - -class CreateGoodJobExecutionErrorBacktrace < ActiveRecord::Migration<%= migration_version %> - def change - reversible do |dir| - dir.up do - # Ensure this incremental update migration is idempotent - # with monolithic install migration. - return if connection.column_exists?(:good_job_executions, :error_backtrace) - end - end - - add_column :good_job_executions, :error_backtrace, :text, array: true - end -end diff --git a/lib/generators/good_job/templates/update/migrations/13_create_good_job_process_lock_ids.rb.erb b/lib/generators/good_job/templates/update/migrations/13_create_good_job_process_lock_ids.rb.erb deleted file mode 100644 index a42ec5117..000000000 --- a/lib/generators/good_job/templates/update/migrations/13_create_good_job_process_lock_ids.rb.erb +++ /dev/null @@ -1,18 +0,0 @@ -# frozen_string_literal: true - -class CreateGoodJobProcessLockIds < ActiveRecord::Migration<%= migration_version %> - def change - reversible do |dir| - dir.up do - # Ensure this incremental update migration is idempotent - # with monolithic install migration. - return if connection.column_exists?(:good_jobs, :locked_by_id) - end - end - - add_column :good_jobs, :locked_by_id, :uuid - add_column :good_jobs, :locked_at, :datetime - add_column :good_job_executions, :process_id, :uuid - add_column :good_job_processes, :lock_type, :integer, limit: 2 - end -end diff --git a/lib/generators/good_job/templates/update/migrations/14_create_good_job_process_lock_indexes.rb.erb b/lib/generators/good_job/templates/update/migrations/14_create_good_job_process_lock_indexes.rb.erb deleted file mode 100644 index efb3509d9..000000000 --- a/lib/generators/good_job/templates/update/migrations/14_create_good_job_process_lock_indexes.rb.erb +++ /dev/null @@ -1,38 +0,0 @@ -# frozen_string_literal: true - -class CreateGoodJobProcessLockIndexes < ActiveRecord::Migration<%= migration_version %> - disable_ddl_transaction! - - def change - reversible do |dir| - dir.up do - unless connection.index_name_exists?(:good_jobs, :index_good_jobs_on_priority_scheduled_at_unfinished_unlocked) - add_index :good_jobs, [:priority, :scheduled_at], - order: { priority: "ASC NULLS LAST", scheduled_at: :asc }, - where: "finished_at IS NULL AND locked_by_id IS NULL", - name: :index_good_jobs_on_priority_scheduled_at_unfinished_unlocked, - algorithm: :concurrently - end - - unless connection.index_name_exists?(:good_jobs, :index_good_jobs_on_locked_by_id) - add_index :good_jobs, :locked_by_id, - where: "locked_by_id IS NOT NULL", - name: :index_good_jobs_on_locked_by_id, - algorithm: :concurrently - end - - unless connection.index_name_exists?(:good_job_executions, :index_good_job_executions_on_process_id_and_created_at) - add_index :good_job_executions, [:process_id, :created_at], - name: :index_good_job_executions_on_process_id_and_created_at, - algorithm: :concurrently - end - end - - dir.down do - remove_index(:good_jobs, name: :index_good_jobs_on_priority_scheduled_at_unfinished_unlocked) if connection.index_name_exists?(:good_jobs, :index_good_jobs_on_priority_scheduled_at_unfinished_unlocked) - remove_index(:good_jobs, name: :index_good_jobs_on_locked_by_id) if connection.index_name_exists?(:good_jobs, :index_good_jobs_on_locked_by_id) - remove_index(:good_job_executions, name: :index_good_job_executions_on_process_id_and_created_at) if connection.index_name_exists?(:good_job_executions, :index_good_job_executions_on_process_id_and_created_at) - end - end - end -end diff --git a/lib/generators/good_job/templates/update/migrations/15_create_good_job_execution_duration.rb.erb b/lib/generators/good_job/templates/update/migrations/15_create_good_job_execution_duration.rb.erb deleted file mode 100644 index 8d8919750..000000000 --- a/lib/generators/good_job/templates/update/migrations/15_create_good_job_execution_duration.rb.erb +++ /dev/null @@ -1,15 +0,0 @@ -# frozen_string_literal: true - -class CreateGoodJobExecutionDuration < ActiveRecord::Migration<%= migration_version %> - def change - reversible do |dir| - dir.up do - # Ensure this incremental update migration is idempotent - # with monolithic install migration. - return if connection.column_exists?(:good_job_executions, :duration) - end - end - - add_column :good_job_executions, :duration, :interval - end -end diff --git a/lib/good_job.rb b/lib/good_job.rb index 09126c137..b25ed027a 100644 --- a/lib/good_job.rb +++ b/lib/good_job.rb @@ -58,7 +58,7 @@ module GoodJob # @!attribute [rw] active_record_parent_class # @!scope class - # The ActiveRecord parent class inherited by +GoodJob::Execution+ (default: +ActiveRecord::Base+). + # The ActiveRecord parent class inherited by +GoodJob::Job+ (default: +ActiveRecord::Base+). # Use this when using multiple databases or other custom ActiveRecord configuration. # @return [ActiveRecord::Base] # @example Change the base class: @@ -133,6 +133,7 @@ def self._on_thread_error(exception) def self.configure_active_record(&block) self._active_record_configuration = block end + mattr_accessor :_active_record_configuration, default: nil # Stop executing jobs. @@ -208,7 +209,7 @@ def self.cleanup_preserved_jobs(older_than: nil, in_batches_of: 1_000) include_discarded = GoodJob.configuration.cleanup_discarded_jobs? ActiveSupport::Notifications.instrument("cleanup_preserved_jobs.good_job", { older_than: older_than, timestamp: timestamp }) do |payload| - deleted_executions_count = 0 + deleted_jobs_count = 0 deleted_batches_count = 0 deleted_discrete_executions_count = 0 @@ -218,31 +219,27 @@ def self.cleanup_preserved_jobs(older_than: nil, in_batches_of: 1_000) active_job_ids = jobs_query.pluck(:active_job_id) break if active_job_ids.empty? - if GoodJob::Execution.discrete_support? - deleted_discrete_executions = GoodJob::DiscreteExecution.where(active_job_id: active_job_ids).delete_all - deleted_discrete_executions_count += deleted_discrete_executions - end + deleted_discrete_executions = GoodJob::DiscreteExecution.where(active_job_id: active_job_ids).delete_all + deleted_discrete_executions_count += deleted_discrete_executions - deleted_executions = GoodJob::Execution.where(active_job_id: active_job_ids).delete_all - deleted_executions_count += deleted_executions + deleted_jobs = GoodJob::Job.where(active_job_id: active_job_ids).delete_all + deleted_jobs_count += deleted_jobs end - if GoodJob::BatchRecord.migrated? - batches_query = GoodJob::BatchRecord.finished_before(timestamp).limit(in_batches_of) - batches_query = batches_query.succeeded unless include_discarded - loop do - deleted = batches_query.delete_all - break if deleted.zero? + batches_query = GoodJob::BatchRecord.finished_before(timestamp).limit(in_batches_of) + batches_query = batches_query.succeeded unless include_discarded + loop do + deleted = batches_query.delete_all + break if deleted.zero? - deleted_batches_count += deleted - end + deleted_batches_count += deleted end payload[:destroyed_batches_count] = deleted_batches_count payload[:destroyed_discrete_executions_count] = deleted_discrete_executions_count - payload[:destroyed_executions_count] = deleted_executions_count + payload[:destroyed_jobs_count] = deleted_jobs_count - destroyed_records_count = deleted_batches_count + deleted_discrete_executions_count + deleted_executions_count + destroyed_records_count = deleted_batches_count + deleted_discrete_executions_count + deleted_jobs_count payload[:destroyed_records_count] = destroyed_records_count destroyed_records_count @@ -291,9 +288,7 @@ def self.deprecator # For use in tests/CI to validate GoodJob is up-to-date. # @return [Boolean] def self.migrated? - # Always update with the most recent migration check - GoodJob::DiscreteExecution.reset_column_information - GoodJob::DiscreteExecution.duration_interval_migrated? + true end ActiveSupport.run_load_hooks(:good_job, self) diff --git a/lib/good_job/active_job_extensions/batches.rb b/lib/good_job/active_job_extensions/batches.rb index a7d7218fc..831c041b2 100644 --- a/lib/good_job/active_job_extensions/batches.rb +++ b/lib/good_job/active_job_extensions/batches.rb @@ -12,7 +12,7 @@ module Batches end def batch - @_batch ||= CurrentThread.execution&.batch&.to_batch if CurrentThread.execution.present? && CurrentThread.execution.active_job_id == job_id + @_batch ||= CurrentThread.job&.batch&.to_batch if CurrentThread.job.present? && CurrentThread.job.active_job_id == job_id end alias batch? batch end diff --git a/lib/good_job/active_job_extensions/concurrency.rb b/lib/good_job/active_job_extensions/concurrency.rb index 4a5abb460..45625d052 100644 --- a/lib/good_job/active_job_extensions/concurrency.rb +++ b/lib/good_job/active_job_extensions/concurrency.rb @@ -66,7 +66,7 @@ def deserialize(job_data) perform_throttle = job.class.good_job_concurrency_config[:perform_throttle] perform_throttle = instance_exec(&perform_throttle) if perform_throttle.respond_to?(:call) - perform_throttle = nil unless GoodJob::DiscreteExecution.migrated? && perform_throttle.present? && perform_throttle.is_a?(Array) && perform_throttle.size == 2 + perform_throttle = nil unless perform_throttle.present? && perform_throttle.is_a?(Array) && perform_throttle.size == 2 limit = perform_limit || total_limit throttle = perform_throttle @@ -75,17 +75,17 @@ def deserialize(job_data) key = job.good_job_concurrency_key next if key.blank? - if CurrentThread.execution.blank? || CurrentThread.execution.active_job_id != job_id + if CurrentThread.job.blank? || CurrentThread.job.active_job_id != job_id logger.debug("Ignoring concurrency limits because the job is executed with `perform_now`.") next end - GoodJob::Execution.advisory_lock_key(key, function: "pg_advisory_lock") do + GoodJob::Job.advisory_lock_key(key, function: "pg_advisory_lock") do if limit - allowed_active_job_ids = GoodJob::Execution.unfinished.where(concurrency_key: key) - .advisory_locked - .order(Arel.sql("COALESCE(performed_at, scheduled_at, created_at) ASC")) - .limit(limit).pluck(:active_job_id) + allowed_active_job_ids = GoodJob::Job.unfinished.where(concurrency_key: key) + .advisory_locked + .order(Arel.sql("COALESCE(performed_at, scheduled_at, created_at) ASC")) + .limit(limit).pluck(:active_job_id) # The current job has already been locked and will appear in the previous query raise GoodJob::ActiveJobExtensions::Concurrency::ConcurrencyExceededError unless allowed_active_job_ids.include?(job.job_id) end @@ -172,12 +172,12 @@ def good_job_enqueue_concurrency_check(job, on_abort:, on_enqueue:) throttle = enqueue_throttle return on_enqueue&.call unless limit || throttle - GoodJob::Execution.advisory_lock_key(key, function: "pg_advisory_lock") do + GoodJob::Job.advisory_lock_key(key, function: "pg_advisory_lock") do if limit enqueue_concurrency = if enqueue_limit - GoodJob::Execution.where(concurrency_key: key).unfinished.advisory_unlocked.count + GoodJob::Job.where(concurrency_key: key).unfinished.advisory_unlocked.count else - GoodJob::Execution.where(concurrency_key: key).unfinished.count + GoodJob::Job.where(concurrency_key: key).unfinished.count end # The job has not yet been enqueued, so check if adding it will go over the limit diff --git a/lib/good_job/adapter.rb b/lib/good_job/adapter.rb index c7a6bda7b..dae9276f9 100644 --- a/lib/good_job/adapter.rb +++ b/lib/good_job/adapter.rb @@ -37,7 +37,7 @@ def initialize(execution_mode: nil, _capsule: GoodJob.capsule) # rubocop:disable # Enqueues the ActiveJob job to be performed. # For use by Rails; you should generally not call this directly. # @param active_job [ActiveJob::Base] the job to be enqueued from +#perform_later+ - # @return [GoodJob::Execution] + # @return [GoodJob::Job] def enqueue(active_job) enqueue_at(active_job, nil) end @@ -58,28 +58,17 @@ def enqueue_all(active_jobs) Rails.application.executor.wrap do current_time = Time.current executions = active_jobs.map do |active_job| - GoodJob::Execution.build_for_enqueue(active_job).tap do |execution| - if GoodJob::Execution.discrete_support? - execution.make_discrete - execution.scheduled_at = current_time if execution.scheduled_at == execution.created_at - end - - execution.created_at = current_time - execution.updated_at = current_time + GoodJob::Job.build_for_enqueue(active_job).tap do |job| + job.scheduled_at = current_time if job.scheduled_at == job.created_at + job.created_at = current_time + job.updated_at = current_time end end inline_executions = [] - GoodJob::Execution.transaction(requires_new: true, joinable: false) do - execution_attributes = executions.map do |execution| - if GoodJob::Execution.error_event_migrated? - execution.attributes - else - execution.attributes.except('error_event') - end - end - - results = GoodJob::Execution.insert_all(execution_attributes, returning: %w[id active_job_id]) # rubocop:disable Rails/SkipsModelValidations + GoodJob::Job.transaction(requires_new: true, joinable: false) do + execution_attributes = executions.map(&:attributes) + results = GoodJob::Job.insert_all(execution_attributes, returning: %w[id active_job_id]) # rubocop:disable Rails/SkipsModelValidations job_id_to_provider_job_id = results.each_with_object({}) { |result, hash| hash[result['active_job_id']] = result['id'] } active_jobs.each do |active_job| @@ -146,7 +135,7 @@ def enqueue_all(active_jobs) # For use by Rails; you should generally not call this directly. # @param active_job [ActiveJob::Base] the job to be enqueued from +#perform_later+ # @param timestamp [Integer, nil] the epoch time to perform the job - # @return [GoodJob::Execution] + # @return [GoodJob::Job] def enqueue_at(active_job, timestamp) scheduled_at = timestamp ? Time.zone.at(timestamp) : nil @@ -156,15 +145,15 @@ def enqueue_at(active_job, timestamp) Rails.application.executor.wrap do will_execute_inline = execute_inline? && (scheduled_at.nil? || scheduled_at <= Time.current) - will_retry_inline = will_execute_inline && CurrentThread.execution&.active_job_id == active_job.job_id && !CurrentThread.retry_now + will_retry_inline = will_execute_inline && CurrentThread.job&.active_job_id == active_job.job_id && !CurrentThread.retry_now if will_retry_inline - execution = GoodJob::Execution.enqueue( + execution = GoodJob::Job.enqueue( active_job, scheduled_at: scheduled_at ) elsif will_execute_inline - execution = GoodJob::Execution.enqueue( + execution = GoodJob::Job.enqueue( active_job, scheduled_at: scheduled_at, create_with_advisory_lock: true @@ -186,7 +175,7 @@ def enqueue_at(active_job, timestamp) end raise result.unhandled_error if result.unhandled_error else - execution = GoodJob::Execution.enqueue( + execution = GoodJob::Job.enqueue( active_job, scheduled_at: scheduled_at ) diff --git a/lib/good_job/current_thread.rb b/lib/good_job/current_thread.rb index 9078d40b0..eb1cb3db1 100644 --- a/lib/good_job/current_thread.rb +++ b/lib/good_job/current_thread.rb @@ -13,7 +13,7 @@ module CurrentThread error_on_discard error_on_retry error_on_retry_stopped - execution + job execution_interrupted execution_retried retry_now @@ -49,11 +49,11 @@ module CurrentThread # @return [Exception, nil] thread_mattr_accessor :error_on_retry_stopped - # @!attribute [rw] executions + # @!attribute [rw] jobs # @!scope class # Execution - # @return [GoodJob::Execution, nil] - thread_mattr_accessor :execution + # @return [GoodJob::Job, nil] + thread_mattr_accessor :job # @!attribute [rw] execution_interrupted # @!scope class @@ -90,9 +90,9 @@ def self.to_h end end - # @return [String] UUID of the currently executing GoodJob::Execution + # @return [String] UUID of the currently executing GoodJob::Job def self.active_job_id - execution&.active_job_id + job&.active_job_id end # @return [Integer] Current process ID diff --git a/lib/good_job/job_performer.rb b/lib/good_job/job_performer.rb index a0a6c734b..f5ed4a4cb 100644 --- a/lib/good_job/job_performer.rb +++ b/lib/good_job/job_performer.rb @@ -108,11 +108,11 @@ def reset_stats attr_reader :queue_string def job_query - @_job_query ||= GoodJob::Execution.queue_string(queue_string) + @_job_query ||= GoodJob::Job.queue_string(queue_string) end def parsed_queues - @_parsed_queues ||= GoodJob::Execution.queue_parser(queue_string) + @_parsed_queues ||= GoodJob::Job.queue_parser(queue_string) end end end diff --git a/lib/good_job/log_subscriber.rb b/lib/good_job/log_subscriber.rb index 95dc2771d..6e3527c95 100644 --- a/lib/good_job/log_subscriber.rb +++ b/lib/good_job/log_subscriber.rb @@ -18,14 +18,6 @@ class LogSubscriber < ActiveSupport::LogSubscriber # Responds to the +$0.good_job+ notification. # @param event [ActiveSupport::Notifications::Event] # @return [void] - def create(event) - # FIXME: This method does not match any good_job notifications. - execution = event.payload[:execution] - - debug do - "GoodJob created job resource with id #{execution.id}" - end - end # @!macro notification_responder def finished_timer_task(event) @@ -110,12 +102,12 @@ def scheduler_restart_pools(event) # @!macro notification_responder def perform_job(event) - execution = event.payload[:execution] + job = event.payload[:job] process_id = event.payload[:process_id] thread_name = event.payload[:thread_name] info(tags: [process_id, thread_name]) do - "Executed GoodJob #{execution.id}" + "Executed GoodJob #{job.id}" end end diff --git a/lib/good_job/notifier.rb b/lib/good_job/notifier.rb index 99cf99f71..2a8621b48 100644 --- a/lib/good_job/notifier.rb +++ b/lib/good_job/notifier.rb @@ -51,7 +51,7 @@ class Notifier # Send a message via Postgres NOTIFY # @param message [#to_json] def self.notify(message) - connection = ::GoodJob::Execution.connection + connection = ::GoodJob::Job.connection connection.exec_query <<~SQL.squish NOTIFY #{CHANNEL}, #{connection.quote(message.to_json)} SQL @@ -251,8 +251,8 @@ def create_listen_task(delay: 0) def with_connection Rails.application.executor.wrap do - self.connection = ::GoodJob::Execution.connection_pool.checkout.tap do |conn| - ::GoodJob::Execution.connection_pool.remove(conn) + self.connection = ::GoodJob::Job.connection_pool.checkout.tap do |conn| + ::GoodJob::Job.connection_pool.remove(conn) end end connection.execute("SET application_name = #{connection.quote(self.class.name)}") diff --git a/sorbet/rbi/todo.rbi b/sorbet/rbi/todo.rbi index 3c3d55f43..d4c1fee99 100644 --- a/sorbet/rbi/todo.rbi +++ b/sorbet/rbi/todo.rbi @@ -38,7 +38,9 @@ module ::TestJob::Error; end module ::TestJob::ExpectedError; end module ::TestJob::RunError; end module ::TestJob::SuccessCallbackJob; end +module ::TestRecord; end module ::WrapperJob; end module GoodJob::Job::ERROR_EVENT_INTERRUPTED; end module GoodJob::Job::ERROR_EVENT_RETRIED; end module Rails::Server; end + diff --git a/spec/app/filters/good_job/jobs_filter_spec.rb b/spec/app/filters/good_job/jobs_filter_spec.rb index 83daa3c8f..0b5c2c421 100644 --- a/spec/app/filters/good_job/jobs_filter_spec.rb +++ b/spec/app/filters/good_job/jobs_filter_spec.rb @@ -26,16 +26,16 @@ end Timecop.return - running_job = ExampleJob.perform_later('success') - running_execution = GoodJob::Execution.find(running_job.provider_job_id) - running_execution.update!( + running_active_job = ExampleJob.perform_later('success') + running_job = GoodJob::Job.find(running_active_job.provider_job_id) + running_job.update!( finished_at: nil ) - running_execution.advisory_lock + running_job.advisory_lock end after do - GoodJob::Execution.advisory_unlock_session + GoodJob::Job.advisory_unlock_session end describe '#job_classes' do diff --git a/spec/app/jobs/example_job_spec.rb b/spec/app/jobs/example_job_spec.rb index 7e0b2f604..ed4a43293 100644 --- a/spec/app/jobs/example_job_spec.rb +++ b/spec/app/jobs/example_job_spec.rb @@ -12,8 +12,8 @@ describe "SUCCESS_TYPE" do it 'completes successfully' do active_job = described_class.perform_later(described_class::SUCCESS_TYPE) - execution = GoodJob::Execution.find(active_job.provider_job_id) - expect(execution.error).to be_nil + job = GoodJob::Job.find(active_job.provider_job_id) + expect(job.error).to be_nil end end @@ -72,8 +72,8 @@ active_job = described_class.perform_later(described_class::SLOW_TYPE) - execution = GoodJob::Execution.find(active_job.provider_job_id) - expect(execution.error).to be_nil + job = GoodJob::Job.find(active_job.provider_job_id) + expect(job.error).to be_nil end end end diff --git a/spec/app/models/concerns/good_job/advisory_lockable_spec.rb b/spec/app/models/concerns/good_job/advisory_lockable_spec.rb index d52eea314..57333601e 100644 --- a/spec/app/models/concerns/good_job/advisory_lockable_spec.rb +++ b/spec/app/models/concerns/good_job/advisory_lockable_spec.rb @@ -3,9 +3,17 @@ require 'rails_helper' RSpec.describe GoodJob::AdvisoryLockable do - let(:model_class) { GoodJob::Execution } - let!(:execution) { model_class.create(active_job_id: SecureRandom.uuid, queue_name: "default") } - let!(:another_execution) { model_class.create(active_job_id: SecureRandom.uuid, queue_name: "default") } + before do + stub_const "TestRecord", (Class.new(GoodJob::BaseRecord) do + include GoodJob::AdvisoryLockable + self.table_name = "good_jobs" + self.advisory_lockable_column = "active_job_id" + end) + end + + let(:model_class) { TestRecord } + let!(:job) { model_class.create!(active_job_id: SecureRandom.uuid, queue_name: "default") } + let!(:another_job) { model_class.create!(active_job_id: SecureRandom.uuid, queue_name: "default") } describe '.advisory_lock' do around do |example| @@ -101,53 +109,53 @@ end it 'returns first row of the query with a lock' do - execution.update!(queue_name: "aaaaaa") - another_execution.update!(queue_name: "bbbbbb") + job.update!(queue_name: "aaaaaa") + another_job.update!(queue_name: "bbbbbb") - expect(execution).not_to be_advisory_locked - result_execution = model_class.order(queue_name: :asc).limit(1).advisory_lock.first - expect(result_execution).to eq execution - expect(execution).to be_advisory_locked - expect(another_execution).not_to be_advisory_locked + expect(job).not_to be_advisory_locked + result_job = model_class.order(queue_name: :asc).limit(1).advisory_lock.first + expect(result_job).to eq job + expect(job).to be_advisory_locked + expect(another_job).not_to be_advisory_locked - execution.advisory_unlock + job.advisory_unlock end it 'can lock an alternative column' do - expect(execution).not_to be_advisory_locked - result_execution = model_class.order(created_at: :asc).limit(1).advisory_lock(column: :queue_name).first - expect(result_execution).to eq execution - expect(execution).to be_advisory_locked(key: "good_jobs-default") - expect(execution).not_to be_advisory_locked # on default key + expect(job).not_to be_advisory_locked + result_job = model_class.order(created_at: :asc).limit(1).advisory_lock(column: :queue_name).first + expect(result_job).to eq job + expect(job).to be_advisory_locked(key: "good_jobs-default") + expect(job).not_to be_advisory_locked # on default key - execution.advisory_unlock(key: "good_jobs-default") + job.advisory_unlock(key: "good_jobs-default") end end describe '.advisory_lock_key' do it 'locks a key' do - model_class.advisory_lock_key(execution.lockable_key) - expect(execution).to be_advisory_locked - expect(model_class.advisory_locked_key?(execution.lockable_key)).to be true - model_class.advisory_unlock_key(execution.lockable_key) + model_class.advisory_lock_key(job.lockable_key) + expect(job).to be_advisory_locked + expect(model_class.advisory_locked_key?(job.lockable_key)).to be true + model_class.advisory_unlock_key(job.lockable_key) end context 'when a block is passed' do it 'locks that key for the bloc and then unlocks it' do - model_class.advisory_lock_key(execution.lockable_key) do - expect(execution.advisory_locked?).to be true - expect(execution.owns_advisory_lock?).to be true + model_class.advisory_lock_key(job.lockable_key) do + expect(job.advisory_locked?).to be true + expect(job.owns_advisory_lock?).to be true expect(PgLock.current_database.advisory_lock.count).to eq 1 end - expect(execution.advisory_locked?).to be false - expect(execution.owns_advisory_lock?).to be false + expect(job.advisory_locked?).to be false + expect(job.owns_advisory_lock?).to be false end it 'does not invoke the block if the key is already locked' do - model_class.advisory_lock_key(execution.lockable_key) do + model_class.advisory_lock_key(job.lockable_key) do promise = rails_promise do - result = model_class.advisory_lock_key(execution.lockable_key) { raise } + result = model_class.advisory_lock_key(job.lockable_key) { raise } expect(result).to be_nil end expect { promise.value! }.not_to raise_error @@ -175,8 +183,8 @@ done_event = Concurrent::Event.new promise = rails_promise do - model_class.advisory_lock_key(execution.lockable_key) do - expect(execution.owns_advisory_lock?).to be true + model_class.advisory_lock_key(job.lockable_key) do + expect(job.owns_advisory_lock?).to be true locked_event.set done_event.wait(5) @@ -184,7 +192,7 @@ end locked_event.wait(5) - expect(execution.owns_advisory_lock?).to be false + expect(job.owns_advisory_lock?).to be false ensure locked_event.set done_event.set @@ -198,7 +206,7 @@ model_class.limit(2).with_advisory_lock do |results| records = results - expect(records).to include(execution) + expect(records).to include(job) expect(records).to all be_advisory_locked end @@ -250,85 +258,85 @@ describe '.includes_advisory_locks' do it 'includes the locktable data' do - execution.advisory_lock! + job.advisory_lock! - record = model_class.where(id: execution.id).includes_advisory_locks.first + record = model_class.where(id: job.id).includes_advisory_locks.first expect(record['locktype']).to eq "advisory" expect(record['owns_advisory_lock']).to be true - execution.advisory_unlock + job.advisory_unlock end end describe '#advisory_lock' do it 'results in a locked record' do - execution.advisory_lock! - expect(execution.advisory_locked?).to be true - expect(execution.owns_advisory_lock?).to be true + job.advisory_lock! + expect(job.advisory_locked?).to be true + expect(job.owns_advisory_lock?).to be true - other_thread_owns_advisory_lock = rails_promise(execution, &:owns_advisory_lock?).value! + other_thread_owns_advisory_lock = rails_promise(job, &:owns_advisory_lock?).value! expect(other_thread_owns_advisory_lock).to be false - execution.advisory_unlock + job.advisory_unlock end it 'returns true or false if the lock is acquired' do - expect(execution.advisory_lock).to be true + expect(job.advisory_lock).to be true - expect(rails_promise(execution, &:advisory_lock).value!).to be false + expect(rails_promise(job, &:advisory_lock).value!).to be false - execution.advisory_unlock + job.advisory_unlock end it 'can lock alternative values' do - execution.advisory_lock!(key: "alternative") - expect(execution.advisory_locked?(key: "alternative")).to be true - expect(execution.advisory_locked?).to be false + job.advisory_lock!(key: "alternative") + expect(job.advisory_locked?(key: "alternative")).to be true + expect(job.advisory_locked?).to be false - execution.advisory_unlock(key: "alternative") + job.advisory_unlock(key: "alternative") end it 'can lock alternative postgres functions' do - execution.advisory_lock!(function: "pg_advisory_lock") - expect(execution.advisory_locked?).to be true - execution.advisory_unlock + job.advisory_lock!(function: "pg_advisory_lock") + expect(job.advisory_locked?).to be true + job.advisory_unlock end end describe '#advisory_unlock' do it 'unlocks the record' do - execution.advisory_lock! + job.advisory_lock! expect do - execution.advisory_unlock - end.to change(execution, :advisory_locked?).from(true).to(false) + job.advisory_unlock + end.to change(job, :advisory_locked?).from(true).to(false) end it 'unlocks the record only once' do - execution.advisory_lock! - execution.advisory_lock! + job.advisory_lock! + job.advisory_lock! expect do - execution.advisory_unlock - end.not_to change(execution, :advisory_locked?).from(true) + job.advisory_unlock + end.not_to change(job, :advisory_locked?).from(true) - execution.advisory_unlock + job.advisory_unlock end it 'unlocks the record even after the record is destroyed' do - execution.advisory_lock! - execution.destroy! + job.advisory_lock! + job.destroy! expect do - execution.advisory_unlock - end.to change(execution, :advisory_locked?).from(true).to(false) + job.advisory_unlock + end.to change(job, :advisory_locked?).from(true).to(false) end it 'returns true or false if the unlock operation is successful' do - execution.advisory_lock + job.advisory_lock - expect(rails_promise(execution, &:advisory_unlock).value!).to be false - expect(execution.advisory_unlock).to be true + expect(rails_promise(job, &:advisory_unlock).value!).to be false + expect(job.advisory_unlock).to be true unless RUBY_PLATFORM.include?('java') expect(POSTGRES_NOTICES.first).to include "you don't own a lock of type ExclusiveLock" @@ -339,53 +347,53 @@ describe '#advisory_locked?' do it 'reflects whether the record is locked' do - expect(execution.advisory_locked?).to be false - execution.advisory_lock - expect(execution.advisory_locked?).to be true - execution.advisory_unlock - expect(execution.advisory_locked?).to be false + expect(job.advisory_locked?).to be false + job.advisory_lock + expect(job.advisory_locked?).to be true + job.advisory_unlock + expect(job.advisory_locked?).to be false end - it 'is accurate even if the execution has been destroyed' do - execution.advisory_lock - expect(execution.advisory_locked?).to be true - execution.destroy! - expect(execution.advisory_locked?).to be true - execution.advisory_unlock - expect(execution.advisory_locked?).to be false + it 'is accurate even if the job has been destroyed' do + job.advisory_lock + expect(job.advisory_locked?).to be true + job.destroy! + expect(job.advisory_locked?).to be true + job.advisory_unlock + expect(job.advisory_locked?).to be false end end describe '#advisory_unlock!' do it 'unlocks the record entirely' do - execution.advisory_lock! - execution.advisory_lock! + job.advisory_lock! + job.advisory_lock! expect do - execution.advisory_unlock! - end.to change(execution, :advisory_locked?).from(true).to(false) + job.advisory_unlock! + end.to change(job, :advisory_locked?).from(true).to(false) end end describe '.advisory_unlock_session' do it 'unlocks all locks in the session' do - execution.advisory_lock! + job.advisory_lock! model_class.advisory_unlock_session - expect(execution.advisory_locked?).to be false + expect(job.advisory_locked?).to be false end end describe 'create_with_advisory_lock' do - it 'causes the execution to be saved and locked' do - execution = model_class.new - execution.create_with_advisory_lock = true - execution.save! + it 'causes the job to be saved and locked' do + job = model_class.new + job.create_with_advisory_lock = true + job.save! - expect(execution).to be_advisory_locked + expect(job).to be_advisory_locked - execution.advisory_unlock + job.advisory_unlock end it 'aborts when the lock already exists' do @@ -401,13 +409,13 @@ it 'is lockable' do ActiveRecord::Base.connection_handler.clear_active_connections! - execution.advisory_lock! + job.advisory_lock! expect do - rails_promise(execution, &:advisory_lock!).value! + rails_promise(job, &:advisory_lock!).value! end.to raise_error GoodJob::AdvisoryLockable::RecordAlreadyAdvisoryLockedError - execution.advisory_unlock + job.advisory_unlock end describe 'Advisory Lock behavior' do @@ -418,10 +426,10 @@ done_event = Concurrent::Event.new promise = rails_promise do - execution.class.connection # <= This is necessary to fixate the connection in the thread + job.class.connection # <= This is necessary to fixate the connection in the thread - execution.class.transaction do - execution.advisory_lock + job.class.transaction do + job.advisory_lock locked_event.set commit_event.wait(10) @@ -429,15 +437,15 @@ committed_event.set done_event.wait(10) - execution.advisory_unlock + job.advisory_unlock end locked_event.wait(10) - expect(execution.advisory_locked?).to be true + expect(job.advisory_locked?).to be true commit_event.set committed_event.wait(10) - expect(execution.advisory_locked?).to be true + expect(job.advisory_locked?).to be true done_event.set promise.value! @@ -450,8 +458,8 @@ done_event = Concurrent::Event.new promise = rails_promise do - execution.class.transaction do - execution.advisory_lock(function: "pg_advisory_xact_lock") + job.class.transaction do + job.advisory_lock(function: "pg_advisory_xact_lock") locked_event.set commit_event.wait(10) @@ -462,11 +470,11 @@ end locked_event.wait(10) - expect(execution.advisory_locked?).to be true + expect(job.advisory_locked?).to be true commit_event.set committed_event.wait(10) - expect(execution.advisory_locked?).to be false + expect(job.advisory_locked?).to be false done_event.set promise.value! diff --git a/spec/app/models/concerns/good_job/filterable_spec.rb b/spec/app/models/concerns/good_job/filterable_spec.rb index 5c6cbfa44..be812b229 100644 --- a/spec/app/models/concerns/good_job/filterable_spec.rb +++ b/spec/app/models/concerns/good_job/filterable_spec.rb @@ -5,12 +5,15 @@ RSpec.describe GoodJob::Filterable do let(:model_class) { GoodJob::Job } let!(:job) do - model_class.create( + model_class.create!( active_job_id: SecureRandom.uuid, queue_name: "default", + job_class: "ExampleJob", + scheduled_at: Time.current, serialized_params: { example_key: 'example_value' }, labels: %w[buffalo gopher], - error: "ExampleJob::ExampleError: a message" + error: "ExampleJob::ExampleError: a message", + error_event: GoodJob::ErrorEvents::ERROR_EVENT_RETRIED ) end diff --git a/spec/app/models/good_job/batch_spec.rb b/spec/app/models/good_job/batch_spec.rb index 1ff1ad32c..df117e07e 100644 --- a/spec/app/models/good_job/batch_spec.rb +++ b/spec/app/models/good_job/batch_spec.rb @@ -129,7 +129,7 @@ def perform(batch, params) end it 'can serialize GlobalId objects' do - globalid = GoodJob::Execution.create! + globalid = GoodJob::Job.create! batch = described_class.new batch.save diff --git a/spec/app/models/good_job/cron_entry_spec.rb b/spec/app/models/good_job/cron_entry_spec.rb index 4215d5bb5..e356ebc48 100644 --- a/spec/app/models/good_job/cron_entry_spec.rb +++ b/spec/app/models/good_job/cron_entry_spec.rb @@ -175,9 +175,9 @@ def perform(meaning, name:) cron_at = 10.minutes.ago entry.enqueue(cron_at) - execution = GoodJob::Execution.last - expect(execution.cron_key).to eq 'test' - expect(execution.cron_at).to be_within(0.001.seconds).of(cron_at) + job = GoodJob::Job.last + expect(job.cron_key).to eq 'test' + expect(job.cron_at).to be_within(0.001.seconds).of(cron_at) end end diff --git a/spec/app/models/good_job/execution_spec.rb b/spec/app/models/good_job/execution_spec.rb deleted file mode 100644 index 2604ac640..000000000 --- a/spec/app/models/good_job/execution_spec.rb +++ /dev/null @@ -1,833 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -RSpec.describe GoodJob::Execution do - let(:process_id) { SecureRandom.uuid } - - around do |example| - Rails.application.executor.wrap { example.run } - end - - before do - allow(described_class).to receive(:discrete_support?).and_return(false) - - stub_const "RUN_JOBS", Concurrent::Array.new - stub_const 'TestJob', (Class.new(ActiveJob::Base) do - self.queue_name = 'test' - self.priority = 50 - - def perform(result_value = nil, raise_error: false) - RUN_JOBS << provider_job_id - raise TestJob::ExpectedError, "Raised expected error" if raise_error - - result_value - end - end) - stub_const 'TestJob::ExpectedError', Class.new(StandardError) - end - - describe 'implicit sort order' do - it 'is by created at' do - first_job = described_class.create(active_job_id: '67160140-1bec-4c3b-bc34-1a8b36f87b21') - described_class.create(active_job_id: '3732d706-fd5a-4c39-b1a5-a9bc6d265811') - last_job = described_class.create(active_job_id: '4fbae77c-6f22-488f-ad42-5bd20f39c28c') - - result = described_class.all - - expect(result.first).to eq first_job - expect(result.last).to eq last_job - end - end - - describe '.enqueue' do - let(:active_job) { TestJob.new } - - context 'when discrete' do - before do - allow(described_class).to receive(:discrete_support?).and_return(true) - end - - it 'assigns is discrete, id, scheduled_at' do - expect { described_class.enqueue(active_job) }.to change(described_class, :count).by(1) - - execution = described_class.last - expect(execution).to have_attributes( - is_discrete: true, - id: active_job.job_id, - active_job_id: active_job.job_id, - created_at: execution.scheduled_at, - scheduled_at: execution.created_at - ) - end - end - - context 'when NOT discrete' do - before { allow(described_class).to receive(:discrete_support?).and_return(false) } - - it 'does not assign id, scheduled_at' do - expect { described_class.enqueue(active_job) }.to change(described_class, :count).by(1) - - execution = described_class.last - expect(execution.id).not_to eq(active_job.job_id) - expect(execution).to have_attributes( - is_discrete: nil, - active_job_id: active_job.job_id, - scheduled_at: nil - ) - end - end - - it 'creates a new GoodJob record' do - execution = nil - - expect do - execution = described_class.enqueue(active_job) - end.to change(described_class, :count).by(1) - - expect(execution).to have_attributes( - active_job_id: a_kind_of(String), - serialized_params: a_kind_of(Hash), - queue_name: 'test', - priority: 50, - scheduled_at: nil - ) - end - - it 'is schedulable' do - execution = described_class.enqueue(active_job, scheduled_at: 1.day.from_now) - expect(execution).to have_attributes( - scheduled_at: within(1.second).of(1.day.from_now) - ) - end - - it 'can be created with an advisory lock' do - unlocked_execution = described_class.enqueue(active_job) - expect(unlocked_execution.advisory_locked?).to be false - - locked_execution = described_class.enqueue(active_job, create_with_advisory_lock: true) - expect(locked_execution.advisory_locked?).to be true - - locked_execution.advisory_unlock - end - - describe 'deprecation of higher is higher priority order, change to smaller is higher priority' do - before { allow(GoodJob.deprecator).to receive(:warn) } - - context 'when smaller_number_higher_priority is not set' do - before { allow(Rails.application.config).to receive(:good_job).and_return({}) } - - it 'does not warn when priority is not set' do - active_job.priority = nil - described_class.enqueue(active_job) - expect(GoodJob.deprecator).not_to have_received(:warn) - end - - it 'does warn when priority is set' do - active_job.priority = 50 - described_class.enqueue(active_job) - expect(GoodJob.deprecator).to have_received(:warn) - end - end - - context 'when smaller_number_higher_priority=true' do - before { allow(Rails.application.config).to receive(:good_job).and_return(smaller_number_is_higher_priority: true) } - - it 'does not warn' do - active_job.priority = 50 - described_class.enqueue(active_job) - expect(GoodJob.deprecator).not_to have_received(:warn) - end - end - - context 'when smaller_number_higher_priority=false' do - before { allow(Rails.application.config).to receive(:good_job).and_return(smaller_number_is_higher_priority: false) } - - it 'warns to ensure the upgrade is obvious' do - active_job.priority = 50 - described_class.enqueue(active_job) - expect(GoodJob.deprecator).to have_received(:warn) - end - end - end - end - - describe '.perform_with_advisory_lock' do - context 'with one job' do - let(:active_job) { TestJob.new('a string') } - let!(:good_job) { described_class.enqueue(active_job) } - - it 'performs one job' do - good_job_2 = described_class.create!(serialized_params: {}) - - described_class.perform_with_advisory_lock(lock_id: process_id) - - expect(good_job.reload.finished_at).to be_present - expect(good_job_2.reload.finished_at).to be_blank - end - - it 'returns the result or nil if not' do - result = described_class.perform_with_advisory_lock(lock_id: process_id) - - expect(result).to be_a GoodJob::ExecutionResult - expect(result.value).to eq 'a string' - expect(result.unhandled_error).to be_nil - - described_class.enqueue(TestJob.new(true, raise_error: true)) - errored_result = described_class.all.perform_with_advisory_lock(lock_id: process_id) - - expect(result).to be_a GoodJob::ExecutionResult - expect(errored_result.value).to be_nil - expect(errored_result.unhandled_error).to be_an TestJob::ExpectedError - end - end - - context 'with multiple jobs' do - def job_params - { active_job_id: SecureRandom.uuid, queue_name: "default", priority: 0, serialized_params: { job_class: "TestJob" } } - end - - let!(:older_job) { described_class.create!(job_params.merge(created_at: 10.minutes.ago)) } - let!(:newer_job) { described_class.create!(job_params.merge(created_at: 5.minutes.ago)) } - let!(:low_priority_job) { described_class.create!(job_params.merge(priority: 20)) } - let!(:high_priority_job) { described_class.create!(job_params.merge(priority: -20)) } - - it "orders by priority ascending and creation descending" do - 4.times do - described_class.perform_with_advisory_lock(lock_id: process_id) - end - expect(described_class.order(finished_at: :asc).to_a).to eq([ - high_priority_job, - older_job, - newer_job, - low_priority_job, - ]) - end - end - - context "with multiple jobs and ordered queues" do - def job_params - { active_job_id: SecureRandom.uuid, queue_name: "default", priority: 0, serialized_params: { job_class: "TestJob" } } - end - - let(:parsed_queues) { { include: %w{one two}, ordered_queues: true } } - let!(:queue_two_job) { described_class.create!(job_params.merge(queue_name: "two", created_at: 10.minutes.ago, priority: 100)) } - let!(:queue_one_job) { described_class.create!(job_params.merge(queue_name: "one", created_at: 1.minute.ago, priority: 1)) } - - it "orders by queue order" do - described_class.perform_with_advisory_lock(lock_id: process_id, parsed_queues: parsed_queues) do |execution| - expect(execution).to eq queue_one_job - end - described_class.perform_with_advisory_lock(lock_id: process_id, parsed_queues: parsed_queues) do |execution| - expect(execution).to eq queue_two_job - end - end - end - end - - describe '.queue_parser' do - it 'creates an intermediary hash' do - result = described_class.queue_parser('first,second') - expect(result).to eq({ - include: %w[first second], - }) - - result = described_class.queue_parser('-first,second') - expect(result).to eq({ - exclude: %w[first second], - }) - - result = described_class.queue_parser('') - expect(result).to eq({ - all: true, - }) - result = described_class.queue_parser('+first,second') - expect(result).to eq({ - include: %w[first second], - ordered_queues: true, - }) - end - end - - describe '.queue_string' do - it 'separates commas' do - query = described_class.queue_string('first,second') - expect(query.to_sql).to eq described_class.where(queue_name: %w[first second]).to_sql - end - - it 'excludes queues commas' do - query = described_class.queue_string('-first,second') - expect(query.to_sql).to eq described_class.where.not(queue_name: %w[first second]).or(described_class.where(queue_name: nil)).to_sql - end - - it 'accepts empty strings' do - query = described_class.queue_string('') - expect(query.to_sql).to eq described_class.all.to_sql - end - end - - describe '.queue_ordered' do - it "produces SQL to order by queue order" do - query_sql = described_class.queue_ordered(%w{one two three}).to_sql - expect(query_sql).to include( - "ORDER BY (CASE WHEN queue_name = 'one' THEN 0 WHEN queue_name = 'two' THEN 1 WHEN queue_name = 'three' THEN 2 ELSE 3 END)" - ) - end - end - - describe '.priority_ordered' do - let!(:small_priority_job) { described_class.create!(priority: -50) } - let!(:large_priority_job) { described_class.create!(priority: 50) } - - it 'smaller_number_is_higher_priority=true orders with smaller number being HIGHER priority' do - allow(Rails.application.config).to receive(:good_job).and_return({ smaller_number_is_higher_priority: true }) - expect(described_class.priority_ordered.pluck(:priority)).to eq([-50, 50]) - end - - it 'smaller_number_is_higher_priority=false orders with smaller priority being LOWER priority' do - allow(Rails.application.config).to receive(:good_job).and_return({ smaller_number_is_higher_priority: false }) - expect(described_class.priority_ordered.pluck(:priority)).to eq([50, -50]) - end - - it 'smaller_number_is_higher_priority=false orders with lower priority being LOWER priority' do - allow(Rails.application.config).to receive(:good_job).and_return({}) - expect(described_class.priority_ordered.pluck(:priority)).to eq([50, -50]) - end - end - - describe '.next_scheduled_at' do - let(:active_job) { TestJob.new } - - it 'returns an empty array when nothing is scheduled' do - expect(described_class.all.next_scheduled_at).to eq [] - end - - it 'returns previously scheduled and unscheduled jobs' do - described_class.enqueue(active_job, scheduled_at: 1.day.ago) - Timecop.travel 5.minutes.ago do - described_class.enqueue(active_job, scheduled_at: nil) - end - - expect(described_class.all.next_scheduled_at(now_limit: 5)).to contain_exactly( - within(2.seconds).of(1.day.ago), - within(2.seconds).of(5.minutes.ago) - ) - end - - it 'returns future scheduled jobs' do - 2.times do - described_class.enqueue(active_job, scheduled_at: 1.day.from_now) - end - - expect(described_class.all.next_scheduled_at(limit: 1)).to contain_exactly( - within(2.seconds).of(1.day.from_now) - ) - end - - it 'contains both past and future jobs' do - 2.times { described_class.enqueue(active_job, scheduled_at: 1.day.ago) } - 2.times { described_class.enqueue(active_job, scheduled_at: 1.day.from_now) } - - expect(described_class.all.next_scheduled_at(limit: 1, now_limit: 1)).to contain_exactly( - within(2.seconds).of(1.day.ago), - within(2.seconds).of(1.day.from_now) - ) - end - end - - describe '.display_all' do - let(:active_job) { TestJob.new } - - it 'does not return jobs after last scheduled at' do - described_class.enqueue(active_job, scheduled_at: '2021-05-14 09:33:16 +0200') - - expect(described_class.display_all(after_scheduled_at: Time.zone.parse('2021-05-14 09:33:16 +0200')).count).to eq(0) - end - - it 'does not return jobs after last scheduled at and job id' do - described_class.enqueue(active_job, scheduled_at: '2021-05-14 09:33:16 +0200') - job_id = described_class.last!.id - - expect( - described_class.display_all(after_scheduled_at: Time.zone.parse('2021-05-14 09:33:16 +0200'), after_id: job_id).count - ).to eq(0) - end - end - - describe '#executable?' do - let(:good_job) { described_class.create!(active_job_id: SecureRandom.uuid) } - - it 'is true when locked' do - good_job.with_advisory_lock do - expect(good_job.executable?).to be true - end - end - - it 'is false when job no longer exists' do - good_job.with_advisory_lock do - good_job.destroy! - expect(good_job.executable?).to be false - end - end - - it 'is false when the job has finished' do - good_job.with_advisory_lock do - good_job.update! finished_at: Time.current - expect(good_job.executable?).to be false - end - end - end - - describe '#perform' do - let(:active_job) { TestJob.new("a string") } - let!(:good_job) { described_class.enqueue(active_job) } - - describe 'return value' do - it 'returns the results of the job' do - result = good_job.perform(lock_id: process_id) - - expect(result.value).to eq "a string" - expect(result.unhandled_error).to be_nil - end - - context 'when there is an error' do - let(:active_job) { TestJob.new("whoops", raise_error: true) } - let(:batch_id) { SecureRandom.uuid } - - let!(:good_job) do - execution = nil - GoodJob::CurrentThread.within do - GoodJob::Batch.within_thread(batch_id: batch_id) do - GoodJob::CurrentThread.cron_key = 'test_key' - execution = described_class.enqueue(active_job) - end - end - - execution - end - - it 'returns the error' do - result = good_job.perform(lock_id: process_id) - - expect(result.value).to be_nil - expect(result.unhandled_error).to be_an_instance_of TestJob::ExpectedError - end - - context 'when there is a retry handler' do - before do - ActiveJob::Base.queue_adapter = GoodJob::Adapter.new(execution_mode: :inline) - allow(GoodJob).to receive(:preserve_job_records).and_return(true) - TestJob.retry_on(TestJob::ExpectedError, attempts: 2) - end - - it 'copies job info, including the cron key to the new record' do - new_record = described_class.order(created_at: :asc).last - expect(new_record.active_job_id).to eq good_job.active_job_id - expect(new_record.cron_key).to eq "test_key" - expect(new_record.batch_id).to eq batch_id - end - - it 'records the new job UUID on the executing record' do - good_job.perform(lock_id: process_id) - expect(good_job.reload.retried_good_job_id).to be_present - end - end - - context 'when there is an retry handler with exhausted attempts' do - before do - TestJob.retry_on(TestJob::ExpectedError, attempts: 1) - - good_job.serialized_params["exception_executions"] = { "[TestJob::ExpectedError]" => 1 } - good_job.save! - end - - it 'does not modify the original good_job serialized params' do - allow(GoodJob).to receive(:preserve_job_records).and_return(true) - - expect do - good_job.perform(lock_id: process_id) - end.not_to change { good_job.reload.serialized_params["exception_executions"]["[TestJob::ExpectedError]"] } - end - end - - context 'when error monitoring service intercepts exception' do - before do - # Similar to Sentry monitor's implementation - # https://github.com/getsentry/raven-ruby/blob/20b260a6d04e0ca01d5cddbd9728e6fc8ae9a90c/lib/raven/integrations/rails/active_job.rb#L21-L31 - TestJob.around_perform do |_job, block| - block.call - rescue StandardError => e - next if rescue_with_handler(e) - - raise e - ensure - nil - end - end - - it 'returns the error' do - result = good_job.perform(lock_id: process_id) - - expect(result.value).to be_nil - expect(result.unhandled_error).to be_an_instance_of TestJob::ExpectedError - end - - context 'when retry_on is used' do - before do - TestJob.retry_on(StandardError, wait: 0, attempts: Float::INFINITY) { nil } - end - - it 'returns the error' do - result = good_job.perform(lock_id: process_id) - - expect(result.value).to be_nil - expect(result.handled_error).to be_an_instance_of TestJob::ExpectedError - end - end - - context 'when discard_on is used' do - before do - TestJob.discard_on(StandardError) { nil } - end - - it 'returns the error' do - result = good_job.perform(lock_id: process_id) - - expect(result.value).to be_nil - expect(result.handled_error).to be_an_instance_of TestJob::ExpectedError - end - end - end - end - end - - it 'preserves the job by default' do - good_job.perform(lock_id: process_id) - expect(good_job.reload).to have_attributes( - performed_at: within(1.second).of(Time.current), - finished_at: within(1.second).of(Time.current) - ) - end - - it 'can destroy the job when preserve_job_records is false' do - allow(GoodJob).to receive(:preserve_job_records).and_return(false) - good_job.perform(lock_id: process_id) - expect { good_job.reload }.to raise_error ActiveRecord::RecordNotFound - end - - it 'destroys the job when preserving record only on error' do - allow(GoodJob).to receive(:preserve_job_records).and_return(:on_unhandled_error) - good_job.perform(lock_id: process_id) - expect { good_job.reload }.to raise_error ActiveRecord::RecordNotFound - end - - context 'when there are prior executions' do - let!(:prior_execution) do - described_class.enqueue(active_job).tap do |job| - job.update!( - error: "TestJob::ExpectedError: Raised expected error", - performed_at: Time.current, - finished_at: Time.current - ) - end - end - - it 'destroys the job and prior executions when preserving record only on error' do - allow(GoodJob).to receive(:preserve_job_records).and_return(:on_unhandled_error) - good_job.perform(lock_id: process_id) - expect { good_job.reload }.to raise_error ActiveRecord::RecordNotFound - expect { prior_execution.reload }.to raise_error ActiveRecord::RecordNotFound - end - end - - context 'when the job is directly re-enqueued' do - before do - allow(GoodJob).to receive(:preserve_job_records).and_return(false) - TestJob.queue_adapter = GoodJob::Adapter.new(execution_mode: :inline) - TestJob.after_perform do - enqueue(wait: 1.minute) - end - end - - it 'does not destroy the execution records' do - good_job.perform(lock_id: process_id) - expect { good_job.reload }.not_to raise_error - expect(described_class.where(active_job_id: good_job.active_job_id).count).to eq 2 - end - end - - context 'when the job is a cron job and records are not preserved' do - before do - allow(GoodJob).to receive(:preserve_job_records).and_return(false) - TestJob.queue_adapter = GoodJob::Adapter.new(execution_mode: :inline) - good_job.update(cron_key: "test_key", cron_at: Time.current) - end - - it 'preserves the job record anyway' do - good_job.perform(lock_id: process_id) - expect(good_job.reload).to have_attributes( - performed_at: within(1.second).of(Time.current), - finished_at: within(1.second).of(Time.current) - ) - end - end - - it 'raises an error if the job is attempted to be re-run' do - good_job.update!(finished_at: Time.current) - expect { good_job.perform(lock_id: process_id) }.to raise_error described_class::PreviouslyPerformedError - end - - context 'when ActiveJob rescues an error' do - let(:active_job) { TestJob.new("a string", raise_error: true) } - - before do - TestJob.retry_on(StandardError, wait: 0, attempts: Float::INFINITY) { nil } - end - - it 'returns the results of the job' do - result = good_job.perform(lock_id: process_id) - - expect(result.value).to be_nil - expect(result.handled_error).to be_a(TestJob::ExpectedError) - end - - it 'can preserves the job' do - allow(GoodJob).to receive(:preserve_job_records).and_return(true) - - good_job.perform(lock_id: process_id) - - expect(good_job.reload).to have_attributes( - error: "TestJob::ExpectedError: Raised expected error", - performed_at: within(1.second).of(Time.current), - finished_at: within(1.second).of(Time.current) - ) - end - end - - context 'when ActiveJob raises an error' do - let(:active_job) { TestJob.new("a string", raise_error: true) } - - it 'returns the results of the job' do - result = good_job.perform(lock_id: process_id) - - expect(result.value).to be_nil - expect(result.unhandled_error).to be_a(TestJob::ExpectedError) - end - - describe 'GoodJob.retry_on_unhandled_error behavior' do - context 'when true' do - before do - allow(GoodJob).to receive(:retry_on_unhandled_error).and_return(true) - end - - it 'leaves the job record unfinished' do - allow(GoodJob).to receive(:preserve_job_records).and_return(true) - - good_job.perform(lock_id: process_id) - - expect(good_job.reload).to have_attributes( - error: "TestJob::ExpectedError: Raised expected error", - performed_at: within(1.second).of(Time.current), - finished_at: nil - ) - end - - it 'does not destroy the job record' do - allow(GoodJob).to receive(:preserve_job_records).and_return(false) - - good_job.perform(lock_id: process_id) - expect { good_job.reload }.not_to raise_error - end - end - - context 'when false' do - before do - allow(GoodJob).to receive(:retry_on_unhandled_error).and_return(false) - end - - it 'destroys the job' do - allow(GoodJob).to receive(:preserve_job_records).and_return(false) - - good_job.perform(lock_id: process_id) - expect { good_job.reload }.to raise_error ActiveRecord::RecordNotFound - end - - it 'can preserve the job' do - allow(GoodJob).to receive(:preserve_job_records).and_return(true) - - good_job.perform(lock_id: process_id) - - expect(good_job.reload).to have_attributes( - error: "TestJob::ExpectedError: Raised expected error", - performed_at: within(1.second).of(Time.current), - finished_at: within(1.second).of(Time.current) - ) - end - - it 'preserves the job when preserving record only on error' do - allow(GoodJob).to receive(:preserve_job_records).and_return(:on_unhandled_error) - good_job.perform(lock_id: process_id) - - expect(good_job.reload).to have_attributes( - error: "TestJob::ExpectedError: Raised expected error", - performed_at: within(1.second).of(Time.current), - finished_at: within(1.second).of(Time.current) - ) - end - end - end - end - - context 'when Discrete' do - before do - ActiveJob::Base.queue_adapter = GoodJob::Adapter.new(execution_mode: :inline) - allow(described_class).to receive(:discrete_support?).and_return(true) - good_job.update!(is_discrete: true) - end - - it 'updates the Execution record and creates a DiscreteExecution record' do - good_job.perform(lock_id: process_id) - - expect(good_job.reload).to have_attributes( - executions_count: 1, - finished_at: within(1.second).of(Time.current) - ) - - dexecution = good_job.discrete_executions.first - expect(dexecution).to be_present - expect(dexecution).to have_attributes( - active_job_id: good_job.active_job_id, - job_class: good_job.job_class, - queue_name: good_job.queue_name, - created_at: within(0.001).of(good_job.performed_at), - scheduled_at: within(0.001).of(good_job.created_at), - finished_at: within(1.second).of(Time.current), - duration: GoodJob::DiscreteExecution.duration_interval_usable? ? be_present : nil, - error: nil, - serialized_params: good_job.serialized_params - ) - end - - context 'when ActiveJob rescues an error' do - let(:active_job) { TestJob.new("a string", raise_error: true) } - let!(:good_job) { described_class.enqueue(active_job) } - - before do - allow(described_class).to receive(:discrete_support?).and_return(true) - allow(GoodJob).to receive(:preserve_job_records).and_return(true) - TestJob.retry_on(StandardError, wait: 1.hour, attempts: 2) { nil } - good_job.update!(is_discrete: true) - end - - it 'updates the existing Execution/Job record instead of creating a new one' do - expect { good_job.perform(lock_id: process_id) } - .to not_change(described_class, :count) - .and change { good_job.reload.serialized_params["executions"] }.by(1) - .and not_change { good_job.reload.id } - .and not_change { described_class.count } - - expect(good_job.reload).to have_attributes( - error: "TestJob::ExpectedError: Raised expected error", - created_at: within(1.second).of(Time.current), - performed_at: nil, - finished_at: nil, - scheduled_at: within(10.minutes).of(1.hour.from_now) # interval because of retry jitter - ) - expect(GoodJob::DiscreteExecution.count).to eq(1) - discrete_execution = good_job.discrete_executions.first - expect(discrete_execution).to have_attributes( - active_job_id: good_job.active_job_id, - error: "TestJob::ExpectedError: Raised expected error", - created_at: within(1.second).of(Time.current), - scheduled_at: within(1.second).of(Time.current), - finished_at: within(1.second).of(Time.current), - duration: GoodJob::DiscreteExecution.duration_interval_usable? ? be_present : nil - ) - end - end - - context 'when retry_job is invoked directly during execution' do - before do - TestJob.after_perform do |job| - job.retry_job wait: 1.second - end - end - - it 'finishes the execution but does not finish the job' do - good_job.perform(lock_id: process_id) - - expect(good_job.reload).to have_attributes( - performed_at: nil, - finished_at: nil, - scheduled_at: within(0.5).of(1.second.from_now) - ) - - expect(good_job.discrete_executions.size).to eq(1) - expect(good_job.discrete_executions.first).to have_attributes( - performed_at: within(1.second).of(Time.current), - finished_at: within(1.second).of(Time.current), - duration: GoodJob::DiscreteExecution.duration_interval_usable? ? be_present : nil - ) - end - end - end - end - - describe '#destroy_job' do - let!(:execution) { described_class.create! active_job_id: SecureRandom.uuid } - let!(:prior_execution) { described_class.create! active_job_id: execution.active_job_id } - let!(:other_job) { described_class.create! active_job_id: SecureRandom.uuid } - - it 'destroys all of the job executions' do - execution.destroy_job - expect { execution.reload }.to raise_error ActiveRecord::RecordNotFound - expect { prior_execution.reload }.to raise_error ActiveRecord::RecordNotFound - expect { other_job.reload }.not_to raise_error - end - end - - describe '#queue_latency' do - let(:execution) { described_class.create! } - - it 'is nil for future scheduled execution' do - execution.scheduled_at = 1.minute.from_now - expect(execution.queue_latency).to be_nil - end - - it 'is distance between scheduled_at and now for past scheduled job' do - execution.scheduled_at = 1.minute.ago - expect(execution.queue_latency).to be_within(0.1).of(Time.zone.now - execution.scheduled_at) - end - - it 'is distance between created_at and now for queued job' do - execution.scheduled_at = nil - expect(execution.queue_latency).to be_within(0.1).of(Time.zone.now - execution.created_at) - end - - it 'is distance between created_at and performed_at for started job' do - execution.scheduled_at = nil - execution.performed_at = 10.seconds.ago - expect(execution.queue_latency).to eq(execution.performed_at - execution.created_at) - end - end - - describe "#runtime_latency" do - let(:execution) { described_class.create! } - - it 'is nil for queued job' do - expect(execution.runtime_latency).to be_nil - end - - it 'is distance between performed_at and now for started job' do - execution.performed_at = 10.seconds.ago - execution.finished_at = nil - expect(execution.runtime_latency).to be_within(0.1).of(Time.zone.now - execution.performed_at) - end - - it 'is distance between performed_at and finished_at' do - execution.performed_at = 5.seconds.ago - execution.finished_at = 1.second.ago - expect(execution.runtime_latency).to eq(execution.finished_at - execution.performed_at) - end - end -end diff --git a/spec/app/models/good_job/job_spec.rb b/spec/app/models/good_job/job_spec.rb index 5e2664396..e2e675b96 100644 --- a/spec/app/models/good_job/job_spec.rb +++ b/spec/app/models/good_job/job_spec.rb @@ -7,11 +7,12 @@ let(:job) do described_class.create!( - is_discrete: true, + id: active_job_id, active_job_id: active_job_id, scheduled_at: 10.minutes.from_now, queue_name: 'mice', priority: 10, + job_class: "TestJob", serialized_params: { 'job_id' => active_job_id, 'job_class' => 'TestJob', @@ -31,48 +32,6 @@ ) end end - let(:undiscrete_job) { described_class.find(head_execution.active_job_id) } - - let(:tail_execution) do - GoodJob::Execution.create!( - active_job_id: active_job_id, - created_at: 1.minute.ago, - queue_name: 'mice', - priority: 10, - serialized_params: { - 'job_id' => active_job_id, - 'job_class' => 'TestJob', - 'executions' => 0, - 'queue_name' => 'mice', - 'priority' => 10, - 'arguments' => ['cat', { 'canine' => 'dog' }], - } - ) - end - - let(:head_execution) do - GoodJob::Execution.create!( - active_job_id: tail_execution.active_job_id, - scheduled_at: 10.minutes.from_now, - queue_name: 'mice', - priority: 10, - serialized_params: { - 'job_id' => tail_execution.active_job_id, - 'job_class' => 'TestJob', - 'executions' => 1, - 'exception_executions' => { 'TestJob::Error' => 1 }, - 'queue_name' => 'mice', - 'priority' => 10, - 'arguments' => ['cat', { 'canine' => 'dog' }], - } - ).tap do |execution| - tail_execution.update!( - retried_good_job_id: execution.id, - error: "TestJob::Error: TestJob::Error", - finished_at: execution.created_at - ) - end - end before do allow(GoodJob).to receive(:preserve_job_records).and_return(true) @@ -87,13 +46,6 @@ def perform(*) stub_const 'TestJob::Error', Class.new(StandardError) end - describe '.find' do - it 'returns a record that is the same as the head execution' do - job = described_class.find(head_execution.active_job_id) - expect(job.executions.last).to eq head_execution - end - end - describe '#id' do it 'is the ActiveJob ID' do expect(job.id).to eq job.active_job_id @@ -113,50 +65,6 @@ def perform(*) end end - describe '#job_class' do - it 'is the job class' do - expect(job.id).to eq head_execution.active_job_id - end - end - - describe '#reload' do - it 'reloads the job execution' do - job = undiscrete_job - original_head_execution = job.head_execution - - new_execution = GoodJob::Execution.create!( - active_job_id: job.active_job_id, - queue_name: 'newnewnew' - ) - original_head_execution.update!(retried_good_job_id: new_execution.id) - - expect do - job.reload - end.to change { job.queue_name }.from('mice').to('newnewnew') - end - end - - describe '#head_execution' do - it 'is the head execution (which should be the same record)' do - job = undiscrete_job - expect(job.head_execution).to eq head_execution - expect(job._execution_id).to eq head_execution.id - end - end - - describe '#tail_execution' do - it 'is the tail execution' do - job = undiscrete_job - expect(job.tail_execution).to eq tail_execution - end - end - - describe '#recent_error' do - it 'is the current executions error or the previous jobs error' do - expect(job.recent_error).to eq tail_execution.error - end - end - describe '#running?' do context 'when advisory_locks are NOT eagerloaded' do it 'is true if the job is Advisory Locked' do @@ -229,8 +137,6 @@ def perform(*) end it 'updates the original job' do - expect(job).to be_discrete - expect do job.retry_job end.to change { job.reload.finished? }.from(true).to(false) @@ -242,37 +148,6 @@ def perform(*) ) end - context 'when job is not discrete' do - let(:job) { undiscrete_job } - - before do - head_execution.update!( - finished_at: Time.current, - error: "TestJob::Error: TestJob::Error" - ) - end - - it 'enqueues another execution and updates the original job' do - original_head_execution = undiscrete_job.head_execution - - expect do - job.retry_job - end.to change { job.executions.reload.size }.by(1) - expect(job.reload).not_to be_finished - - new_head_execution = job.head_execution(reload: true) - expect(new_head_execution.serialized_params).to include( - "executions" => 2, - "queue_name" => "mice", - "priority" => 10, - "arguments" => ['cat', hash_including('canine' => 'dog')] - ) - - original_head_execution.reload - expect(original_head_execution.retried_good_job_id).to eq new_head_execution.id - end - end - context 'when run inline' do before do stub_const "TestJob", (Class.new(ActiveJob::Base) do @@ -290,8 +165,8 @@ def perform(*) job.retry_job expect(job).to be_finished - executions = job.discrete_executions.order(:created_at).to_a + executions = job.discrete_executions.order(created_at: :asc).to_a expect(executions.size).to eq 3 # initial execution isn't created in test expect(executions.map(&:error)).to eq ["TestJob::Error: TestJob::Error", "TestJob::Error: TestJob::Error", nil] expect(executions[0].finished_at).to be < executions[1].finished_at @@ -354,7 +229,7 @@ def perform(*) job.discard_job("Discarded in test") end.to change { job.reload.status }.from(:scheduled).to(:discarded) - expect(job.head_execution(reload: true)).to have_attributes( + expect(job.reload).to have_attributes( error: "GoodJob::Job::DiscardJobError: Discarded in test", error_event: "discarded", finished_at: within(1.second).of(Time.current) @@ -364,7 +239,7 @@ def perform(*) context 'when a job is not in scheduled/queued state' do before do - job.head_execution.update! finished_at: Time.current + job.update! finished_at: Time.current end it 'raises an ActionForStateMismatchError' do @@ -442,7 +317,7 @@ def perform(*) job.reschedule_job end.to change { job.reload.status }.from(:scheduled).to(:queued) - expect(job.head_execution(reload: true)).to have_attributes( + expect(job.reload).to have_attributes( scheduled_at: within(1.second).of(Time.current) ) end @@ -450,7 +325,7 @@ def perform(*) context 'when a job is not in scheduled/queued state' do before do - job.head_execution.update! finished_at: Time.current + job.update! finished_at: Time.current end it 'raises an ActionForStateMismatchError' do @@ -474,29 +349,743 @@ def perform(*) expect { job.destroy_job }.to raise_error GoodJob::Job::ActionForStateMismatchError end end + end + + describe "behavior adopted from v3 Execution" do + let(:process_id) { SecureRandom.uuid } + + around do |example| + Rails.application.executor.wrap { example.run } + end + + before do + allow(described_class).to receive(:discrete_support?).and_return(false) + + stub_const "RUN_JOBS", Concurrent::Array.new + stub_const 'TestJob', (Class.new(ActiveJob::Base) do + self.queue_name = 'test' + self.priority = 50 + + def perform(result_value = nil, raise_error: false) + RUN_JOBS << provider_job_id + raise TestJob::ExpectedError, "Raised expected error" if raise_error + + result_value + end + end) + stub_const 'TestJob::ExpectedError', Class.new(StandardError) + end + + describe 'implicit sort order' do + it 'is by created at' do + first_job = described_class.create(active_job_id: '67160140-1bec-4c3b-bc34-1a8b36f87b21') + described_class.create(active_job_id: '3732d706-fd5a-4c39-b1a5-a9bc6d265811') + last_job = described_class.create(active_job_id: '4fbae77c-6f22-488f-ad42-5bd20f39c28c') + + result = described_class.all + + expect(result.first).to eq first_job + expect(result.last).to eq last_job + end + end + + describe '.enqueue' do + let(:active_job) { TestJob.new } + + context 'when discrete' do + it 'assigns is discrete, id, scheduled_at' do + expect { described_class.enqueue(active_job) }.to change(described_class, :count).by(1) + + job = described_class.last + expect(job).to have_attributes( + id: active_job.job_id, + active_job_id: active_job.job_id, + created_at: within(1.second).of(Time.current), + scheduled_at: job.created_at + ) + end + end + + it 'creates a new GoodJob record' do + job = nil + + expect do + job = described_class.enqueue(active_job) + end.to change(described_class, :count).by(1) + + expect(job).to have_attributes( + active_job_id: job.id, + serialized_params: a_kind_of(Hash), + queue_name: 'test', + priority: 50, + scheduled_at: job.created_at + ) + end + + it 'is schedulable' do + execution = described_class.enqueue(active_job, scheduled_at: 1.day.from_now) + expect(execution).to have_attributes( + scheduled_at: within(1.second).of(1.day.from_now) + ) + end + + it 'can be created with an advisory lock' do + unlocked_execution = described_class.enqueue(TestJob.new) + expect(unlocked_execution.advisory_locked?).to be false + + locked_execution = described_class.enqueue(TestJob.new, create_with_advisory_lock: true) + expect(locked_execution.advisory_locked?).to be true + + locked_execution.advisory_unlock + end + end + + describe '.perform_with_advisory_lock' do + context 'with one job' do + let(:active_job) { TestJob.new('a string') } + let!(:good_job) { described_class.enqueue(active_job) } + + it 'performs one job' do + good_job_2 = described_class.create!(active_job_id: SecureRandom.uuid, serialized_params: {}) + + described_class.perform_with_advisory_lock(lock_id: process_id) + + expect(good_job.reload.finished_at).to be_present + expect(good_job_2.reload.finished_at).to be_blank + end + + it 'returns the result or nil if not' do + result = described_class.perform_with_advisory_lock(lock_id: process_id) + + expect(result).to be_a GoodJob::ExecutionResult + expect(result.value).to eq 'a string' + expect(result.unhandled_error).to be_nil + + described_class.enqueue(TestJob.new(true, raise_error: true)) + errored_result = described_class.all.perform_with_advisory_lock(lock_id: process_id) + + expect(result).to be_a GoodJob::ExecutionResult + expect(errored_result.value).to be_nil + expect(errored_result.unhandled_error).to be_an TestJob::ExpectedError + end + end + + context 'with multiple jobs' do + def job_params + { + active_job_id: SecureRandom.uuid, + queue_name: "default", + priority: 0, + job_class: "TestJob", + scheduled_at: Time.current, + serialized_params: { job_class: "TestJob" }, + } + end + + let!(:older_job) { described_class.create!(job_params.merge(created_at: 10.minutes.ago)) } + let!(:newer_job) { described_class.create!(job_params.merge(created_at: 5.minutes.ago)) } + let!(:low_priority_job) { described_class.create!(job_params.merge(priority: 20)) } + let!(:high_priority_job) { described_class.create!(job_params.merge(priority: -20)) } + + it "orders by priority ascending and creation descending" do + 4.times do + described_class.perform_with_advisory_lock(lock_id: process_id) + end + expect(described_class.order(finished_at: :asc).to_a).to eq([ + high_priority_job, + older_job, + newer_job, + low_priority_job, + ]) + end + end + + context "with multiple jobs and ordered queues" do + def job_params + { active_job_id: SecureRandom.uuid, queue_name: "default", priority: 0, serialized_params: { job_class: "TestJob" } } + end + + let(:parsed_queues) { { include: %w{one two}, ordered_queues: true } } + let!(:queue_two_job) { described_class.create!(job_params.merge(queue_name: "two", created_at: 10.minutes.ago, priority: 100)) } + let!(:queue_one_job) { described_class.create!(job_params.merge(queue_name: "one", created_at: 1.minute.ago, priority: 1)) } + + it "orders by queue order" do + described_class.perform_with_advisory_lock(lock_id: process_id, parsed_queues: parsed_queues) do |job| + expect(job).to eq queue_one_job + end + described_class.perform_with_advisory_lock(lock_id: process_id, parsed_queues: parsed_queues) do |job| + expect(job).to eq queue_two_job + end + end + end + end + + describe '.queue_parser' do + it 'creates an intermediary hash' do + result = described_class.queue_parser('first,second') + expect(result).to eq({ + include: %w[first second], + }) + + result = described_class.queue_parser('-first,second') + expect(result).to eq({ + exclude: %w[first second], + }) + + result = described_class.queue_parser('') + expect(result).to eq({ + all: true, + }) + result = described_class.queue_parser('+first,second') + expect(result).to eq({ + include: %w[first second], + ordered_queues: true, + }) + end + end + + describe '.queue_string' do + it 'separates commas' do + query = described_class.queue_string('first,second') + expect(query.to_sql).to eq described_class.where(queue_name: %w[first second]).to_sql + end + + it 'excludes queues commas' do + query = described_class.queue_string('-first,second') + expect(query.to_sql).to eq described_class.where.not(queue_name: %w[first second]).or(described_class.where(queue_name: nil)).to_sql + end + + it 'accepts empty strings' do + query = described_class.queue_string('') + expect(query.to_sql).to eq described_class.all.to_sql + end + end + + describe '.queue_ordered' do + it "produces SQL to order by queue order" do + query_sql = described_class.queue_ordered(%w{one two three}).to_sql + expect(query_sql).to include( + "ORDER BY (CASE WHEN queue_name = 'one' THEN 0 WHEN queue_name = 'two' THEN 1 WHEN queue_name = 'three' THEN 2 ELSE 3 END)" + ) + end + end + + describe '.priority_ordered' do + let!(:small_priority_job) { described_class.create!(priority: -50) } + let!(:large_priority_job) { described_class.create!(priority: 50) } + + it 'smaller_number_is_higher_priority=true orders with smaller number being HIGHER priority' do + allow(Rails.application.config).to receive(:good_job).and_return({ smaller_number_is_higher_priority: true }) + expect(described_class.priority_ordered.pluck(:priority)).to eq([-50, 50]) + end + + it 'smaller_number_is_higher_priority=false orders with smaller priority being LOWER priority' do + allow(Rails.application.config).to receive(:good_job).and_return({ smaller_number_is_higher_priority: false }) + expect(described_class.priority_ordered.pluck(:priority)).to eq([50, -50]) + end + + it 'smaller_number_is_higher_priority=false orders with lower priority being LOWER priority' do + allow(Rails.application.config).to receive(:good_job).and_return({}) + expect(described_class.priority_ordered.pluck(:priority)).to eq([50, -50]) + end + end + + describe '.next_scheduled_at' do + let(:active_job) { TestJob.new } + + it 'returns an empty array when nothing is scheduled' do + expect(described_class.all.next_scheduled_at).to eq [] + end + + it 'returns previously scheduled and unscheduled jobs' do + described_class.enqueue(TestJob.new, scheduled_at: 1.day.ago) + Timecop.travel 5.minutes.ago do + described_class.enqueue(TestJob.new, scheduled_at: nil) + end + + expect(described_class.all.next_scheduled_at(now_limit: 5)).to contain_exactly( + within(2.seconds).of(1.day.ago), + within(2.seconds).of(5.minutes.ago) + ) + end + + it 'returns future scheduled jobs' do + 2.times do + described_class.enqueue(TestJob.new, scheduled_at: 1.day.from_now) + end + + expect(described_class.all.next_scheduled_at(limit: 1)).to contain_exactly( + within(2.seconds).of(1.day.from_now) + ) + end + + it 'contains both past and future jobs' do + 2.times { described_class.enqueue(TestJob.new, scheduled_at: 1.day.ago) } + 2.times { described_class.enqueue(TestJob.new, scheduled_at: 1.day.from_now) } + + expect(described_class.all.next_scheduled_at(limit: 1, now_limit: 1)).to contain_exactly( + within(2.seconds).of(1.day.ago), + within(2.seconds).of(1.day.from_now) + ) + end + end + + describe '.display_all' do + let(:active_job) { TestJob.new } + + it 'does not return jobs after last scheduled at' do + described_class.enqueue(active_job, scheduled_at: '2021-05-14 09:33:16 +0200') + + expect(described_class.display_all(after_scheduled_at: Time.zone.parse('2021-05-14 09:33:16 +0200')).count).to eq(0) + end + + it 'does not return jobs after last scheduled at and job id' do + described_class.enqueue(active_job, scheduled_at: '2021-05-14 09:33:16 +0200') + job_id = described_class.last!.id + + expect( + described_class.display_all(after_scheduled_at: Time.zone.parse('2021-05-14 09:33:16 +0200'), after_id: job_id).count + ).to eq(0) + end + end + + describe '#executable?' do + let(:good_job) { described_class.create!(active_job_id: SecureRandom.uuid) } + + it 'is true when locked' do + good_job.with_advisory_lock do + expect(good_job.executable?).to be true + end + end + + it 'is false when job no longer exists' do + good_job.with_advisory_lock do + good_job.destroy! + expect(good_job.executable?).to be false + end + end + + it 'is false when the job has finished' do + good_job.with_advisory_lock do + good_job.update! finished_at: Time.current + expect(good_job.executable?).to be false + end + end + end + + describe '#perform' do + let(:active_job) { TestJob.new("a string") } + let!(:good_job) { described_class.enqueue(active_job) } + + describe 'return value' do + it 'returns the results of the job' do + result = good_job.perform(lock_id: process_id) + + expect(result.value).to eq "a string" + expect(result.unhandled_error).to be_nil + end + + context 'when there is an error' do + let(:active_job) { TestJob.new("whoops", raise_error: true) } + let(:batch_id) { SecureRandom.uuid } + + let!(:good_job) do + execution = nil + GoodJob::CurrentThread.within do + GoodJob::Batch.within_thread(batch_id: batch_id) do + GoodJob::CurrentThread.cron_key = 'test_key' + execution = described_class.enqueue(active_job) + end + end + + execution + end + + it 'returns the error' do + result = good_job.perform(lock_id: process_id) + + expect(result.value).to be_nil + expect(result.unhandled_error).to be_an_instance_of TestJob::ExpectedError + end + + context 'when there is a retry handler' do + before do + ActiveJob::Base.queue_adapter = GoodJob::Adapter.new(execution_mode: :inline) + allow(GoodJob).to receive(:preserve_job_records).and_return(true) + TestJob.retry_on(TestJob::ExpectedError, attempts: 2) + end + + it 'copies job info, including the cron key to the new record' do + new_record = described_class.order(created_at: :asc).last + expect(new_record.active_job_id).to eq good_job.active_job_id + expect(new_record.cron_key).to eq "test_key" + expect(new_record.batch_id).to eq batch_id + end + end + + context 'when there is an retry handler with exhausted attempts' do + before do + TestJob.retry_on(TestJob::ExpectedError, attempts: 1) + + good_job.serialized_params["exception_executions"] = { "[TestJob::ExpectedError]" => 1 } + good_job.save! + end + + it 'does not modify the original good_job serialized params' do + allow(GoodJob).to receive(:preserve_job_records).and_return(true) + + expect do + good_job.perform(lock_id: process_id) + end.not_to change { good_job.reload.serialized_params["exception_executions"]["[TestJob::ExpectedError]"] } + end + end + + context 'when error monitoring service intercepts exception' do + before do + # Similar to Sentry monitor's implementation + # https://github.com/getsentry/raven-ruby/blob/20b260a6d04e0ca01d5cddbd9728e6fc8ae9a90c/lib/raven/integrations/rails/active_job.rb#L21-L31 + TestJob.around_perform do |_job, block| + block.call + rescue StandardError => e + next if rescue_with_handler(e) + + raise e + ensure + nil + end + end + + it 'returns the error' do + result = good_job.perform(lock_id: process_id) + + expect(result.value).to be_nil + expect(result.unhandled_error).to be_an_instance_of TestJob::ExpectedError + end + + context 'when retry_on is used' do + before do + TestJob.retry_on(StandardError, wait: 0, attempts: Float::INFINITY) { nil } + end + + it 'returns the error' do + result = good_job.perform(lock_id: process_id) + + expect(result.value).to be_nil + expect(result.handled_error).to be_an_instance_of TestJob::ExpectedError + end + end + + context 'when discard_on is used' do + before do + TestJob.discard_on(StandardError) { nil } + end + + it 'returns the error' do + result = good_job.perform(lock_id: process_id) + + expect(result.value).to be_nil + expect(result.handled_error).to be_an_instance_of TestJob::ExpectedError + end + end + end + end + end + + it 'preserves the job by default' do + good_job.perform(lock_id: process_id) + expect(good_job.reload).to have_attributes( + performed_at: within(1.second).of(Time.current), + finished_at: within(1.second).of(Time.current) + ) + end + + it 'can destroy the job when preserve_job_records is false' do + allow(GoodJob).to receive(:preserve_job_records).and_return(false) + good_job.perform(lock_id: process_id) + expect { good_job.reload }.to raise_error ActiveRecord::RecordNotFound + end + + it 'destroys the job when preserving record only on error' do + allow(GoodJob).to receive(:preserve_job_records).and_return(:on_unhandled_error) + good_job.perform(lock_id: process_id) + expect { good_job.reload }.to raise_error ActiveRecord::RecordNotFound + end + + context 'when the job is directly re-enqueued' do + before do + allow(GoodJob).to receive(:preserve_job_records).and_return(false) + TestJob.queue_adapter = GoodJob::Adapter.new(execution_mode: :inline) + TestJob.after_perform do + enqueue(wait: 1.minute) + end + end + + it 'does not destroy the execution records' do + good_job.perform(lock_id: process_id) + expect { good_job.reload }.not_to raise_error + end + end + + context 'when the job is a cron job and records are not preserved' do + before do + allow(GoodJob).to receive(:preserve_job_records).and_return(false) + TestJob.queue_adapter = GoodJob::Adapter.new(execution_mode: :inline) + good_job.update(cron_key: "test_key", cron_at: Time.current) + end + + it 'preserves the job record anyway' do + good_job.perform(lock_id: process_id) + expect(good_job.reload).to have_attributes( + performed_at: within(1.second).of(Time.current), + finished_at: within(1.second).of(Time.current) + ) + end + end - context "when undiscrete job" do - let(:job) { undiscrete_job } + it 'raises an error if the job is attempted to be re-run' do + good_job.update!(finished_at: Time.current) + expect { good_job.perform(lock_id: process_id) }.to raise_error described_class::PreviouslyPerformedError + end + + context 'when ActiveJob rescues an error' do + let(:active_job) { TestJob.new("a string", raise_error: true) } - context 'when a job is finished' do before do - job.head_execution.update! finished_at: Time.current + TestJob.retry_on(StandardError, wait: 0, attempts: Float::INFINITY) { nil } + end + + it 'returns the results of the job' do + result = good_job.perform(lock_id: process_id) + + expect(result.value).to be_nil + expect(result.handled_error).to be_a(TestJob::ExpectedError) + end + + it 'can preserves the job' do + allow(GoodJob).to receive(:preserve_job_records).and_return(true) + + good_job.perform(lock_id: process_id) + + expect(good_job.reload).to have_attributes( + error: "TestJob::ExpectedError: Raised expected error", + performed_at: nil, + finished_at: nil + ) + end + end + + context 'when ActiveJob raises an error' do + let(:active_job) { TestJob.new("a string", raise_error: true) } + + it 'returns the results of the job' do + result = good_job.perform(lock_id: process_id) + + expect(result.value).to be_nil + expect(result.unhandled_error).to be_a(TestJob::ExpectedError) end - it 'destroys all the job executions' do - job.destroy_job + describe 'GoodJob.retry_on_unhandled_error behavior' do + context 'when true' do + before do + allow(GoodJob).to receive(:retry_on_unhandled_error).and_return(true) + end + + it 'leaves the job record unfinished' do + allow(GoodJob).to receive(:preserve_job_records).and_return(true) + + good_job.perform(lock_id: process_id) + + expect(good_job.reload).to have_attributes( + error: "TestJob::ExpectedError: Raised expected error", + performed_at: nil, + finished_at: nil + ) + end + + it 'does not destroy the job record' do + allow(GoodJob).to receive(:preserve_job_records).and_return(false) + + good_job.perform(lock_id: process_id) + expect { good_job.reload }.not_to raise_error + end + end + + context 'when false' do + before do + allow(GoodJob).to receive(:retry_on_unhandled_error).and_return(false) + end + + it 'destroys the job' do + allow(GoodJob).to receive(:preserve_job_records).and_return(false) + + good_job.perform(lock_id: process_id) + expect { good_job.reload }.to raise_error ActiveRecord::RecordNotFound + end + + it 'can preserve the job' do + allow(GoodJob).to receive(:preserve_job_records).and_return(true) + + good_job.perform(lock_id: process_id) + + expect(good_job.reload).to have_attributes( + error: "TestJob::ExpectedError: Raised expected error", + performed_at: within(1.second).of(Time.current), + finished_at: within(1.second).of(Time.current) + ) + end + + it 'preserves the job when preserving record only on error' do + allow(GoodJob).to receive(:preserve_job_records).and_return(:on_unhandled_error) + good_job.perform(lock_id: process_id) - expect { head_execution.reload }.to raise_error ActiveRecord::RecordNotFound - expect { tail_execution.reload }.to raise_error ActiveRecord::RecordNotFound - expect { job.reload }.to raise_error ActiveRecord::RecordNotFound + expect(good_job.reload).to have_attributes( + error: "TestJob::ExpectedError: Raised expected error", + performed_at: within(1.second).of(Time.current), + finished_at: within(1.second).of(Time.current) + ) + end + end end end - context 'when a job is not finished' do - it 'raises an ActionForStateMismatchError' do - expect { job.destroy_job }.to raise_error GoodJob::Job::ActionForStateMismatchError + context 'when Discrete' do + before do + ActiveJob::Base.queue_adapter = GoodJob::Adapter.new(execution_mode: :inline) + allow(described_class).to receive(:discrete_support?).and_return(true) + good_job.update!(is_discrete: true) + end + + it 'updates the Execution record and creates a DiscreteExecution record' do + good_job.perform(lock_id: process_id) + + expect(good_job.reload).to have_attributes( + executions_count: 1, + finished_at: within(1.second).of(Time.current) + ) + + dexecution = good_job.discrete_executions.first + expect(dexecution).to be_present + expect(dexecution).to have_attributes( + active_job_id: good_job.active_job_id, + job_class: good_job.job_class, + queue_name: good_job.queue_name, + created_at: within(0.001).of(good_job.performed_at), + scheduled_at: within(0.001).of(good_job.created_at), + finished_at: within(1.second).of(Time.current), + duration: be_present, + error: nil, + serialized_params: good_job.serialized_params + ) + end + + context 'when ActiveJob rescues an error' do + let(:active_job) { TestJob.new("a string", raise_error: true) } + let!(:good_job) { described_class.enqueue(active_job) } + + before do + allow(described_class).to receive(:discrete_support?).and_return(true) + allow(GoodJob).to receive(:preserve_job_records).and_return(true) + TestJob.retry_on(StandardError, wait: 1.hour, attempts: 2) { nil } + good_job.update!(is_discrete: true) + end + + it 'updates the existing Execution/Job record instead of creating a new one' do + expect { good_job.perform(lock_id: process_id) } + .to not_change(described_class, :count) + .and change { good_job.reload.serialized_params["executions"] }.by(1) + .and not_change { good_job.reload.id } + .and not_change { described_class.count } + + expect(good_job.reload).to have_attributes( + error: "TestJob::ExpectedError: Raised expected error", + created_at: within(1.second).of(Time.current), + performed_at: nil, + finished_at: nil, + scheduled_at: within(10.minutes).of(1.hour.from_now) # interval because of retry jitter + ) + expect(GoodJob::DiscreteExecution.count).to eq(1) + discrete_execution = good_job.discrete_executions.first + expect(discrete_execution).to have_attributes( + active_job_id: good_job.active_job_id, + error: "TestJob::ExpectedError: Raised expected error", + created_at: within(1.second).of(Time.current), + scheduled_at: within(1.second).of(Time.current), + finished_at: within(1.second).of(Time.current), + duration: be_present + ) + end + end + + context 'when retry_job is invoked directly during execution' do + before do + TestJob.after_perform do |job| + job.retry_job wait: 1.second + end + end + + it 'finishes the execution but does not finish the job' do + good_job.perform(lock_id: process_id) + + expect(good_job.reload).to have_attributes( + performed_at: nil, + finished_at: nil, + scheduled_at: within(0.5).of(1.second.from_now) + ) + + expect(good_job.discrete_executions.size).to eq(1) + expect(good_job.discrete_executions.first).to have_attributes( + performed_at: within(1.second).of(Time.current), + finished_at: within(1.second).of(Time.current), + duration: be_present + ) + end end end end + + describe '#queue_latency' do + let(:execution) { described_class.create! } + + it 'is nil for future scheduled execution' do + execution.scheduled_at = 1.minute.from_now + expect(execution.queue_latency).to be_nil + end + + it 'is distance between scheduled_at and now for past scheduled job' do + execution.scheduled_at = 1.minute.ago + expect(execution.queue_latency).to be_within(0.1).of(Time.zone.now - execution.scheduled_at) + end + + it 'is distance between created_at and now for queued job' do + execution.scheduled_at = nil + expect(execution.queue_latency).to be_within(0.1).of(Time.zone.now - execution.created_at) + end + + it 'is distance between created_at and performed_at for started job' do + execution.scheduled_at = nil + execution.performed_at = 10.seconds.ago + expect(execution.queue_latency).to eq(execution.performed_at - execution.created_at) + end + end + + describe "#runtime_latency" do + let(:execution) { described_class.create! } + + it 'is nil for queued job' do + expect(execution.runtime_latency).to be_nil + end + + it 'is distance between performed_at and now for started job' do + execution.performed_at = 10.seconds.ago + execution.finished_at = nil + expect(execution.runtime_latency).to be_within(0.1).of(Time.zone.now - execution.performed_at) + end + + it 'is distance between performed_at and finished_at' do + execution.performed_at = 5.seconds.ago + execution.finished_at = 1.second.ago + expect(execution.runtime_latency).to eq(execution.finished_at - execution.performed_at) + end + end end end diff --git a/spec/generators/good_job/update_generator_spec.rb b/spec/generators/good_job/update_generator_spec.rb index 7ed2e2fcb..d97d8a3d5 100644 --- a/spec/generators/good_job/update_generator_spec.rb +++ b/spec/generators/good_job/update_generator_spec.rb @@ -18,19 +18,20 @@ expect(Dir.glob("#{example_app_path}/db/migrate/[0-9]*_create_good_jobs.rb")).not_to be_empty # TODO: remove/replace this when migrations are removed/re-added - expect(Dir.glob("#{example_app_path}/db/migrate/[0-9]*_create_good_job_settings.rb")).not_to be_empty + # expect(Dir.glob("#{example_app_path}/db/migrate/[0-9]*_create_good_job_settings.rb")).not_to be_empty quiet do run_in_example_app 'rails db:migrate' end + # TODO: update this when a new migration exists in v4 # Check that `GoodJob.migrated?` is updated - expect(GoodJob.migrated?).to be true - quiet do - run_in_example_app 'rails db:rollback' - expect(GoodJob.migrated?).to be false - run_in_example_app 'rails db:migrate' - end + # expect(GoodJob.migrated?).to be true + # quiet do + # run_in_example_app 'rails db:rollback' + # expect(GoodJob.migrated?).to be false + # run_in_example_app 'rails db:migrate' + # end expect(GoodJob.migrated?).to be true end end diff --git a/spec/integration/adapter_spec.rb b/spec/integration/adapter_spec.rb index bfb063a55..f42d804ba 100644 --- a/spec/integration/adapter_spec.rb +++ b/spec/integration/adapter_spec.rb @@ -32,7 +32,7 @@ def perform(*_args, **_kwargs) describe '#perform_later' do it 'assigns a provider_job_id' do enqueued_job = TestJob.perform_later - execution = GoodJob::Execution.find(enqueued_job.provider_job_id) + execution = GoodJob::Job.find(enqueued_job.provider_job_id) expect(enqueued_job.provider_job_id).to eq execution.id end @@ -52,11 +52,11 @@ def perform(*_args, **_kwargs) it 'without a scheduled time' do expect do TestJob.perform_later('first', 'second', keyword_arg: 'keyword_arg') - end.to change(GoodJob::Execution, :count).by(1) + end.to change(GoodJob::Job, :count).by(1) - execution = GoodJob::Execution.last - expect(execution).to be_present - expect(execution).to have_attributes( + job = GoodJob::Job.last + expect(job).to be_present + expect(job).to have_attributes( queue_name: 'test', priority: 50, scheduled_at: within(1).of(Time.current) @@ -66,10 +66,10 @@ def perform(*_args, **_kwargs) it 'with a scheduled time' do expect do TestJob.set(wait: 1.minute, priority: 100).perform_later('first', 'second', keyword_arg: 'keyword_arg') - end.to change(GoodJob::Execution, :count).by(1) + end.to change(GoodJob::Job, :count).by(1) - execution = GoodJob::Execution.last - expect(execution).to have_attributes( + job = GoodJob::Job.last + expect(job).to have_attributes( queue_name: 'test', priority: 100, scheduled_at: be_within(1.second).of(1.minute.from_now) diff --git a/spec/integration/batch_spec.rb b/spec/integration/batch_spec.rb index 5665852eb..58aa0cf30 100644 --- a/spec/integration/batch_spec.rb +++ b/spec/integration/batch_spec.rb @@ -216,12 +216,11 @@ def perform(*_args, **_kwargs) GoodJob.perform_inline expect(GoodJob::Job.count).to eq 3 - expect(GoodJob::Execution.count).to eq 3 expect(GoodJob::DiscreteExecution.count).to eq 5 expect(GoodJob::Job.where(batch_id: batch.id).count).to eq 1 expect(GoodJob::Job.where(batch_callback_id: batch.id).count).to eq 2 - callback_arguments = GoodJob::Job.where(batch_callback_id: batch.id).map { |job| job.head_execution.active_job.arguments.second } + callback_arguments = GoodJob::Job.where(batch_callback_id: batch.id).map { |job| job.active_job.arguments.second } expect(callback_arguments).to contain_exactly({ event: :discard }, { event: :finish }) end @@ -232,12 +231,11 @@ def perform(*_args, **_kwargs) GoodJob.perform_inline expect(GoodJob::Job.count).to eq 3 - expect(GoodJob::Execution.count).to eq 3 expect(GoodJob::DiscreteExecution.count).to eq 5 expect(GoodJob::Job.where(batch_id: batch.id).count).to eq 1 expect(GoodJob::Job.where(batch_callback_id: batch.id).count).to eq 2 - callback_arguments = GoodJob::Job.where(batch_callback_id: batch.id).map { |job| job.head_execution.active_job.arguments.second } + callback_arguments = GoodJob::Job.where(batch_callback_id: batch.id).map { |job| job.active_job.arguments.second } expect(callback_arguments).to contain_exactly({ event: :success }, { event: :finish }) end end diff --git a/spec/integration/scheduler_spec.rb b/spec/integration/scheduler_spec.rb index de68e92d3..4683891fc 100644 --- a/spec/integration/scheduler_spec.rb +++ b/spec/integration/scheduler_spec.rb @@ -26,7 +26,7 @@ def perform(*_args, **_kwargs) puts "Thread #{thread_name} owns #{locks_count} locks." puts "GoodJobs locked by this connection:" - GoodJob::Execution.owns_advisory_locked.select('good_jobs.id', 'good_jobs.active_job_id', 'pg_locks.*').each do |execution| + GoodJob::Job.owns_advisory_locked.select('good_jobs.id', 'good_jobs.active_job_id', 'pg_locks.*').each do |execution| puts " - GoodJob #{execution.id} / ActiveJob #{execution.active_job_id} / #{execution.attributes.to_json}" end @@ -78,10 +78,10 @@ def perform(*args, **kwargs) scheduler = GoodJob::Scheduler.new(performer, max_threads: max_threads) max_threads.times { scheduler.create_thread } - wait_until(max: 60, increments_of: 0.5) { expect(GoodJob::Execution.unfinished.count).to be_zero } + wait_until(max: 60, increments_of: 0.5) { expect(GoodJob::Job.unfinished.count).to be_zero } scheduler.shutdown - expect(GoodJob::Execution.unfinished.count).to eq(0), -> { "Unworked jobs are #{GoodJob::Execution.unfinished.map(&:id)}" } + expect(GoodJob::Job.unfinished.count).to eq(0), -> { "Unworked jobs are #{GoodJob::Job.unfinished.map(&:id)}" } expect(RUN_JOBS.size).to eq(number_of_jobs), lambda { jobs_tally = RUN_JOBS.each_with_object(Hash.new(0)) do |(provider_job_id, _job_id, _thread_name), hash| hash[provider_job_id] += 1 @@ -113,7 +113,7 @@ def perform(*args, **kwargs) scheduler.create_thread sleep_until(max: 10, increments_of: 0.5) do - GoodJob::Execution.unfinished.count.zero? + GoodJob::Job.unfinished.count.zero? end scheduler.shutdown expect(scheduler).to be_shutdown @@ -129,7 +129,7 @@ def perform(*args, **kwargs) scheduler.create_thread wait_until(max: 5, increments_of: 0.5) do - expect(GoodJob::Execution.unfinished.count).to eq 0 + expect(GoodJob::Job.unfinished.count).to eq 0 end scheduler.shutdown @@ -146,7 +146,7 @@ def perform(*args, **kwargs) performer = GoodJob::JobPerformer.new('*') scheduler = GoodJob::Scheduler.new(performer, max_threads: 5, max_cache: 5) scheduler.warm_cache - sleep_until(max: 5, increments_of: 0.5) { GoodJob::Execution.unfinished.count.zero? } + sleep_until(max: 5, increments_of: 0.5) { GoodJob::Job.unfinished.count.zero? } scheduler.shutdown expect(scheduler).to be_shutdown end diff --git a/spec/lib/good_job/active_job_extensions/concurrency_spec.rb b/spec/lib/good_job/active_job_extensions/concurrency_spec.rb index e6c951633..d58f7b8e6 100644 --- a/spec/lib/good_job/active_job_extensions/concurrency_spec.rb +++ b/spec/lib/good_job/active_job_extensions/concurrency_spec.rb @@ -68,7 +68,7 @@ def perform(name:) expect(TestJob.perform_later(name: "Alice")).to be_present Rails.application.executor.wrap do - GoodJob::Execution.all.with_advisory_lock do + GoodJob::Job.all.with_advisory_lock do expect(TestJob.perform_later(name: "Alice")).to be false end end @@ -95,8 +95,8 @@ def perform(name:) # Usage of different key does enqueue expect(TestJob.perform_later(name: "Bob")).to be_present - expect(GoodJob::Execution.where(concurrency_key: "Alice").count).to eq 2 - expect(GoodJob::Execution.where(concurrency_key: "Bob").count).to eq 1 + expect(GoodJob::Job.where(concurrency_key: "Alice").count).to eq 2 + expect(GoodJob::Job.where(concurrency_key: "Bob").count).to eq 1 expect(TestJob.logger.formatter).to have_received(:call).with("INFO", anything, anything, a_string_matching(/Aborted enqueue of TestJob \(Job ID: .*\) because the concurrency key 'Alice' has reached its enqueue limit of 2 jobs/)).exactly(:once) if ActiveJob.gem_version >= Gem::Version.new("6.1.0") @@ -111,7 +111,7 @@ def perform(name:) # Lock one of the jobs Rails.application.executor.wrap do - GoodJob::Execution.first.with_advisory_lock do + GoodJob::Job.first.with_advisory_lock do # Third usage does enqueue expect(TestJob.perform_later(name: "Alice")).to be_present end @@ -244,8 +244,8 @@ def perform(*) nil end - expect(GoodJob::Execution.count).to eq 1 - expect(GoodJob::Execution.first.concurrency_key).to be_present + expect(GoodJob::Job.count).to eq 1 + expect(GoodJob::Job.first.concurrency_key).to be_present expect(GoodJob::Job.first).not_to be_finished end @@ -254,15 +254,16 @@ def perform(*) TestJob.set(wait_until: 5.minutes.ago).perform_later(name: "Alice") GoodJob::Job.first.update!(is_discrete: false) + original_concurrency_key = GoodJob::Job.last.concurrency_key + expect(original_concurrency_key).to be_present + begin GoodJob.perform_inline rescue StandardError nil end - expect(GoodJob::Execution.count).to eq 2 - first_execution, retried_execution = GoodJob::Execution.order(created_at: :asc).to_a - expect(retried_execution.concurrency_key).to eq first_execution.concurrency_key + expect(GoodJob::Job.last.concurrency_key).to eq original_concurrency_key end end end diff --git a/spec/lib/good_job/active_job_extensions/interrupt_errors_spec.rb b/spec/lib/good_job/active_job_extensions/interrupt_errors_spec.rb index b406d4cb4..e5cea4f50 100644 --- a/spec/lib/good_job/active_job_extensions/interrupt_errors_spec.rb +++ b/spec/lib/good_job/active_job_extensions/interrupt_errors_spec.rb @@ -30,47 +30,15 @@ def perform ) end - context 'when discrete execution is NOT enabled' do - before do - allow(GoodJob::Execution).to receive(:discrete_support?).and_return(false) - end - - it 'is rescuable' do - TestJob.retry_on GoodJob::InterruptError - - expect { GoodJob.perform_inline }.not_to raise_error - expect(GoodJob::Execution.count).to eq(2) - - job = GoodJob::Job.first - expect(job.executions.order(created_at: :asc).to_a).to contain_exactly(have_attributes( - performed_at: be_present, - finished_at: be_present, - error: start_with('GoodJob::InterruptError: Interrupted after starting perform at'), - error_event: GoodJob::Job::ERROR_EVENT_RETRIED - ), have_attributes( - performed_at: nil, - finished_at: nil, - error: nil, - error_event: nil - )) - end - end - context 'when discrete execution is enabled' do - before do - allow(GoodJob::Execution).to receive(:discrete_support?).and_return(true) - end - it 'does not create a new execution' do TestJob.retry_on GoodJob::InterruptError expect { GoodJob.perform_inline }.not_to raise_error expect(GoodJob::Job.count).to eq(1) - expect(GoodJob::Execution.count).to eq(1) expect(GoodJob::DiscreteExecution.count).to eq(2) job = GoodJob::Job.first - expect(job.executions.count).to eq(1) expect(job.discrete_executions.count).to eq(2) expect(job).to have_attributes( performed_at: be_blank, @@ -83,7 +51,7 @@ def perform expect(initial_discrete_execution).to have_attributes( performed_at: be_present, finished_at: be_present, - duration: GoodJob::DiscreteExecution.duration_interval_usable? ? be_present : nil, + duration: be_present, error: start_with('GoodJob::InterruptError: Interrupted after starting perform at'), error_event: GoodJob::Job::ERROR_EVENT_INTERRUPTED ) @@ -92,7 +60,7 @@ def perform expect(retried_discrete_execution).to have_attributes( performed_at: be_present, finished_at: be_present, - duration: GoodJob::DiscreteExecution.duration_interval_usable? ? be_present : nil, + duration: be_present, error: start_with('GoodJob::InterruptError: Interrupted after starting perform at'), error_event: GoodJob::Job::ERROR_EVENT_RETRIED ) diff --git a/spec/lib/good_job/adapter_spec.rb b/spec/lib/good_job/adapter_spec.rb index 589c23acd..19cc515d4 100644 --- a/spec/lib/good_job/adapter_spec.rb +++ b/spec/lib/good_job/adapter_spec.rb @@ -5,7 +5,7 @@ RSpec.describe GoodJob::Adapter do let(:adapter) { described_class.new(execution_mode: :external) } let(:active_job) { instance_double(ActiveJob::Base) } - let(:good_job) { instance_double(GoodJob::Execution, queue_name: 'default', scheduled_at: nil, job_state: { queue_name: 'default' }) } + let(:good_job) { instance_double(GoodJob::Job, queue_name: 'default', scheduled_at: nil, job_state: { queue_name: 'default' }) } before do GoodJob.configuration.instance_variable_set(:@_in_webserver, nil) @@ -49,12 +49,12 @@ ) end - it 'calls GoodJob::Execution.enqueue with parameters' do - allow(GoodJob::Execution).to receive(:enqueue).and_return(good_job) + it 'calls GoodJob::Job.enqueue with parameters' do + allow(GoodJob::Job).to receive(:enqueue).and_return(good_job) adapter.enqueue(active_job) - expect(GoodJob::Execution).to have_received(:enqueue).with( + expect(GoodJob::Job).to have_received(:enqueue).with( active_job, scheduled_at: nil ) @@ -97,7 +97,7 @@ def perform(succeed: true) context 'when async' do it 'triggers the capsule and the notifier' do - allow(GoodJob::Execution).to receive(:enqueue).and_return(good_job) + allow(GoodJob::Job).to receive(:enqueue).and_return(good_job) allow(GoodJob::Notifier).to receive(:notify) capsule = instance_double(GoodJob::Capsule, start: nil, create_thread: nil) @@ -115,14 +115,14 @@ def perform(succeed: true) end describe '#enqueue_at' do - it 'calls GoodJob::Execution.enqueue with parameters' do - allow(GoodJob::Execution).to receive(:enqueue).and_return(good_job) + it 'calls GoodJob::Job.enqueue with parameters' do + allow(GoodJob::Job).to receive(:enqueue).and_return(good_job) scheduled_at = 1.minute.from_now adapter.enqueue_at(active_job, scheduled_at.to_i) - expect(GoodJob::Execution).to have_received(:enqueue).with( + expect(GoodJob::Job).to have_received(:enqueue).with( active_job, scheduled_at: scheduled_at.change(usec: 0) ) @@ -204,7 +204,7 @@ def perform(succeed: true) context 'when a job fails to enqueue' do it 'does not set a provider_job_id' do - allow(GoodJob::Execution).to receive(:insert_all).and_wrap_original do |original_method, *args| + allow(GoodJob::Job).to receive(:insert_all).and_wrap_original do |original_method, *args| attributes, kwargs = *args original_method.call(attributes[0, 1], **kwargs) # pretend only the first item is successfully inserted end @@ -218,7 +218,7 @@ def perform(succeed: true) end it 'sets successfully_enqueued, if Rails supports it' do - allow(GoodJob::Execution).to receive(:insert_all).and_wrap_original do |original_method, *args| + allow(GoodJob::Job).to receive(:insert_all).and_wrap_original do |original_method, *args| attributes, kwargs = *args original_method.call(attributes[0, 1], **kwargs) # pretend only the first item is successfully inserted end @@ -238,7 +238,7 @@ def perform(succeed: true) stub_const 'PERFORMED', [] stub_const 'TestJob', (Class.new(ActiveJob::Base) do def perform - raise "Not advisory locked" unless GoodJob::Execution.find(provider_job_id).advisory_locked? + raise "Not advisory locked" unless GoodJob::Job.find(provider_job_id).advisory_locked? PERFORMED << Time.current end diff --git a/spec/lib/good_job/cli_spec.rb b/spec/lib/good_job/cli_spec.rb index 4d47ad713..f07d54ae9 100644 --- a/spec/lib/good_job/cli_spec.rb +++ b/spec/lib/good_job/cli_spec.rb @@ -159,9 +159,9 @@ end describe '#cleanup_preserved_jobs' do - let!(:recent_job) { GoodJob::Execution.create!(active_job_id: SecureRandom.uuid, finished_at: 12.hours.ago) } - let!(:old_unfinished_job) { GoodJob::Execution.create!(active_job_id: SecureRandom.uuid, scheduled_at: 2.days.ago, finished_at: nil) } - let!(:old_finished_job) { GoodJob::Execution.create!(active_job_id: SecureRandom.uuid, finished_at: 36.hours.ago) } + let!(:recent_job) { GoodJob::Job.create!(active_job_id: SecureRandom.uuid, finished_at: 12.hours.ago) } + let!(:old_unfinished_job) { GoodJob::Job.create!(active_job_id: SecureRandom.uuid, scheduled_at: 2.days.ago, finished_at: nil) } + let!(:old_finished_job) { GoodJob::Job.create!(active_job_id: SecureRandom.uuid, finished_at: 36.hours.ago) } it 'destroys finished jobs' do cli = described_class.new([], { before_seconds_ago: 24.hours.to_i }, {}) diff --git a/spec/lib/good_job/cron_manager_spec.rb b/spec/lib/good_job/cron_manager_spec.rb index 77d3aac4a..ca5f2730e 100644 --- a/spec/lib/good_job/cron_manager_spec.rb +++ b/spec/lib/good_job/cron_manager_spec.rb @@ -46,12 +46,12 @@ cron_manager = described_class.new(cron_entries, start_on_initialize: true) wait_until(max: 5) do - expect(GoodJob::Execution.count).to be > 3 + expect(GoodJob::Job.count).to be > 3 end cron_manager.shutdown - execution = GoodJob::Execution.first - expect(execution).to have_attributes( + job = GoodJob::Job.first + expect(job).to have_attributes( cron_key: 'example', priority: -10 ) @@ -62,14 +62,14 @@ other_cron_manager = described_class.new(cron_entries, start_on_initialize: true) wait_until(max: 5) do - expect(GoodJob::Execution.count).to be > 3 + expect(GoodJob::Job.count).to be > 3 end cron_manager.shutdown other_cron_manager.shutdown - executions = GoodJob::Execution.all.to_a - expect(executions.size).to eq executions.map(&:cron_at).uniq.size + jobs = GoodJob::Job.all.to_a + expect(jobs.size).to eq jobs.map(&:cron_at).uniq.size end it 'respects the disabled setting' do @@ -79,7 +79,7 @@ sleep 2 cron_manager.shutdown - expect(GoodJob::Execution.count).to eq 0 + expect(GoodJob::Job.count).to eq 0 end context 'when schedule is a proc' do diff --git a/spec/lib/good_job/current_thread_spec.rb b/spec/lib/good_job/current_thread_spec.rb index 00fd5a19e..d336f20b8 100644 --- a/spec/lib/good_job/current_thread_spec.rb +++ b/spec/lib/good_job/current_thread_spec.rb @@ -8,13 +8,12 @@ [ :cron_at, :cron_key, - :execution, :error_on_discard, :error_on_retry, :error_on_retry_stopped, - :execution, :execution_interrupted, :execution_retried, + :job, ].each do |accessor| describe ".#{accessor}" do it 'maintains value across threads' do @@ -44,13 +43,13 @@ end describe '.active_job_id' do - let!(:execution) { GoodJob::Execution.create! active_job_id: SecureRandom.uuid } + let!(:job) { GoodJob::Job.create! active_job_id: SecureRandom.uuid } it 'delegates to good_job' do expect(described_class.active_job_id).to be_nil - described_class.execution = execution - expect(described_class.active_job_id).to eq execution.active_job_id + described_class.job = job + expect(described_class.active_job_id).to eq job.active_job_id end end @@ -62,7 +61,7 @@ error_on_discard: false, error_on_retry: false, error_on_retry_stopped: nil, - execution: instance_double(GoodJob::Execution), + job: instance_double(GoodJob::Job), execution_interrupted: nil, execution_retried: nil, retry_now: nil, diff --git a/spec/lib/good_job/job_performer_spec.rb b/spec/lib/good_job/job_performer_spec.rb index d83ae37ff..adf145633 100644 --- a/spec/lib/good_job/job_performer_spec.rb +++ b/spec/lib/good_job/job_performer_spec.rb @@ -13,19 +13,19 @@ describe '#next' do it 'calls GoodJob.perform_with_advisory_lock' do - allow(GoodJob::Execution).to receive(:perform_with_advisory_lock) + allow(GoodJob::Job).to receive(:perform_with_advisory_lock) job_performer = described_class.new('*') job_performer.next - expect(GoodJob::Execution).to have_received(:perform_with_advisory_lock) + expect(GoodJob::Job).to have_received(:perform_with_advisory_lock) end it 'records the active job id for the duration of #next' do job_performer = described_class.new('*') - execution = instance_double(GoodJob::Execution, perform: nil, active_job_id: '123') - allow(GoodJob::Execution).to receive(:perform_with_advisory_lock) do |&block| + execution = instance_double(GoodJob::Job, perform: nil, active_job_id: '123') + allow(GoodJob::Job).to receive(:perform_with_advisory_lock) do |&block| block.call(execution) expect(job_performer.performing_active_job_ids).to include('123') diff --git a/spec/lib/good_job_spec.rb b/spec/lib/good_job_spec.rb index 5c25cc00e..098e4f230 100644 --- a/spec/lib/good_job_spec.rb +++ b/spec/lib/good_job_spec.rb @@ -53,23 +53,21 @@ end describe '.cleanup_preserved_jobs' do - let!(:recent_job) { GoodJob::Execution.create!(active_job_id: SecureRandom.uuid, finished_at: 12.hours.ago) } - let!(:old_unfinished_job) { GoodJob::Execution.create!(active_job_id: SecureRandom.uuid, scheduled_at: 15.days.ago, finished_at: nil) } - let!(:old_finished_job) { GoodJob::Execution.create!(active_job_id: SecureRandom.uuid, finished_at: 15.days.ago) } - let!(:old_finished_job_execution) { GoodJob::Execution.create!(active_job_id: old_finished_job.active_job_id, retried_good_job_id: old_finished_job.id, finished_at: 16.days.ago) } + let!(:recent_job) { GoodJob::Job.create!(active_job_id: SecureRandom.uuid, finished_at: 12.hours.ago) } + let!(:old_unfinished_job) { GoodJob::Job.create!(active_job_id: SecureRandom.uuid, scheduled_at: 15.days.ago, finished_at: nil) } + let!(:old_finished_job) { GoodJob::Job.create!(active_job_id: SecureRandom.uuid, finished_at: 15.days.ago) } let!(:old_finished_job_discrete_execution) { GoodJob::DiscreteExecution.create!(active_job_id: old_finished_job.active_job_id, finished_at: 16.days.ago) } - let!(:old_discarded_job) { GoodJob::Execution.create!(active_job_id: SecureRandom.uuid, finished_at: 15.days.ago, error: "Error") } + let!(:old_discarded_job) { GoodJob::Job.create!(active_job_id: SecureRandom.uuid, finished_at: 15.days.ago, error: "Error") } let!(:old_batch) { GoodJob::BatchRecord.create!(finished_at: 15.days.ago) } it 'deletes finished jobs' do destroyed_records_count = described_class.cleanup_preserved_jobs(in_batches_of: 1) - expect(destroyed_records_count).to eq 5 + expect(destroyed_records_count).to eq 4 expect { recent_job.reload }.not_to raise_error expect { old_unfinished_job.reload }.not_to raise_error expect { old_finished_job.reload }.to raise_error ActiveRecord::RecordNotFound - expect { old_finished_job_execution.reload }.to raise_error ActiveRecord::RecordNotFound expect { old_finished_job_discrete_execution.reload }.to raise_error ActiveRecord::RecordNotFound expect { old_discarded_job.reload }.to raise_error ActiveRecord::RecordNotFound expect { old_batch.reload }.to raise_error ActiveRecord::RecordNotFound @@ -78,12 +76,11 @@ it 'takes arguments' do destroyed_records_count = described_class.cleanup_preserved_jobs(older_than: 10.seconds) - expect(destroyed_records_count).to eq 6 + expect(destroyed_records_count).to eq 5 expect { recent_job.reload }.to raise_error ActiveRecord::RecordNotFound expect { old_unfinished_job.reload }.not_to raise_error expect { old_finished_job.reload }.to raise_error ActiveRecord::RecordNotFound - expect { old_finished_job_execution.reload }.to raise_error ActiveRecord::RecordNotFound expect { old_finished_job_discrete_execution.reload }.to raise_error ActiveRecord::RecordNotFound expect { old_discarded_job.reload }.to raise_error ActiveRecord::RecordNotFound expect { old_batch.reload }.to raise_error ActiveRecord::RecordNotFound @@ -104,12 +101,11 @@ allow(described_class.configuration).to receive(:env).and_return ENV.to_hash.merge({ 'GOOD_JOB_CLEANUP_DISCARDED_JOBS' => 'false' }) destroyed_records_count = described_class.cleanup_preserved_jobs - expect(destroyed_records_count).to eq 4 + expect(destroyed_records_count).to eq 3 expect { recent_job.reload }.not_to raise_error expect { old_unfinished_job.reload }.not_to raise_error expect { old_finished_job.reload }.to raise_error ActiveRecord::RecordNotFound - expect { old_finished_job_execution.reload }.to raise_error ActiveRecord::RecordNotFound expect { old_finished_job_discrete_execution.reload }.to raise_error ActiveRecord::RecordNotFound expect { old_discarded_job.reload }.not_to raise_error expect { old_batch.reload }.to raise_error ActiveRecord::RecordNotFound diff --git a/spec/support/reset_good_job.rb b/spec/support/reset_good_job.rb index d484c6187..aafd2a637 100644 --- a/spec/support/reset_good_job.rb +++ b/spec/support/reset_good_job.rb @@ -74,7 +74,7 @@ puts "There are #{own_locks.count} advisory locks still open by the current database connection AFTER test run." puts "\nAdvisory locked executions:" - GoodJob::Execution.advisory_locked.owns_advisory_locked.each do |execution| + GoodJob::Job.advisory_locked.owns_advisory_locked.each do |execution| puts " - Execution ID: #{execution.id} / Active Job ID: #{execution.active_job_id}" end @@ -96,7 +96,7 @@ puts "There are #{other_locks.count} advisory locks owned by other connections still open AFTER test run." puts "\nAdvisory locked executions:" - GoodJob::Execution.advisory_locked.each do |execution| + GoodJob::Job.advisory_locked.each do |execution| puts " - Execution ID: #{execution.id} / Active Job ID: #{execution.active_job_id} / Locked by: #{execution[:pid]}" end @@ -173,11 +173,11 @@ def self.debug_own_locks(connection) output = [] output << "There are #{count} advisory locks still open by the current database connection." - GoodJob::Execution.include(GoodJob::OverridableConnection) - GoodJob::Execution.override_connection(connection) do - GoodJob::Execution.owns_advisory_locked.each.with_index do |execution, index| - output << "\nAdvisory locked GoodJob::Execution:" if index.zero? - output << " - Execution ID: #{execution.id} / Active Job ID: #{execution.active_job_id}" + GoodJob::Job.include(GoodJob::OverridableConnection) + GoodJob::Job.override_connection(connection) do + GoodJob::Job.owns_advisory_locked.each.with_index do |job, index| + output << "\nAdvisory locked GoodJob::Job:" if index.zero? + output << " - Job ID: #{job.id} / Active Job ID: #{job.active_job_id}" end end @@ -214,13 +214,13 @@ def unlock ActiveRecord::Relation::QueryAttribute.new('classid', classid, ActiveRecord::Type::String.new), ActiveRecord::Relation::QueryAttribute.new('objid', objid, ActiveRecord::Type::String.new), ] - self.class.connection.exec_query(GoodJob::Execution.pg_or_jdbc_query(query), 'PgLock Advisory Unlock', binds).first['unlocked'] + self.class.connection.exec_query(GoodJob::Job.pg_or_jdbc_query(query), 'PgLock Advisory Unlock', binds).first['unlocked'] end def unlock! query = <<~SQL.squish SELECT pg_terminate_backend(#{self[:pid]}) AS terminated SQL - self.class.connection.exec_query(GoodJob::Execution.pg_or_jdbc_query(query), 'PgLock Terminate Backend Lock', []).first['terminated'] + self.class.connection.exec_query(GoodJob::Job.pg_or_jdbc_query(query), 'PgLock Terminate Backend Lock', []).first['terminated'] end end diff --git a/spec/system/jobs_spec.rb b/spec/system/jobs_spec.rb index 93d5ca390..6dda74d97 100644 --- a/spec/system/jobs_spec.rb +++ b/spec/system/jobs_spec.rb @@ -136,7 +136,7 @@ accept_confirm { click_link 'Discard job' } end expect(page).to have_content "Job has been discarded" - end.to change { unfinished_job.head_execution(reload: true).finished_at }.from(nil).to within(1.second).of(Time.current) + end.to change { unfinished_job.reload.finished_at }.from(nil).to within(1.second).of(Time.current) end it 'can force discard jobs' do @@ -165,7 +165,7 @@ accept_confirm { click_link 'Force discard' } end expect(page).to have_content "Job has been force discarded" - end.to change { unfinished_job.head_execution(reload: true).finished_at }.from(nil).to within(1.second).of(Time.current) + end.to change { unfinished_job.reload.finished_at }.from(nil).to within(1.second).of(Time.current) ensure locked_event.set done_event.set