Fix bug when selecting releasable executions and make sure these are reported

Via SolidQueue.on_thread_error, in test environment.

Author: rosa

app/models/solid_queue/blocked_execution.rb

@@ -26,10 +26,10 @@ def release_one(concurrency_key)
 
       private
         def releasable(concurrency_keys)
-          semaphores = Semaphore.where(key: concurrency_keys).pluck(:key, :value).index_by(&:key)
+          semaphores = Semaphore.where(key: concurrency_keys).select(:key, :value).index_by(&:key)
 
           # Concurrency keys without semaphore + concurrency keys with open semaphore
-          (concurrency_keys - semaphores.keys) | semaphores.select { |key, value| value > 0 }.map(&:first)
+          (concurrency_keys - semaphores.keys) | semaphores.select { |key, semaphore| semaphore.value > 0 }.map(&:first)
         end
     end
 

lib/solid_queue/scheduler.rb

@@ -39,6 +39,10 @@ def launch_concurrency_maintenance
           unblock_blocked_executions
         end
 
+        @concurrency_maintenance_task.add_observer do |_, _, error|
+          handle_thread_error(error) if error
+        end
+
         @concurrency_maintenance_task.execute
       end
 

test/dummy/app/jobs/sequential_update_result_job.rb

@@ -1,3 +1,3 @@
 class SequentialUpdateResultJob < UpdateResultJob
-  limits_concurrency key: ->(job_result, **) { job_result }, duration: 2.seconds
+  limits_concurrency key: ->(job_result, **) { job_result }
 end

test/dummy/config/application.rb

@@ -28,8 +28,6 @@ class Application < Rails::Application
     # config.eager_load_paths << Rails.root.join("extras")
 
     config.active_job.queue_adapter = :solid_queue
-
-    config.solid_queue.logger = ActiveSupport::Logger.new(nil)
     config.solid_queue.delete_finished_jobs = false
   end
 end

test/dummy/config/environments/development.rb

@@ -56,4 +56,6 @@
 
   # Uncomment if you wish to allow Action Cable access from any origin.
   # config.action_cable.disable_request_forgery_protection = true
+
+  config.solid_queue.logger = ActiveSupport::Logger.new(nil)
 end

test/dummy/config/environments/test.rb

@@ -47,4 +47,8 @@
 
   # Annotate rendered view with file names.
   # config.action_view.annotate_rendered_view_with_filenames = true
+
+  logger = ActiveSupport::Logger.new(STDOUT)
+  config.solid_queue.on_thread_error = ->(exception) { logger.error("#{exception.class.name}: #{exception.message}\n#{exception.backtrace.join("\n")}") }
+  config.solid_queue.logger = ActiveSupport::Logger.new(nil)
 end

test/integration/concurrency_controls_test.rb

@@ -94,20 +94,20 @@ class ConcurrencyControlsTest < ActiveSupport::TestCase
       assert SolidQueue::Semaphore.wait(job)
     end
 
-    # Now enqueue more jobs under that same key. They'll be all locked. Use priorities
-    # to ensure order.
+    # Now enqueue more jobs under that same key. They'll be all locked
     assert_difference -> { SolidQueue::BlockedExecution.count }, +10 do
       ("B".."K").each do |name|
         SequentialUpdateResultJob.perform_later(@result, name: name)
       end
     end
 
-    # Then unlock the semaphore: this would be as if the first job had released
-    # the semaphore but hadn't unblocked any jobs
+    # Then unlock the semaphore and expire the jobs: this would be as if the first job had
+    # released the semaphore but hadn't unblocked any jobs
+    SolidQueue::BlockedExecution.update_all(expires_at: 15.minutes.ago)
     assert SolidQueue::Semaphore.signal(job)
 
     # And wait for the scheduler to release the jobs
-    wait_for_jobs_to_finish_for(5.seconds)
+    wait_for_jobs_to_finish_for(3.seconds)
     assert_no_pending_jobs
 
     # We can't ensure the order between B and C, because it depends on which worker wins when
@@ -133,8 +133,12 @@ class ConcurrencyControlsTest < ActiveSupport::TestCase
       end
     end
 
+    # Simulate expiration of semaphore and executions
+    SolidQueue::Semaphore.where(key: job.concurrency_key).update_all(expires_at: 15.minutes.ago)
+    SolidQueue::BlockedExecution.update_all(expires_at: 15.minutes.ago)
+
     # And wait for scheduler to release the jobs
-    wait_for_jobs_to_finish_for(5.seconds)
+    wait_for_jobs_to_finish_for(3.seconds)
     assert_no_pending_jobs
 
     # We can't ensure the order between B and C, because it depends on which worker wins when