diff --git a/Gemfile.lock b/Gemfile.lock index a3ae36d..92b4005 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: . specs: - delayed (1.2.1) + delayed (2.0.0) activerecord (>= 6.0) concurrent-ruby diff --git a/README.md b/README.md index 5180864..0895247 100644 --- a/README.md +++ b/README.md @@ -603,4 +603,5 @@ creating a new issue to get early feedback on your proposed change. * Fork the project and create a new branch for your contribution. * Write your contribution (and any applicable test coverage). * Make sure all tests pass (`bundle exec rake`). + * If you are changing SQL queries, re-record snapshots with `RECORD_SNAPSHOTS=1` * Submit a pull request. diff --git a/Rakefile b/Rakefile index bddf975..1c7ae41 100644 --- a/Rakefile +++ b/Rakefile @@ -20,6 +20,10 @@ task :adapter do ENV['ADAPTER'] = nil end +if ENV['RECORD_SNAPSHOTS'] + `rm -rf spec/**/__snapshots__` +end + require 'rubocop/rake_task' RuboCop::RakeTask.new diff --git a/db/migrate/4_index_live_jobs.rb b/db/migrate/4_index_live_jobs.rb index ba6c59e..b8d8960 100644 --- a/db/migrate/4_index_live_jobs.rb +++ b/db/migrate/4_index_live_jobs.rb @@ -8,7 +8,7 @@ class IndexLiveJobs < ActiveRecord::Migration[6.0] disable_ddl_transaction! if concurrent_index_creation_supported? def change - opts = {} + opts = { name: 'idx_delayed_jobs_live' } columns = %i(priority run_at locked_at queue attempts) # Postgres supports creating indexes concurrently, @@ -28,6 +28,6 @@ def change # (On other databases, we can include `locked_at` to allow for more "covering" index lookups) columns -= %i(locked_at) if connection.adapter_name == 'PostgreSQL' - upsert_index :delayed_jobs, columns, wait_timeout: WAIT_TIMEOUT, name: 'idx_delayed_jobs_live', **opts + upsert_index :delayed_jobs, columns, wait_timeout: WAIT_TIMEOUT, **opts end end diff --git a/db/migrate/5_index_failed_jobs.rb b/db/migrate/5_index_failed_jobs.rb index 127c787..a06b8c9 100644 --- a/db/migrate/5_index_failed_jobs.rb +++ b/db/migrate/5_index_failed_jobs.rb @@ -11,15 +11,14 @@ def change # You can delete this migration if your database does not support partial indexes. return unless connection.supports_partial_index? + # If partial indexes are supported, then the "live" index does not cover failed jobs. + # To aid in monitoring, this adds a separate (smaller) index for failed jobs: + opts = { name: 'idx_delayed_jobs_failed', where: '(failed_at IS NOT NULL)' } + # Postgres supports creating indexes concurrently, which avoids locking the table # while the index is building: - opts = {} opts[:algorithm] = :concurrently if concurrent_index_creation_supported? - # If partial indexes are supported, then the "live" index does not cover failed jobs. - # To aid in monitoring, this adds a separate (smaller) index for failed jobs: - opts.merge!(name: 'idx_delayed_jobs_failed', where: '(failed_at IS NOT NULL)') - upsert_index :delayed_jobs, %i(priority queue), wait_timeout: WAIT_TIMEOUT, **opts end end diff --git a/lib/delayed/backend/job_preparer.rb b/lib/delayed/backend/job_preparer.rb index f436d03..ce71499 100644 --- a/lib/delayed/backend/job_preparer.rb +++ b/lib/delayed/backend/job_preparer.rb @@ -34,7 +34,7 @@ def set_priority end def scheduled_into_fall_back_hour? - options.key?(:run_at) && + options[:run_at] && !options[:run_at].in_time_zone.dst? && (options[:run_at] - 1.hour).dst? end diff --git a/lib/delayed/helpers/migration.rb b/lib/delayed/helpers/migration.rb index e93be0d..fd3a68a 100644 --- a/lib/delayed/helpers/migration.rb +++ b/lib/delayed/helpers/migration.rb @@ -24,12 +24,18 @@ def remove_index_if_exists(*args, **opts) dir(:down) { _add_or_replace_index(*args, **opts) } end + RETRY_EXCEPTIONS = [ + ActiveRecord::LockWaitTimeout, + ActiveRecord::StatementTimeout, + (PG::LockNotAvailable if defined?(PG::LockNotAvailable)), + ].compact.freeze + def with_retry_loop(wait_timeout: 5.minutes, **opts) with_timeouts(**opts) do loop do yield break - rescue ActiveRecord::LockWaitTimeout, ActiveRecord::StatementTimeout => e + rescue *RETRY_EXCEPTIONS => e raise if Delayed::Job.db_time_now - @migration_start > wait_timeout Delayed.logger.warn("Index creation failed for #{opts[:name]}: #{e.message}. Retrying...") diff --git a/lib/delayed/version.rb b/lib/delayed/version.rb index 7c56605..3cbeea9 100644 --- a/lib/delayed/version.rb +++ b/lib/delayed/version.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true module Delayed - VERSION = '1.2.1' + VERSION = '2.0.0' end diff --git a/spec/delayed/job_spec.rb b/spec/delayed/job_spec.rb index add2854..1bb4faa 100644 --- a/spec/delayed/job_spec.rb +++ b/spec/delayed/job_spec.rb @@ -239,7 +239,7 @@ def create_job(opts = {}) end end - context 'when using the legacy index', index: :legacy do + context 'when using the legacy index', :with_legacy_table_index do it "[legacy index] generates the expected #{current_adapter} query plan" do expect(query.explain).to match_snapshot end @@ -277,7 +277,7 @@ def create_job(opts = {}) expect(query.explain).to match_snapshot end - context 'when using legacy index', index: :legacy do + context 'when using legacy index', :with_legacy_table_index do it "[legacy index] generates an efficient #{current_adapter} query plan" do expect(query.explain).to match_snapshot end @@ -344,6 +344,11 @@ def create_job(opts = {}) job = create_job run_at: dst_start + 1.minute expect(described_class.reserve(worker, dst_start + 59.minutes)).to eq([job]) end + + it 'does not break when run_at is explicitly nil' do + job = create_job run_at: nil + expect(job.run_at).not_to be_nil + end end context 'when using :local non-UTC time for DB timestamps' do diff --git a/spec/delayed/monitor_spec.rb b/spec/delayed/monitor_spec.rb index b2187eb..7b19d7a 100644 --- a/spec/delayed/monitor_spec.rb +++ b/spec/delayed/monitor_spec.rb @@ -308,7 +308,7 @@ def query_for(metric) expect(query_for(metric).explain).to match_snapshot end - context 'when using the legacy index', index: :legacy do + context 'when using the legacy index', :with_legacy_table_index do it "[legacy index] produces the expected #{current_adapter} query plan for #{metric}" do expect(query_for(metric).explain).to match_snapshot end diff --git a/spec/helper.rb b/spec/helper.rb index 5e78649..30c4c3e 100644 --- a/spec/helper.rb +++ b/spec/helper.rb @@ -180,7 +180,7 @@ class SingletonClass Delayed::Job.delete_all end - config.around(:each, index: :legacy) do |example| + config.around(:each, :with_legacy_table_index) do |example| IndexFailedJobs.migrate(:down) IndexLiveJobs.migrate(:down) AddIndexToDelayedJobsName.migrate(:down)