Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add MaintenanceTasks.stuck_task_duration configuration #949

Merged
merged 1 commit into from
Jan 15, 2024
Merged

Add MaintenanceTasks.stuck_task_duration configuration #949

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -897,6 +897,15 @@ controller class which **must inherit** from `ActionController::Base`.

If no value is specified, it will default to `"ActionController::Base"`.

#### Configure time after which the task will be considered stuck

To specify a time duration after which a task is considered stuck if it has not been updated,
you can configure `MaintenanceTasks.stuck_task_duration`. This duration should account for
job infrastructure events that may prevent the maintenance tasks job from being executed and cancelling the task.

The value for `MaintenanceTasks.stuck_task_duration` must be an `ActiveSupport::Duration`.
If no value is specified, it will default to 5 minutes.

### Metadata

`MaintenanceTasks.metadata` can be configured to specify a proc from which to
Expand Down
3 changes: 1 addition & 2 deletions app/models/maintenance_tasks/run.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ class Run < ApplicationRecord
:cancelled,
]
COMPLETED_STATUSES = [:succeeded, :errored, :cancelled]
STUCK_TASK_TIMEOUT = 5.minutes
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wondering about using #deprecate_constant, but MaintenanceTasks::Run is @api private, so I don't think we need to deprecate 👍 (Apps shouldn't be referencing this).


enum status: STATUSES.to_h { |status| [status, status.to_s] }

Expand Down Expand Up @@ -342,7 +341,7 @@ def pause
#
# @return [Boolean] whether the Run is stuck.
def stuck?
(cancelling? || pausing?) && updated_at <= STUCK_TASK_TIMEOUT.ago
(cancelling? || pausing?) && updated_at <= MaintenanceTasks.stuck_task_duration.ago
end

# Performs validation on the task_name attribute.
Expand Down
7 changes: 7 additions & 0 deletions lib/maintenance_tasks.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,4 +89,11 @@ module MaintenanceTasks
#
# @return [Proc] generates a hash containing the metadata to be stored on the Run
mattr_accessor :metadata, default: nil

# @!attribute stuck_task_duration
# @scope class
# The duration after which a task is considered stuck and can be force cancelled.
#
# @return [ActiveSupport::Duration] the threshold in seconds after which a task is considered stuck.
mattr_accessor :stuck_task_duration, default: 5.minutes
end
2 changes: 1 addition & 1 deletion test/jobs/maintenance_tasks/task_job_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class TaskJobTest < ActiveJob::TestCase
freeze_time
TaskJob.perform_later(@run)
@run.cancel
travel Run::STUCK_TASK_TIMEOUT
travel MaintenanceTasks.stuck_task_duration
@run.cancel # force cancel the Run
assert_predicate @run, :cancelled?
Maintenance::TestTask.any_instance.expects(:process).never
Expand Down
8 changes: 4 additions & 4 deletions test/models/maintenance_tasks/run_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -527,7 +527,7 @@ class RunTest < ActiveSupport::TestCase
)
refute_predicate run, :stuck?

travel Run::STUCK_TASK_TIMEOUT
travel MaintenanceTasks.stuck_task_duration
assert_predicate run, :stuck?
end

Expand All @@ -538,7 +538,7 @@ class RunTest < ActiveSupport::TestCase
task_name: "Maintenance::UpdatePostsTask",
status: status,
)
travel Run::STUCK_TASK_TIMEOUT
travel MaintenanceTasks.stuck_task_duration
refute_predicate run, :stuck?
end
end
Expand All @@ -554,7 +554,7 @@ class RunTest < ActiveSupport::TestCase
assert_predicate run, :cancelling?
assert_nil run.ended_at

travel Run::STUCK_TASK_TIMEOUT
travel MaintenanceTasks.stuck_task_duration
run.cancel
assert_predicate run, :cancelled?
assert_equal Time.now, run.ended_at
Expand Down Expand Up @@ -646,7 +646,7 @@ class RunTest < ActiveSupport::TestCase
assert_predicate run, :pausing?
assert_nil run.ended_at

travel Run::STUCK_TASK_TIMEOUT
travel MaintenanceTasks.stuck_task_duration
run.pause
assert_predicate run, :paused?
end
Expand Down
2 changes: 1 addition & 1 deletion test/system/maintenance_tasks/runs_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ class RunsTest < ApplicationSystemTestCase
assert_text "Cancelling…"
refute_button "Cancel"

travel Run::STUCK_TASK_TIMEOUT
travel MaintenanceTasks.stuck_task_duration

refresh
click_on "Cancel"
Expand Down