Skip to content

Commit

Permalink
feat: Support GoodJob v4
Browse files Browse the repository at this point in the history
This adds support to GoodJob v4. The main change is that queries now go
through `GoodJob::Job` instead of `GoodJob::Execution`, based on the
changelog:

> Enqueues and executes jobs via the GoodJob::Job model instead of GoodJob::Execution

They've also dropped support to Ruby 2.6/2.7, which we still support.

I've tweaked the existing Gemfile for AR 6.1 to run against GoodJob v3,
so we have test coverage there, and the main Gemfile and AR 7.0 versions
will run against v4 going forward.

Similarly, we have 2 sample apps for GoodJob, and I've kept one pointing
to GoodJob v3 (the multi-db sample), while upgrading the other to v4, so
we can test both for now.

https://island94.org/2024/07/introducing-goodjob-v4
  • Loading branch information
carlosantoniodasilva committed Jul 9, 2024
1 parent 7d1f89b commit 9fdc30a
Show file tree
Hide file tree
Showing 8 changed files with 55 additions and 30 deletions.
5 changes: 5 additions & 0 deletions .github/workflows/judoscale-good_job-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@ jobs:
ruby: "2.6"
- gemfile: Gemfile-activerecord-7-0
ruby: "2.6"
# GoodJob v4+ dropped support to Ruby 2.6/2.7, we test it against these Gemfiles.
- gemfile: Gemfile
ruby: "2.7"
- gemfile: Gemfile-activerecord-7-0
ruby: "2.7"
# AR 6.1 stable doesn't play nice with Ruby 3.2+. Fixes have been backported
# https://github.com/rails/rails/pull/46895, but no release is planned.
# To avoid testing against 6-1-stable branch, we'll just skip those for now.
Expand Down
1 change: 1 addition & 0 deletions judoscale-good_job/Gemfile-activerecord-6-1
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ gemspec name: "judoscale-good_job"

gem "judoscale-ruby", path: "../judoscale-ruby"
gem "activerecord", "~> 6.1"
gem "good_job", "~> 3.30"
gem "pg"
gem "minitest"
gem "rake"
2 changes: 1 addition & 1 deletion judoscale-good_job/judoscale-good_job.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -26,5 +26,5 @@ Gem::Specification.new do |spec|
spec.required_ruby_version = ">= 2.6.0"

spec.add_dependency "judoscale-ruby", Judoscale::VERSION
spec.add_dependency "good_job", ">= 3.0", "< 4.0"
spec.add_dependency "good_job", ">= 3.0", "< 5.0"
end
18 changes: 14 additions & 4 deletions judoscale-good_job/lib/judoscale/good_job/metrics_collector.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,19 +9,29 @@ module GoodJob
class MetricsCollector < Judoscale::JobMetricsCollector
include ActiveRecordHelper

def self.good_job_v4?
Gem::Version.new(::GoodJob::VERSION) >= Gem::Version.new("4")
end

def self.good_job_base_class
good_job_v4? ? ::GoodJob::Job : ::GoodJob::Execution
end

def self.adapter_config
Judoscale::Config.instance.good_job
end

def self.collect?(config)
super && ActiveRecordHelper.table_exists_for_model?(::GoodJob::Execution)
super && ActiveRecordHelper.table_exists_for_model?(good_job_base_class)
end

def initialize
super

@good_job_base_class = self.class.good_job_base_class

queue_names = run_silently do
::GoodJob::Execution.select("distinct queue_name").map(&:queue_name)
@good_job_base_class.select("distinct queue_name").map(&:queue_name)
end
self.queues |= queue_names
end
Expand All @@ -32,7 +42,7 @@ def collect

# logically we don't need the finished_at condition, but it lets postgres use the indexes
oldest_execution_time_by_queue = run_silently do
::GoodJob::Execution
@good_job_base_class
.where(performed_at: nil, finished_at: nil)
.group(:queue_name)
.pluck(:queue_name, Arel.sql("min(coalesce(scheduled_at, created_at))"))
Expand All @@ -42,7 +52,7 @@ def collect

if track_busy_jobs?
busy_count_by_queue = run_silently do
::GoodJob::Execution.running.group(:queue_name).count
@good_job_base_class.running.group(:queue_name).count
end
self.queues |= busy_count_by_queue.keys
end
Expand Down
17 changes: 13 additions & 4 deletions judoscale-good_job/test/metrics_collector_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -143,9 +143,14 @@ def clear_enqueued_jobs
it "tracks busy jobs when the configuration is enabled" do
use_adapter_config :good_job, track_busy_jobs: true do
Delayable.set(queue: "default").perform_later
::GoodJob::Execution.last.update!(performed_at: Time.now.utc)
metrics = nil
::GoodJob::JobPerformer.new("default").next do |execution|
# GoodJob v3 queries `GoodJob::Execution.running` scope, which uses `performed_at` in the condition,
# so we set it here. v4 uses `GoodJob::Job.running` scope, joined with the advisory lock instead.
execution.update!(performed_at: Time.now.utc)

metrics = subject.collect
metrics = subject.collect
end

_(metrics.size).must_equal 2
_(metrics[1].value).must_equal 1
Expand All @@ -158,9 +163,13 @@ def clear_enqueued_jobs
use_config log_level: :debug do
use_adapter_config :good_job, track_busy_jobs: true do
Delayable.set(queue: "default").perform_later
::GoodJob::Execution.last.update!(performed_at: Time.now.utc)
::GoodJob::JobPerformer.new("default").next do |execution|
# GoodJob v3 queries `GoodJob::Execution.running` scope, which uses `performed_at` in the condition,
# so we set it here. v4 uses `GoodJob::Job.running` scope, joined with the advisory lock instead.
execution.update!(performed_at: Time.now.utc)

subject.collect
subject.collect
end

_(log_string).must_match %r{good_job-qt.default=.+ good_job-busy.default=1}
end
Expand Down
12 changes: 6 additions & 6 deletions sample-apps/good_job-multi-db-sample/Gemfile.lock
Original file line number Diff line number Diff line change
@@ -1,21 +1,21 @@
PATH
remote: ../../judoscale-good_job
specs:
judoscale-good_job (1.7.0)
good_job (>= 3.0, < 4.0)
judoscale-ruby (= 1.7.0)
judoscale-good_job (1.7.1)
good_job (>= 3.0, < 5.0)
judoscale-ruby (= 1.7.1)

PATH
remote: ../../judoscale-rails
specs:
judoscale-rails (1.7.0)
judoscale-ruby (= 1.7.0)
judoscale-rails (1.7.1)
judoscale-ruby (= 1.7.1)
railties

PATH
remote: ../../judoscale-ruby
specs:
judoscale-ruby (1.7.0)
judoscale-ruby (1.7.1)

GEM
remote: https://rubygems.org/
Expand Down
2 changes: 1 addition & 1 deletion sample-apps/good_job-sample/Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,4 @@ gem "judoscale-ruby", path: "../../judoscale-ruby"
gem "judoscale-rails", path: "../../judoscale-rails"
gem "judoscale-good_job", path: "../../judoscale-good_job"

gem "good_job", "~> 3.7"
gem "good_job", "~> 4.0"
28 changes: 14 additions & 14 deletions sample-apps/good_job-sample/Gemfile.lock
Original file line number Diff line number Diff line change
@@ -1,21 +1,21 @@
PATH
remote: ../../judoscale-good_job
specs:
judoscale-good_job (1.7.0)
good_job (>= 3.0, < 4.0)
judoscale-ruby (= 1.7.0)
judoscale-good_job (1.7.1)
good_job (>= 3.0, < 5.0)
judoscale-ruby (= 1.7.1)

PATH
remote: ../../judoscale-rails
specs:
judoscale-rails (1.7.0)
judoscale-ruby (= 1.7.0)
judoscale-rails (1.7.1)
judoscale-ruby (= 1.7.1)
railties

PATH
remote: ../../judoscale-ruby
specs:
judoscale-ruby (1.7.0)
judoscale-ruby (1.7.1)

GEM
remote: https://rubygems.org/
Expand Down Expand Up @@ -57,13 +57,13 @@ GEM
raabro (~> 1.4)
globalid (1.2.1)
activesupport (>= 6.1)
good_job (3.99.0)
activejob (>= 6.0.0)
activerecord (>= 6.0.0)
concurrent-ruby (>= 1.0.2)
fugit (>= 1.1)
railties (>= 6.0.0)
thor (>= 0.14.1)
good_job (4.0.1)
activejob (>= 6.1.0)
activerecord (>= 6.1.0)
concurrent-ruby (>= 1.3.1)
fugit (>= 1.11.0)
railties (>= 6.1.0)
thor (>= 1.0.0)
i18n (1.14.5)
concurrent-ruby (~> 1.0)
loofah (2.22.0)
Expand Down Expand Up @@ -119,7 +119,7 @@ DEPENDENCIES
activejob (~> 7.0.1)
activemodel (~> 7.0.1)
activerecord (~> 7.0.1)
good_job (~> 3.7)
good_job (~> 4.0)
judoscale-good_job!
judoscale-rails!
judoscale-ruby!
Expand Down

0 comments on commit 9fdc30a

Please sign in to comment.