From ff1ff11bae90c36b01727391b7632bbda414bbb2 Mon Sep 17 00:00:00 2001 From: Gary Tou Date: Sat, 10 Feb 2024 20:29:28 -0800 Subject: [PATCH 1/9] Extract delayed logic to method --- .../mission_control/jobs/jobs/scheduled/_job.html.erb | 2 +- lib/active_job/scheduled.rb | 7 +++++++ lib/mission_control/jobs/engine.rb | 1 + 3 files changed, 9 insertions(+), 1 deletion(-) create mode 100644 lib/active_job/scheduled.rb diff --git a/app/views/mission_control/jobs/jobs/scheduled/_job.html.erb b/app/views/mission_control/jobs/jobs/scheduled/_job.html.erb index 144c10ad..77cd23ff 100644 --- a/app/views/mission_control/jobs/jobs/scheduled/_job.html.erb +++ b/app/views/mission_control/jobs/jobs/scheduled/_job.html.erb @@ -1,7 +1,7 @@ <%= link_to job.queue_name, application_queue_path(@application, job.queue) %> <%= bidirectional_time_distance_in_words_with_title(job.scheduled_at) %> - <% if job.scheduled_at.before? 1.minute.ago %> + <% if job.scheduled_enqueue_delayed? %>
delayed
<% end %> diff --git a/lib/active_job/scheduled.rb b/lib/active_job/scheduled.rb new file mode 100644 index 00000000..2e099358 --- /dev/null +++ b/lib/active_job/scheduled.rb @@ -0,0 +1,7 @@ +module ActiveJob::Scheduled + extend ActiveSupport::Concern + + def scheduled_enqueue_delayed? + status == :scheduled && scheduled_at.before?(1.minute.ago) + end +end diff --git a/lib/mission_control/jobs/engine.rb b/lib/mission_control/jobs/engine.rb index 3a2102be..ff56ebdb 100644 --- a/lib/mission_control/jobs/engine.rb +++ b/lib/mission_control/jobs/engine.rb @@ -30,6 +30,7 @@ class Engine < ::Rails::Engine include ActiveJob::Querying include ActiveJob::Executing include ActiveJob::Failed + include ActiveJob::Scheduled ActiveJob.extend ActiveJob::Querying::Root end end From 7fe49ea6d46526a8e9a5f21a52d8d1fa9d1753f7 Mon Sep 17 00:00:00 2001 From: Gary Tou Date: Sat, 10 Feb 2024 20:40:54 -0800 Subject: [PATCH 2/9] Add `scheduled_job_delay_threshold` configuration --- lib/active_job/scheduled.rb | 2 +- lib/mission_control/jobs.rb | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/active_job/scheduled.rb b/lib/active_job/scheduled.rb index 2e099358..ab78ad5c 100644 --- a/lib/active_job/scheduled.rb +++ b/lib/active_job/scheduled.rb @@ -2,6 +2,6 @@ module ActiveJob::Scheduled extend ActiveSupport::Concern def scheduled_enqueue_delayed? - status == :scheduled && scheduled_at.before?(1.minute.ago) + status == :scheduled && scheduled_at.before?(MissionControl::Jobs.scheduled_job_delay_threshold.ago) end end diff --git a/lib/mission_control/jobs.rb b/lib/mission_control/jobs.rb index dba59137..f38d9c05 100644 --- a/lib/mission_control/jobs.rb +++ b/lib/mission_control/jobs.rb @@ -15,5 +15,6 @@ module Jobs mattr_accessor :base_controller_class, default: "::ApplicationController" mattr_accessor :delay_between_bulk_operation_batches, default: 0 mattr_accessor :logger, default: ActiveSupport::Logger.new(nil) + mattr_accessor :scheduled_job_delay_threshold, default: 1.minute end end From 1c530f2cf072a730b23c8ad5b93db0b222153a3b Mon Sep 17 00:00:00 2001 From: Gary Tou Date: Sat, 10 Feb 2024 20:53:08 -0800 Subject: [PATCH 3/9] Add tests for `scheduled_job_delay_threshold` configuration and `ActiveJob#scheduled_enqueue_delayed?` Tests are skipped if the adapter does not support scheduled jobs (e.g. Resque) --- .../queue_adapters/adapter_testing.rb | 2 +- .../adapter_testing/job_statuses.rb | 36 +++++++++++++++++++ test/support/jobs_helper.rb | 6 ++++ 3 files changed, 43 insertions(+), 1 deletion(-) create mode 100644 test/active_job/queue_adapters/adapter_testing/job_statuses.rb diff --git a/test/active_job/queue_adapters/adapter_testing.rb b/test/active_job/queue_adapters/adapter_testing.rb index 41236874..cb7b29a0 100644 --- a/test/active_job/queue_adapters/adapter_testing.rb +++ b/test/active_job/queue_adapters/adapter_testing.rb @@ -2,7 +2,7 @@ module ActiveJob::QueueAdapters::AdapterTesting extend ActiveSupport::Concern included do - include CountJobs, DiscardJobs, FindJobs, JobBatches, QueryJobs, Queues, RetryJobs + include CountJobs, DiscardJobs, FindJobs, JobBatches, QueryJobs, Queues, RetryJobs, JobStatuses setup do ActiveJob::Base.queue_adapter = queue_adapter diff --git a/test/active_job/queue_adapters/adapter_testing/job_statuses.rb b/test/active_job/queue_adapters/adapter_testing/job_statuses.rb new file mode 100644 index 00000000..aa92ef07 --- /dev/null +++ b/test/active_job/queue_adapters/adapter_testing/job_statuses.rb @@ -0,0 +1,36 @@ +module ActiveJob::QueueAdapters::AdapterTesting::JobStatuses + extend ActiveSupport::Concern + extend ActiveSupport::Testing::Declarative + + test "job is scheduled_enqueue_delayed?" do + skip_unless_queue_adapter_supports_status :scheduled + + DummyJob.set(wait: 5.minute).perform_later + job = ActiveJob.jobs.scheduled.last + refute job.scheduled_enqueue_delayed? + + travel 5.minute + MissionControl::Jobs.scheduled_job_delay_threshold + 1.second + + assert job.scheduled_enqueue_delayed? + end + + test "job with configured scheduled_job_delay_threshold is scheduled_enqueue_delayed?" do + skip_unless_queue_adapter_supports_status :scheduled + + begin + original_threshold = MissionControl::Jobs.scheduled_job_delay_threshold + MissionControl::Jobs.scheduled_job_delay_threshold = 3.minutes + + DummyJob.set(wait: 5.minute).perform_later + job = ActiveJob.jobs.scheduled.last + refute job.scheduled_enqueue_delayed? + + travel 5.minute + MissionControl::Jobs.scheduled_job_delay_threshold + 1.second + + assert job.scheduled_enqueue_delayed? + ensure + MissionControl::Jobs.scheduled_job_delay_threshold = original_threshold + end + end + +end diff --git a/test/support/jobs_helper.rb b/test/support/jobs_helper.rb index ddb5d135..942cbdbb 100644 --- a/test/support/jobs_helper.rb +++ b/test/support/jobs_helper.rb @@ -16,4 +16,10 @@ def within_job_server(app_id, server: nil, &block) def default_job_server MissionControl::Jobs.applications.first.servers.first end + + def skip_unless_queue_adapter_supports_status(status) + return if ActiveJob::Base.queue_adapter.supported_statuses.include? status + + skip "#{ActiveJob::Base.queue_adapter_name} does not support #{status} jobs" + end end From 0acad6931119933b8367b1b922e8d30ca2e9c3c8 Mon Sep 17 00:00:00 2001 From: Gary Tou Date: Sat, 10 Feb 2024 20:54:45 -0800 Subject: [PATCH 4/9] Add test for existing `ActiveJob#failed?` method --- .../queue_adapters/adapter_testing/job_statuses.rb | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/test/active_job/queue_adapters/adapter_testing/job_statuses.rb b/test/active_job/queue_adapters/adapter_testing/job_statuses.rb index aa92ef07..b9250a06 100644 --- a/test/active_job/queue_adapters/adapter_testing/job_statuses.rb +++ b/test/active_job/queue_adapters/adapter_testing/job_statuses.rb @@ -2,6 +2,18 @@ module ActiveJob::QueueAdapters::AdapterTesting::JobStatuses extend ActiveSupport::Concern extend ActiveSupport::Testing::Declarative + test "job is failed?" do + skip_unless_queue_adapter_supports_status :failed + + job = FailingJob.perform_later + assert_not job.failed? + + perform_enqueued_jobs + job = ActiveJob.jobs.failed.last + + assert job.failed? + end + test "job is scheduled_enqueue_delayed?" do skip_unless_queue_adapter_supports_status :scheduled From a2503760f0ad777e35d72d8b4d21b4342b3c556b Mon Sep 17 00:00:00 2001 From: Gary Tou Date: Sat, 10 Feb 2024 21:07:05 -0800 Subject: [PATCH 5/9] Add `scheduled_job_delay_threshold` configuration to README --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 40b2c303..501e1f4c 100644 --- a/README.md +++ b/README.md @@ -54,6 +54,7 @@ Besides `base_controller_class`, you can also set the following for `MissionCont - `logger`: the logger you want Mission Control Jobs to use. Defaults to `ActiveSupport::Logger.new(nil)` (no logging). Notice that this is different from Active Job's logger or Active Job's backend's configured logger. - `delay_between_bulk_operation_batches`: how long to wait between batches when performing bulk operations, such as _discard all_ or _retry all_ jobs—defaults to `0` - `adapters`: a list of adapters that you want Mission Control to use and extend. By default this will be the adapter you have set for `active_job.queue_adapter`. +- `scheduled_job_delay_threshold`: the time duration before a scheduled job is considered delayed. Defaults to `1.minute` (a job is considered delayed if it hasn't transitioned from the `scheduled` status 1 minute after the scheduled time). This library extends Active Job with a querying interface and the following setting: - `config.active_job.default_page_size`: the internal batch size that Active Job will use when sending queries to the underlying adapter and the batch size for the bulk operations defined above—defaults to `1000`. From 017120105e4db84b7bb9dec1145f880f7dfa1d8d Mon Sep 17 00:00:00 2001 From: Gary Tou Date: Mon, 19 Feb 2024 17:30:24 -0800 Subject: [PATCH 6/9] Remove adapter tests for `scheduled_enqueue_delayed?` --- .../adapter_testing/job_statuses.rb | 32 ------------------- 1 file changed, 32 deletions(-) diff --git a/test/active_job/queue_adapters/adapter_testing/job_statuses.rb b/test/active_job/queue_adapters/adapter_testing/job_statuses.rb index b9250a06..1b48e6df 100644 --- a/test/active_job/queue_adapters/adapter_testing/job_statuses.rb +++ b/test/active_job/queue_adapters/adapter_testing/job_statuses.rb @@ -13,36 +13,4 @@ module ActiveJob::QueueAdapters::AdapterTesting::JobStatuses assert job.failed? end - - test "job is scheduled_enqueue_delayed?" do - skip_unless_queue_adapter_supports_status :scheduled - - DummyJob.set(wait: 5.minute).perform_later - job = ActiveJob.jobs.scheduled.last - refute job.scheduled_enqueue_delayed? - - travel 5.minute + MissionControl::Jobs.scheduled_job_delay_threshold + 1.second - - assert job.scheduled_enqueue_delayed? - end - - test "job with configured scheduled_job_delay_threshold is scheduled_enqueue_delayed?" do - skip_unless_queue_adapter_supports_status :scheduled - - begin - original_threshold = MissionControl::Jobs.scheduled_job_delay_threshold - MissionControl::Jobs.scheduled_job_delay_threshold = 3.minutes - - DummyJob.set(wait: 5.minute).perform_later - job = ActiveJob.jobs.scheduled.last - refute job.scheduled_enqueue_delayed? - - travel 5.minute + MissionControl::Jobs.scheduled_job_delay_threshold + 1.second - - assert job.scheduled_enqueue_delayed? - ensure - MissionControl::Jobs.scheduled_job_delay_threshold = original_threshold - end - end - end From 0ec3dad232428a66d669bfce17c6a47989f25b48 Mon Sep 17 00:00:00 2001 From: Gary Tou Date: Mon, 19 Feb 2024 17:31:46 -0800 Subject: [PATCH 7/9] Switch out `ActiveJob#scheduled_enqueue_delayed?` for view helper method --- app/helpers/mission_control/jobs/jobs_helper.rb | 4 ++++ .../mission_control/jobs/jobs/scheduled/_job.html.erb | 2 +- lib/active_job/scheduled.rb | 7 ------- lib/mission_control/jobs/engine.rb | 1 - 4 files changed, 5 insertions(+), 9 deletions(-) delete mode 100644 lib/active_job/scheduled.rb diff --git a/app/helpers/mission_control/jobs/jobs_helper.rb b/app/helpers/mission_control/jobs/jobs_helper.rb index 9c5a61af..9ea38980 100644 --- a/app/helpers/mission_control/jobs/jobs_helper.rb +++ b/app/helpers/mission_control/jobs/jobs_helper.rb @@ -26,6 +26,10 @@ def attribute_names_for_job_status(status) end end + def job_delayed?(job) + job.scheduled_at.before?(MissionControl::Jobs.scheduled_job_delay_threshold.ago) + end + private def renderable_job_arguments_for(job) job.serialized_arguments.collect do |argument| diff --git a/app/views/mission_control/jobs/jobs/scheduled/_job.html.erb b/app/views/mission_control/jobs/jobs/scheduled/_job.html.erb index 77cd23ff..127dc267 100644 --- a/app/views/mission_control/jobs/jobs/scheduled/_job.html.erb +++ b/app/views/mission_control/jobs/jobs/scheduled/_job.html.erb @@ -1,7 +1,7 @@ <%= link_to job.queue_name, application_queue_path(@application, job.queue) %> <%= bidirectional_time_distance_in_words_with_title(job.scheduled_at) %> - <% if job.scheduled_enqueue_delayed? %> + <% if job_delayed?(job) %>
delayed
<% end %> diff --git a/lib/active_job/scheduled.rb b/lib/active_job/scheduled.rb deleted file mode 100644 index ab78ad5c..00000000 --- a/lib/active_job/scheduled.rb +++ /dev/null @@ -1,7 +0,0 @@ -module ActiveJob::Scheduled - extend ActiveSupport::Concern - - def scheduled_enqueue_delayed? - status == :scheduled && scheduled_at.before?(MissionControl::Jobs.scheduled_job_delay_threshold.ago) - end -end diff --git a/lib/mission_control/jobs/engine.rb b/lib/mission_control/jobs/engine.rb index ff56ebdb..3a2102be 100644 --- a/lib/mission_control/jobs/engine.rb +++ b/lib/mission_control/jobs/engine.rb @@ -30,7 +30,6 @@ class Engine < ::Rails::Engine include ActiveJob::Querying include ActiveJob::Executing include ActiveJob::Failed - include ActiveJob::Scheduled ActiveJob.extend ActiveJob::Querying::Root end end From a54306f8766cfa1b608fd9ff6c2e673ada821dda Mon Sep 17 00:00:00 2001 From: Gary Tou Date: Sat, 2 Mar 2024 02:43:38 -0800 Subject: [PATCH 8/9] Add test for delayed scheduled jobs in `JobsControllerTest` --- test/controllers/jobs_controller_test.rb | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/test/controllers/jobs_controller_test.rb b/test/controllers/jobs_controller_test.rb index ac9871cf..230429e2 100644 --- a/test/controllers/jobs_controller_test.rb +++ b/test/controllers/jobs_controller_test.rb @@ -64,4 +64,23 @@ class MissionControl::Jobs::JobsControllerTest < ActionDispatch::IntegrationTest assert_select "tr.job", /DummyJob\s+Enqueued 2 minutes ago\s+queue_1\s+in 1 minute/ assert_select "tr.job", /DummyJob\s+Enqueued 2 minutes ago\s+queue_1\s+(1 minute ago|less than a minute ago)/ end + + test "get scheduled jobs when some are delayed" do + DummyJob.set(wait: 5.minutes).perform_later(37) + DummyJob.set(wait: 7.minutes).perform_later(42) + + get mission_control_jobs.application_jobs_url(@application, :scheduled) + assert_response :ok + assert_select "tr.job", 2 do # lists two jobs + assert_select "div.tag", text: /delayed/, count: 0 # no delayed tag + end + + travel 5.minutes + MissionControl::Jobs.scheduled_job_delay_threshold + 1.second + + get mission_control_jobs.application_jobs_url(@application, :scheduled) + assert_response :ok + assert_select "tr.job", 2 do # lists two jobs + assert_select "div.tag", text: /delayed/, count: 1 # total of one delayed tag + end + end end From d3ee4813249654c2a4bc2f0be3ea2e30df05eae0 Mon Sep 17 00:00:00 2001 From: Gary Tou Date: Sat, 2 Mar 2024 03:53:16 -0800 Subject: [PATCH 9/9] Remove test for failed jobs --- .../active_job/queue_adapters/adapter_testing.rb | 2 +- .../adapter_testing/job_statuses.rb | 16 ---------------- test/support/jobs_helper.rb | 6 ------ 3 files changed, 1 insertion(+), 23 deletions(-) delete mode 100644 test/active_job/queue_adapters/adapter_testing/job_statuses.rb diff --git a/test/active_job/queue_adapters/adapter_testing.rb b/test/active_job/queue_adapters/adapter_testing.rb index cb7b29a0..41236874 100644 --- a/test/active_job/queue_adapters/adapter_testing.rb +++ b/test/active_job/queue_adapters/adapter_testing.rb @@ -2,7 +2,7 @@ module ActiveJob::QueueAdapters::AdapterTesting extend ActiveSupport::Concern included do - include CountJobs, DiscardJobs, FindJobs, JobBatches, QueryJobs, Queues, RetryJobs, JobStatuses + include CountJobs, DiscardJobs, FindJobs, JobBatches, QueryJobs, Queues, RetryJobs setup do ActiveJob::Base.queue_adapter = queue_adapter diff --git a/test/active_job/queue_adapters/adapter_testing/job_statuses.rb b/test/active_job/queue_adapters/adapter_testing/job_statuses.rb deleted file mode 100644 index 1b48e6df..00000000 --- a/test/active_job/queue_adapters/adapter_testing/job_statuses.rb +++ /dev/null @@ -1,16 +0,0 @@ -module ActiveJob::QueueAdapters::AdapterTesting::JobStatuses - extend ActiveSupport::Concern - extend ActiveSupport::Testing::Declarative - - test "job is failed?" do - skip_unless_queue_adapter_supports_status :failed - - job = FailingJob.perform_later - assert_not job.failed? - - perform_enqueued_jobs - job = ActiveJob.jobs.failed.last - - assert job.failed? - end -end diff --git a/test/support/jobs_helper.rb b/test/support/jobs_helper.rb index 942cbdbb..ddb5d135 100644 --- a/test/support/jobs_helper.rb +++ b/test/support/jobs_helper.rb @@ -16,10 +16,4 @@ def within_job_server(app_id, server: nil, &block) def default_job_server MissionControl::Jobs.applications.first.servers.first end - - def skip_unless_queue_adapter_supports_status(status) - return if ActiveJob::Base.queue_adapter.supported_statuses.include? status - - skip "#{ActiveJob::Base.queue_adapter_name} does not support #{status} jobs" - end end