Skip to content

Commit

Permalink
Rename GoodJob::Job to GoodJob::Execution
Browse files Browse the repository at this point in the history
- Also renames `GoodJob::CurrentExecution` to `GoodJob::CurrentThread`
- Updates Dashboard controllers and views/partials; puts the "Active Job ID" column first in the table view; wraps UUIDs with `<code>` tags
  • Loading branch information
bensheldon committed Sep 13, 2021
1 parent de2184b commit 1484d1f
Show file tree
Hide file tree
Showing 40 changed files with 342 additions and 322 deletions.
5 changes: 3 additions & 2 deletions engine/app/controllers/good_job/active_jobs_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@
module GoodJob
class ActiveJobsController < GoodJob::BaseController
def show
@jobs = GoodJob::Job.where("serialized_params ->> 'job_id' = ?", params[:id])
.order(Arel.sql("COALESCE(scheduled_at, created_at) DESC"))
@executions = GoodJob::Execution.active_job_id(params[:id])
.order(Arel.sql("COALESCE(scheduled_at, created_at) DESC"))
raise ActiveRecord::RecordNotFound if @executions.empty?
end
end
end
33 changes: 17 additions & 16 deletions engine/app/controllers/good_job/dashboards_controller.rb
Original file line number Diff line number Diff line change
@@ -1,22 +1,22 @@
# frozen_string_literal: true
module GoodJob
class DashboardsController < GoodJob::BaseController
class JobFilter
class ExecutionFilter
attr_accessor :params

def initialize(params)
@params = params
end

def last
@_last ||= jobs.last
@_last ||= executions.last
end

def jobs
def executions
after_scheduled_at = params[:after_scheduled_at].present? ? Time.zone.parse(params[:after_scheduled_at]) : nil
sql = GoodJob::Job.display_all(after_scheduled_at: after_scheduled_at, after_id: params[:after_id])
.limit(params.fetch(:limit, 25))
sql = sql.with_job_class(params[:job_class]) if params[:job_class]
sql = GoodJob::Execution.display_all(after_scheduled_at: after_scheduled_at, after_id: params[:after_id])
.limit(params.fetch(:limit, 25))
sql = sql.job_class(params[:job_class]) if params[:job_class]
if params[:state]
case params[:state]
when 'finished'
Expand All @@ -34,15 +34,16 @@ def jobs

def states
{
'finished' => GoodJob::Job.finished.count,
'unfinished' => GoodJob::Job.unfinished.count,
'running' => GoodJob::Job.running.count,
'errors' => GoodJob::Job.where.not(error: nil).count,
'finished' => GoodJob::Execution.finished.count,
'unfinished' => GoodJob::Execution.unfinished.count,
'running' => GoodJob::Execution.running.count,
'errors' => GoodJob::Execution.where.not(error: nil).count,
}
end

def job_classes
GoodJob::Job.group("serialized_params->>'job_class'").count
GoodJob::Execution.group("serialized_params->>'job_class'").count
.sort_by { |name, _count| name }
end

def to_params(override)
Expand All @@ -54,9 +55,9 @@ def to_params(override)
end

def index
@filter = JobFilter.new(params)
@filter = ExecutionFilter.new(params)

count_query = Arel.sql(GoodJob::Job.pg_or_jdbc_query(<<~SQL.squish))
count_query = Arel.sql(GoodJob::Execution.pg_or_jdbc_query(<<~SQL.squish))
SELECT *
FROM generate_series(
date_trunc('hour', $1::timestamp),
Expand All @@ -81,11 +82,11 @@ def index

current_time = Time.current
binds = [[nil, current_time - 1.day], [nil, current_time]]
job_data = GoodJob::Job.connection.exec_query(count_query, "GoodJob Dashboard Chart", binds)
executions_data = GoodJob::Execution.connection.exec_query(count_query, "GoodJob Dashboard Chart", binds)

queue_names = job_data.map { |d| d['queue_name'] }.uniq
queue_names = executions_data.map { |d| d['queue_name'] }.uniq
labels = []
queues_data = job_data.to_a.group_by { |d| d['timestamp'] }.each_with_object({}) do |(timestamp, values), hash|
queues_data = executions_data.to_a.group_by { |d| d['timestamp'] }.each_with_object({}) do |(timestamp, values), hash|
labels << timestamp.in_time_zone.strftime('%H:%M %z')
queue_names.each do |queue_name|
(hash[queue_name] ||= []) << values.find { |d| d['queue_name'] == queue_name }&.[]('count')
Expand Down
10 changes: 10 additions & 0 deletions engine/app/controllers/good_job/executions_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# frozen_string_literal: true
module GoodJob
class ExecutionsController < GoodJob::BaseController
def destroy
deleted_count = GoodJob::Execution.where(id: params[:id]).delete_all
message = deleted_count.positive? ? { notice: "Job execution deleted" } : { alert: "Job execution not deleted" }
redirect_to root_path, **message
end
end
end
10 changes: 0 additions & 10 deletions engine/app/controllers/good_job/jobs_controller.rb

This file was deleted.

4 changes: 3 additions & 1 deletion engine/app/views/good_job/active_jobs/show.html.erb
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
<%= render 'good_job/shared/jobs_table', jobs: @jobs %>
<h1>ActiveJob ID: <code><%= @executions.first.id %></code></h1>

<%= render 'good_job/shared/executions_table', executions: @executions %>
2 changes: 1 addition & 1 deletion engine/app/views/good_job/cron_schedules/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
<th>Configuration</th>
<th>Class</th>
<th>Description</th>
<th>Next execution</th>
<th>Next scheduled</th>
</thead>
<tbody>
<% @cron_schedules.each do |job_key, job| %>
Expand Down
10 changes: 5 additions & 5 deletions engine/app/views/good_job/dashboards/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
<div class='me-4'>
<small>Filter by job class</small>
<br>
<% @filter.job_classes.each do |name, count| %>
<% @filter.job_classes.each do |(name, count)| %>
<% if params[:job_class] == name %>
<%= link_to(root_path(@filter.to_params(job_class: nil)), class: 'btn btn-sm btn-outline-secondary active', role: "button", "aria-pressed": true) do %>
<%= name %> (<%= count %>)
Expand Down Expand Up @@ -37,18 +37,18 @@
</div>
</div>

<% if @filter.jobs.present? %>
<%= render 'good_job/shared/jobs_table', jobs: @filter.jobs %>
<% if @filter.executions.present? %>
<%= render 'good_job/shared/executions_table', executions: @filter.executions %>

<nav aria-label="Job pagination" class="mt-3">
<ul class="pagination">
<li class="page-item">
<%= link_to({ after_scheduled_at: (@filter.last.scheduled_at || @filter.last.created_at), after_id: @filter.last.id }, class: "page-link") do %>
Next jobs <span aria-hidden="true">&raquo;</span>
Older executions <span aria-hidden="true">&raquo;</span>
<% end %>
</li>
</ul>
</nav>
<% else %>
<em>No jobs present.</em>
<em>No executions present.</em>
<% end %>
56 changes: 56 additions & 0 deletions engine/app/views/good_job/shared/_executions_table.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
<div class="card my-3">
<div class="table-responsive">
<table class="table card-table table-bordered table-hover table-sm mb-0">
<thead>
<tr>
<th>ActiveJob ID</th>
<th>Execution ID</th>
<th>Job Class</th>
<th>Queue</th>
<th>Scheduled At</th>
<th>Error</th>
<th>
ActiveJob Params&nbsp;
<%= tag.button "Toggle", type: "button", class: "btn btn-sm btn-outline-primary", role: "button",
data: { bs_toggle: "collapse", bs_target: ".job-params" },
aria: { expanded: false, controls: executions.map { |execution| "##{dom_id(execution, "params")}" }.join(" ") }
%>
</th>
<th>Actions</th>
</tr>
</thead>
<tbody>
<% executions.each do |execution| %>
<tr id="<%= dom_id(execution) %>">
<td>
<%= link_to active_job_path(execution.serialized_params['job_id']) do %>
<code><%= execution.active_job_id %></code>
<% end %>
</td>
<td>
<%= link_to active_job_path(execution.active_job_id, anchor: dom_id(execution)) do %>
<code><%= execution.id %></code>
<% end %>
</td>
<td><%= execution.serialized_params['job_class'] %></td>
<td><%= execution.queue_name %></td>
<td><%= execution.scheduled_at || execution.created_at %></td>
<td class="text-break"><%= truncate(execution.error, length: 1_000) %></td>
<td>
<%= tag.button "Preview", type: "button", class: "btn btn-sm btn-outline-primary", role: "button",
data: { bs_toggle: "collapse", bs_target: "##{dom_id(execution, 'params')}" },
aria: { expanded: false, controls: dom_id(execution, "params") }
%>
<%= tag.pre JSON.pretty_generate(execution.serialized_params), id: dom_id(execution, "params"), class: "collapse job-params" %>
</td>
<td>
<%= button_to execution_path(execution.id), method: :delete, class: "btn btn-sm btn-outline-danger", title: "Delete execution" do %>
<%= render "good_job/shared/icons/trash" %>
<% end %>
</td>
</tr>
<% end %>
</tbody>
</table>
</div>
</div>
48 changes: 0 additions & 48 deletions engine/app/views/good_job/shared/_jobs_table.erb

This file was deleted.

2 changes: 1 addition & 1 deletion engine/app/views/layouts/good_job/base.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
<div class="collapse navbar-collapse" id="navbarSupportedContent">
<ul class="navbar-nav me-auto">
<li class="nav-item">
<%= link_to "All Jobs", root_path, class: ["nav-link", ("active" if current_page?(root_path))] %>
<%= link_to "All Executions", root_path, class: ["nav-link", ("active" if current_page?(root_path))] %>
</li>
<li class="nav-item">
<%= link_to "Cron Schedules", cron_schedules_path, class: ["nav-link", ("active" if current_page?(cron_schedules_path))] %>
Expand Down
2 changes: 1 addition & 1 deletion engine/config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
root to: 'dashboards#index'
resources :cron_schedules, only: %i[index]
resources :active_jobs, only: %i[show]
resources :jobs, only: %i[destroy]
resources :executions, only: %i[destroy]

scope controller: :assets do
constraints(format: :css) do
Expand Down
4 changes: 2 additions & 2 deletions lib/good_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
module GoodJob
# @!attribute [rw] active_record_parent_class
# @!scope class
# The ActiveRecord parent class inherited by +GoodJob::Job+ (default: +ActiveRecord::Base+).
# The ActiveRecord parent class inherited by +GoodJob::Execution+ (default: +ActiveRecord::Base+).
# Use this when using multiple databases or other custom ActiveRecord configuration.
# @return [ActiveRecord::Base]
# @example Change the base class:
Expand Down Expand Up @@ -129,7 +129,7 @@ def self.cleanup_preserved_jobs(older_than: nil)
timestamp = Time.current - older_than

ActiveSupport::Notifications.instrument("cleanup_preserved_jobs.good_job", { older_than: older_than, timestamp: timestamp }) do |payload|
deleted_records_count = GoodJob::Job.finished(timestamp).delete_all
deleted_records_count = GoodJob::Execution.finished(timestamp).delete_all

payload[:deleted_records_count] = deleted_records_count
end
Expand Down
12 changes: 6 additions & 6 deletions lib/good_job/active_job_extensions/concurrency.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ def backtrace
next(block.call) unless job.class.queue_adapter.is_a?(GoodJob::Adapter)

# Always allow jobs to be retried because the current job's execution will complete momentarily
next(block.call) if CurrentExecution.active_job_id == job.job_id
next(block.call) if CurrentThread.active_job_id == job.job_id

enqueue_limit = job.class.good_job_concurrency_config[:enqueue_limit]
total_limit = job.class.good_job_concurrency_config[:total_limit]
Expand All @@ -30,12 +30,12 @@ def backtrace
key = job.good_job_concurrency_key
next(block.call) if key.blank?

GoodJob::Job.new.with_advisory_lock(key: key, function: "pg_advisory_lock") do
GoodJob::Execution.new.with_advisory_lock(key: key, function: "pg_advisory_lock") do
enqueue_concurrency = if enqueue_limit
# TODO: Why is `unscoped` necessary? Nested scope is bleeding into subsequent query?
GoodJob::Job.unscoped.where(concurrency_key: key).unfinished.advisory_unlocked.count
GoodJob::Execution.unscoped.where(concurrency_key: key).unfinished.advisory_unlocked.count
else
GoodJob::Job.unscoped.where(concurrency_key: key).unfinished.count
GoodJob::Execution.unscoped.where(concurrency_key: key).unfinished.count
end

# The job has not yet been enqueued, so check if adding it will go over the limit
Expand All @@ -62,8 +62,8 @@ def backtrace
key = job.good_job_concurrency_key
next if key.blank?

GoodJob::Job.new.with_advisory_lock(key: key, function: "pg_advisory_lock") do
allowed_active_job_ids = GoodJob::Job.unscoped.where(concurrency_key: key).advisory_locked.order(Arel.sql("COALESCE(performed_at, scheduled_at, created_at) ASC")).limit(perform_limit).pluck(:active_job_id)
GoodJob::Execution.new.with_advisory_lock(key: key, function: "pg_advisory_lock") do
allowed_active_job_ids = GoodJob::Execution.unscoped.where(concurrency_key: key).advisory_locked.order(Arel.sql("COALESCE(performed_at, scheduled_at, created_at) ASC")).limit(perform_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
Expand Down
16 changes: 8 additions & 8 deletions lib/good_job/adapter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ def initialize(execution_mode: nil, queues: nil, max_threads: nil, poll_interval
# 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::Job]
# @return [GoodJob::Execution]
def enqueue(active_job)
enqueue_at(active_job, nil)
end
Expand All @@ -54,29 +54,29 @@ def enqueue(active_job)
# 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::Job]
# @return [GoodJob::Execution]
def enqueue_at(active_job, timestamp)
good_job = GoodJob::Job.enqueue(
execution = GoodJob::Execution.enqueue(
active_job,
scheduled_at: timestamp ? Time.zone.at(timestamp) : nil,
create_with_advisory_lock: execute_inline?
)

if execute_inline?
begin
good_job.perform
execution.perform
ensure
good_job.advisory_unlock
execution.advisory_unlock
end
else
job_state = { queue_name: good_job.queue_name }
job_state[:scheduled_at] = good_job.scheduled_at if good_job.scheduled_at
job_state = { queue_name: execution.queue_name }
job_state[:scheduled_at] = execution.scheduled_at if execution.scheduled_at

executed_locally = execute_async? && @scheduler.create_thread(job_state)
Notifier.notify(job_state) unless executed_locally
end

good_job
execution
end

# Shut down the thread pool executors.
Expand Down
Loading

0 comments on commit 1484d1f

Please sign in to comment.