Skip to content

Commit

Permalink
"Force" discard jobs that are already running/runaway to prevent retry (
Browse files Browse the repository at this point in the history
#1073)

* Force discard jobs

Allows a new action to a running job: force discard. This will mark the
jobs as discarded (with a force discarded error message). Note that the
job will continue to run on the thread - that part is outside the scope
of this pr - but it will not be retried if the job fails. This can be
useful to stop a runaway job from being continuously restarted/retried.

* Add tests

* Lint and wrap promises with Rails executor

---------

Co-authored-by: Ben Sheldon [he/him] <bensheldon@gmail.com>
  • Loading branch information
jgrau and bensheldon committed Sep 19, 2023
1 parent 6c60a4a commit e597ad7
Show file tree
Hide file tree
Showing 16 changed files with 176 additions and 23 deletions.
7 changes: 7 additions & 0 deletions app/controllers/good_job/jobs_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
module GoodJob
class JobsController < GoodJob::ApplicationController
DISCARD_MESSAGE = "Discarded through dashboard"
FORCE_DISCARD_MESSAGE = "Force discarded through dashboard"

ACTIONS = {
discard: "discarded",
Expand Down Expand Up @@ -66,6 +67,12 @@ def discard
redirect_back(fallback_location: jobs_path, notice: t(".notice"))
end

def force_discard
@job = Job.find(params[:id])
@job.force_discard_job(FORCE_DISCARD_MESSAGE)
redirect_back(fallback_location: jobs_path, notice: t(".notice"))
end

def reschedule
@job = Job.find(params[:id])
@job.reschedule_job
Expand Down
59 changes: 36 additions & 23 deletions app/models/good_job/job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -215,32 +215,17 @@ def retry_job
# @return [void]
def discard_job(message)
with_advisory_lock do
execution = head_execution(reload: true)
active_job = execution.active_job(ignore_deserialization_errors: true)

raise ActionForStateMismatchError if execution.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
)
end

if active_job.respond_to?(:instrument)
active_job.send :instrument, :discard, error: job_error, &update_execution
else
update_execution.call
end
_discard_job(message)
end
end

# Force discard a job so that it will not be executed further. Force discard allows discarding
# a running job.
# This action will add a {DiscardJobError} to the job's {Execution} and mark it as finished.
def force_discard_job(message)
_discard_job(message)
end

# Reschedule a scheduled job so that it executes immediately (or later) by the next available execution thread.
# @param scheduled_at [DateTime, Time] When to reschedule the job
# @return [void]
Expand Down Expand Up @@ -277,5 +262,33 @@ def _execution_id
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)

raise ActionForStateMismatchError if execution.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
)
end

if active_job.respond_to?(:instrument)
active_job.send :instrument, :discard, error: job_error, &update_execution
else
update_execution.call
end
end
end
end
7 changes: 7 additions & 0 deletions app/views/good_job/jobs/_table.erb
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,13 @@
<%=t "good_job.actions.discard" %>
<% end %>
</li>
<li>
<% job_force_discardable = job.status.in? [:running] %>
<%= link_to force_discard_job_path(job.id), method: :put, class: "dropdown-item #{'disabled' unless job_force_discardable}", title: t("good_job.jobs.actions.force_discard"), data: { confirm: t("good_job.jobs.actions.confirm_force_discard"), disable: true } do %>
<%= render "good_job/shared/icons/eject" %>
<%=t "good_job.actions.force_discard" %>
<% end %>
</li>
<li>
<%= link_to retry_job_path(job.id), method: :put, class: "dropdown-item #{'disabled' unless job.status == :discarded}", title: t("good_job.jobs.actions.retry"), data: { confirm: t("good_job.jobs.actions.confirm_retry"), disable: true } do %>
<%= render "good_job/shared/icons/arrow_clockwise" %>
Expand Down
4 changes: 4 additions & 0 deletions app/views/good_job/shared/icons/_eject.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
<!-- https://icons.getbootstrap.com/icons/eject/ -->
<svg xmlns="http://www.w3.org/2000/svg" width="16" height="16" fill="currentColor" class="bi bi-eject" viewBox="0 0 16 16">
<path d="M7.27 1.047a1 1 0 0 1 1.46 0l6.345 6.77c.6.638.146 1.683-.73 1.683H1.656C.78 9.5.326 8.455.926 7.816L7.27 1.047zM14.346 8.5 8 1.731 1.654 8.5h12.692zM.5 11.5a1 1 0 0 1 1-1h13a1 1 0 0 1 1 1v1a1 1 0 0 1-1 1h-13a1 1 0 0 1-1-1v-1zm14 0h-13v1h13v-1z" />
</svg>
7 changes: 7 additions & 0 deletions config/locales/de.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ de:
actions:
destroy: Zerstören
discard: Verwerfen
force_discard: Verwerfen erzwingen
inspect: Prüfen
reschedule: Umplanen
retry: Wiederholen
Expand Down Expand Up @@ -110,10 +111,14 @@ de:
actions:
confirm_destroy: Sind Sie sicher, dass Sie den Job zerstören wollen?
confirm_discard: Sind Sie sicher, dass Sie den Job verwerfen wollen?
confirm_force_discard: 'Sind Sie sicher, dass Sie das Verwerfen dieses Jobs erzwingen möchten? Der Job wird als verworfen markiert, aber der laufende Job wird nicht gestoppt – er wird jedoch bei Fehlern nicht erneut versucht.
'
confirm_reschedule: Möchten Sie den Auftrag wirklich verschieben?
confirm_retry: Möchten Sie den Job wirklich wiederholen?
destroy: Arbeit vernichten
discard: Auftrag verwerfen
force_discard: Job verwerfen erzwingen
reschedule: Auftrag neu planen
retry: Job wiederholen
destroy:
Expand All @@ -124,6 +129,8 @@ de:
in_queue: in der Warteschlange
runtime: Laufzeit
title: Hinrichtungen
force_discard:
notice: Der Job wurde zwangsweise verworfen. Die Ausführung wird fortgesetzt, bei Fehlern wird der Vorgang jedoch nicht wiederholt
index:
job_pagination: Job-Paginierung
older_jobs: Ältere Berufe
Expand Down
7 changes: 7 additions & 0 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ en:
actions:
destroy: Destroy
discard: Discard
force_discard: Force discard
inspect: Inspect
reschedule: Reschedule
retry: Retry
Expand Down Expand Up @@ -110,10 +111,14 @@ en:
actions:
confirm_destroy: Are you sure you want to destroy the job?
confirm_discard: Are you usure you want to discard the job?
confirm_force_discard: 'Are you sure you want to force discard this job? The job will be marked as discarded but the running job will not be stopped - it will, however, not be retried on failures.
'
confirm_reschedule: Are you sure you want to reschedule the job?
confirm_retry: Are you sure you want to retry the job?
destroy: Destroy job
discard: Discard job
force_discard: Force discard job
reschedule: Reschedule job
retry: Retry job
destroy:
Expand All @@ -124,6 +129,8 @@ en:
in_queue: in queue
runtime: runtime
title: Executions
force_discard:
notice: Job has been force discarded. It will continue to run but it will not be retried on failures
index:
job_pagination: Job pagination
older_jobs: Older jobs
Expand Down
5 changes: 5 additions & 0 deletions config/locales/es.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ es:
actions:
destroy: Eliminar
discard: Descartar
force_discard: Forzar descarte
inspect: Inspeccionar
reschedule: Reprogramar
retry: Reintentar
Expand Down Expand Up @@ -110,10 +111,12 @@ es:
actions:
confirm_destroy: "¿Estás seguro que querés eliminar esta tarea?"
confirm_discard: "¿Estás seguro que querés descartar esta tarea?"
confirm_force_discard: "¿Está seguro de que desea forzar el descarte de este trabajo? El trabajo se marcará como descartado, pero el trabajo en ejecución no se detendrá; sin embargo, no se volverá a intentar en caso de falla.\n"
confirm_reschedule: "¿Estás seguro que querés reprogramar esta tarea?"
confirm_retry: "¿Estás seguro que querés reintentar esta tarea?"
destroy: Eliminar tarea
discard: Descartar tarea
force_discard: Forzar el descarte del trabajo
reschedule: Reprogramar tarea
retry: Reiuntentar tarea
destroy:
Expand All @@ -124,6 +127,8 @@ es:
in_queue: en cola
runtime: en ejecución
title: Ejecuciones
force_discard:
notice: Job ha sido descartado a la fuerza. Continuará ejecutándose pero no se volverá a intentar en caso de fallas.
index:
job_pagination: Paginación de tareas
older_jobs: Tareas anteriores
Expand Down
7 changes: 7 additions & 0 deletions config/locales/fr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ fr:
actions:
destroy: Détruire
discard: Mettre au rebut
force_discard: Forcer le rejet
inspect: Inspecter
reschedule: Reprogrammer
retry: Recommencez
Expand Down Expand Up @@ -110,10 +111,14 @@ fr:
actions:
confirm_destroy: Voulez-vous vraiment détruire le job ?
confirm_discard: Voulez-vous vraiment mettre au rebut le job ?
confirm_force_discard: 'Êtes-vous sûr de vouloir forcer l''abandon de cette tâche ? Le travail sera marqué comme abandonné mais le travail en cours d''exécution ne sera pas arrêté - il ne sera cependant pas réessayé en cas d''échec.
'
confirm_reschedule: Voulez-vous vraiment replanifier le job ?
confirm_retry: Voulez-vous vraiment réessayer le job ?
destroy: Détruire le job
discard: Mettre au rebut le job
force_discard: Forcer l'abandon du travail
reschedule: Replanifier le job
retry: Réessayer le job
destroy:
Expand All @@ -124,6 +129,8 @@ fr:
in_queue: Dans la file d'attente
runtime: Durée
title: Exécutions
force_discard:
notice: Le travail a été abandonné de force. Il continuera à fonctionner mais il ne sera pas réessayé en cas d'échec
index:
job_pagination: Pagination du job
older_jobs: Jobs plus anciens
Expand Down
7 changes: 7 additions & 0 deletions config/locales/ja.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ ja:
actions:
destroy: 削除する
discard: 破棄する
force_discard: 強制破棄
inspect: 調査する
reschedule: 再スケジュールする
retry: 再試行する
Expand Down Expand Up @@ -110,10 +111,14 @@ ja:
actions:
confirm_destroy: ジョブを削除してもよろしいですか?
confirm_discard: ジョブを破棄してもよろしいですか?
confirm_force_discard: 'このジョブを強制的に破棄してもよろしいですか?ジョブは破棄済みとしてマークされますが、実行中のジョブは停止されません。ただし、失敗した場合は再試行されません。
'
confirm_reschedule: ジョブを再スケジュールしてもよろしいですか?
confirm_retry: ジョブを再試行してもよろしいですか?
destroy: ジョブを削除
discard: ジョブを破棄
force_discard: ジョブを強制的に破棄する
reschedule: ジョブを再スケジュール
retry: ジョブを再試行
destroy:
Expand All @@ -124,6 +129,8 @@ ja:
in_queue: 待機中
runtime: 実行時間
title: 実行
force_discard:
notice: ジョブは強制的に破棄されました。実行は継続されますが、失敗した場合は再試行されません
index:
job_pagination: ジョブのページネーション
older_jobs: 古いジョブ
Expand Down
7 changes: 7 additions & 0 deletions config/locales/nl.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ nl:
actions:
destroy: Vernietigen
discard: Weggooien
force_discard: Forceer weggooien
inspect: Inspecteren
reschedule: Opnieuw plannen
retry: Opnieuw proberen
Expand Down Expand Up @@ -110,10 +111,14 @@ nl:
actions:
confirm_destroy: Weet je zeker dat je de baan wilt vernietigen?
confirm_discard: Wilt u de baan afwijzen?
confirm_force_discard: 'Weet u zeker dat u deze taak geforceerd wilt weggooien? De taak wordt gemarkeerd als verwijderd, maar de lopende taak wordt niet gestopt. Bij fouten wordt deze echter niet opnieuw geprobeerd.
'
confirm_reschedule: Weet u zeker dat u de taak opnieuw wilt inplannen?
confirm_retry: Weet u zeker dat u de taak opnieuw wilt proberen?
destroy: Baan vernietigen
discard: Gooi de baan weg
force_discard: Forceer taak weggooien
reschedule: Taak opnieuw plannen
retry: Taak opnieuw proberen
destroy:
Expand All @@ -124,6 +129,8 @@ nl:
in_queue: in de wachtrij
runtime: looptijd
title: Executies
force_discard:
notice: Baan is gedwongen verwijderd. Het blijft actief, maar wordt niet opnieuw geprobeerd als er fouten optreden
index:
job_pagination: Taak paginering
older_jobs: Oudere banen
Expand Down
7 changes: 7 additions & 0 deletions config/locales/ru.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ ru:
actions:
destroy: Разрушать
discard: Отказаться
force_discard: Принудительно отменить
inspect: Осмотреть
reschedule: Перенести
retry: Повторить попытку
Expand Down Expand Up @@ -134,10 +135,14 @@ ru:
actions:
confirm_destroy: Вы уверены, что хотите уничтожить задание?
confirm_discard: Вы уверены, что хотите отказаться от задания?
confirm_force_discard: 'Вы уверены, что хотите принудительно отменить это задание? Задание будет помечено как отброшенное, но выполняемое задание не будет остановлено, однако в случае сбоя оно не будет повторено.
'
confirm_reschedule: Вы уверены, что хотите перенести задание?
confirm_retry: Вы уверены, что хотите повторить задание?
destroy: Уничтожить работу
discard: Отменить задание
force_discard: Принудительно отменить задание
reschedule: Перенести задание
retry: Повторить задание
destroy:
Expand All @@ -148,6 +153,8 @@ ru:
in_queue: в очереди
runtime: время выполнения
title: Казни
force_discard:
notice: Иов был принудительно отброшен. Он продолжит работу, но не будет повторяться в случае сбоя.
index:
job_pagination: Пагинация вакансий
older_jobs: Старые рабочие места
Expand Down
7 changes: 7 additions & 0 deletions config/locales/tr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ tr:
actions:
destroy: Sil
discard: İptal Et
force_discard: Atmaya zorla
inspect: İncele
reschedule: Yeniden planla
retry: Tekrar dene
Expand Down Expand Up @@ -110,10 +111,14 @@ tr:
actions:
confirm_destroy: Bu işi silmek istediğinizden emin misiniz?
confirm_discard: Bu işi iptal etmek istediğinizden emin misiniz?
confirm_force_discard: 'Bu işi zorla iptal etmek istediğinizden emin misiniz? İş atıldı olarak işaretlenecek ancak devam eden iş durdurulmayacak; ancak başarısızlık durumunda yeniden denenmeyecek.
'
confirm_reschedule: Bu işi yeniden planlamak istediğinizden emin misiniz?
confirm_retry: Bu işi tekrar denemek istediğinizden emin misiniz?
destroy: İşi Sil
discard: İşi İptal Et
force_discard: İşi zorla atmaya zorla
reschedule: İşi Yeniden Planla
retry: İşi Tekrar Dene
destroy:
Expand All @@ -124,6 +129,8 @@ tr:
in_queue: sırada
runtime: çalışma süresi
title: İşlemler
force_discard:
notice: İş zorla atıldı. Çalışmaya devam edecek ancak arıza durumunda yeniden denenmeyecek
index:
job_pagination: İş sayfalandırması
older_jobs: Daha eski işler
Expand Down
Loading

0 comments on commit e597ad7

Please sign in to comment.