Move active jobs check to first query

bin/console

@@ -0,0 +1,15 @@
+#!/usr/bin/env ruby
+# frozen_string_literal: true
+
+require 'bundler/setup'
+require 'delayed_job_groups_plugin'
+
+# You can add fixtures and/or initialization code here to make experimenting
+# with your gem easier. You can also use a different console, if you like.
+
+# (If you use this, don't forget to add pry to your Gemfile!)
+# require "pry"
+# Pry.start
+
+require 'irb'
+IRB.start

lib/delayed/job_groups/complete_stuck_job_groups_job.rb

@@ -10,7 +10,9 @@ def enqueue(**kwargs)
       end
 
       def perform
-        Delayed::JobGroups::JobGroup.ready.find_each(&:check_for_completion)
+        Delayed::JobGroups::JobGroup.ready.with_no_open_jobs.find_each do |job_group|
+          job_group.check_for_completion(skip_pending_jobs_check: true)
+        end
       end
     end
   end

lib/delayed/job_groups/job_group.rb

@@ -23,6 +23,7 @@ class JobGroup < ActiveRecord::Base
                 dependent: :delete_all
 
       scope :ready, -> { where(queueing_complete: true, blocked: false) }
+      scope :with_no_open_jobs, -> { left_joins(:active_jobs).group(:id).having('count(delayed_jobs.id) == 0') }
 
       def mark_queueing_complete
         with_lock do
@@ -54,14 +55,14 @@ def cancel
         destroy
       end
 
-      def check_for_completion
-        self.class.check_for_completion(id)
+      def check_for_completion(skip_pending_jobs_check: false)
+        self.class.check_for_completion(id, skip_pending_jobs_check: skip_pending_jobs_check)
       end
 
-      def self.check_for_completion(job_group_id)
+      def self.check_for_completion(job_group_id, skip_pending_jobs_check: false)
         # Optimization to avoid loading and locking the JobGroup when the group
         # still has pending jobs
-        return if has_pending_jobs?(job_group_id)
+        return if !skip_pending_jobs_check && has_pending_jobs?(job_group_id)
 
         transaction do
           # The first completed job to notice the job group's queue count has dropped to

spec/delayed/job_groups/complete_stuck_job_groups_job_spec.rb

@@ -6,7 +6,12 @@
 
     let!(:blocked) { create(:job_group, blocked: true) }
     let!(:not_queueing_complete) { create(:job_group, queueing_complete: false) }
-    let!(:ready) { create(:job_group, queueing_complete: true, blocked: false) }
+    let!(:ready_without_jobs) { create(:job_group, queueing_complete: true, blocked: false) }
+    let!(:ready_with_jobs) do
+      create(:job_group, queueing_complete: true, blocked: false).tap do |job_group|
+        create(:delayed_job, job_group: job_group)
+      end
+    end
 
     before do
       allow(Delayed::JobGroups::JobGroup).to receive(:check_for_completion)
@@ -17,7 +22,7 @@
 
       expect(Delayed::JobGroups::JobGroup).to have_received(:check_for_completion)
                                                 .once
-                                                .with(ready.id)
+                                                .with(ready_without_jobs.id, skip_pending_jobs_check: true)
     end
   end
 

spec/delayed/job_groups/job_group_spec.rb

@@ -27,13 +27,24 @@
     Timecop.return
   end
 
-  describe "ready scope" do
-    let!(:blocked) { create(:job_group, blocked: true) }
-    let!(:not_queueing_complete) { create(:job_group, queueing_complete: false) }
-    let!(:ready) { create(:job_group, queueing_complete: true, blocked: false) }
+  describe "scopes" do
+    describe "ready" do
+      let!(:blocked) { create(:job_group, blocked: true) }
+      let!(:not_queueing_complete) { create(:job_group, queueing_complete: false) }
+      let!(:ready) { create(:job_group, queueing_complete: true, blocked: false) }
+
+      it "returns the expected job groups" do
+        expect(described_class.ready).to match_array(ready)
+      end
+    end
 
-    it "returns the expected job groups" do
-      expect(described_class.ready).to match_array(ready)
+    describe "with_no_open_jobs" do
+      let!(:job_group_with_jobs) { create(:delayed_job).job_group }
+      let!(:job_group_without_jobs) { subject }
+
+      it "returns groups with no jobs" do
+        expect(described_class.with_no_open_jobs).to match_array(job_group_without_jobs)
+      end
     end
   end
 

spec/factories/delayed_jobs.rb

@@ -0,0 +1,7 @@
+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :delayed_job, class: 'Delayed::Job' do
+    job_group { create(:job_group) }
+  end
+end