Unexpected behavior with max_threads = 1

[reczy]

Hi @bensheldon,

With max_threads = 1, good_job appears to be behaving unexpectedly.

Even with many pending jobs, good_job will perform 1 job, then wait the full polling period, then perform 1 job, and so on. Changing to max_threads = 2 and restarting the server leads to the expected behavior of performing all the jobs in quick succession (without waiting the full polling period between each perform).

GoodJob version "1.7.0"
Postgres version 12.5

# development.rb

config.active_job.queue_adapter = :good_job
config.good_job.execution_mode = :external
config.good_job.max_threads = 1
config.good_job.poll_interval = 10 # seconds

# Procfile

# ...

worker1: IS_GOODJOB=1 bundle exec good_job start

class TestJob < ApplicationJob
  queue_as :test_job

  def perform(opts = {})
    puts "I AM TEST JOB with #{opts}"
  end
end

# Enqueue a bunch of jobs

1000.times { TestJob.perform_later({ id: SecureRandom.uuid }) }

If it's at all helpful, with the above procedure, good_job does actually successfully complete about 20-25% of the jobs by the time all 1000 jobs are enqueued (meaning, it completes them very quickly without waiting for the next poll), then starts the unexpected behavior I described above.

Let me know if I can provide any additional information to be helpful.

[bensheldon]

@reczy wow, I think you've uncovered a big performance problem that I think should be a quick fix.

The explanation:

Every time a thread completes a job, it creates a new job thread (#create_thread):

good_job/lib/good_job/scheduler.rb

Lines 160 to 167 in b950c61

	# Invoked on completion of ThreadPoolExecutor task
	# @!visibility private
	# @return [void]
	def task_observer(time, output, thread_error)
	GoodJob.on_thread_error.call(thread_error) if thread_error && GoodJob.on_thread_error.respond_to?(:call)
	instrument("finished_job_task", { result: output, error: thread_error, time: time })
	create_thread if output
	end

That happens within the active job thread. Which means that the code within #create_thread that is checking whether there are threads available before accepting the job is rejecting it because there isn't an available job thread:

good_job/lib/good_job/scheduler.rb

Line 144 in b950c61

return nil unless @pool.ready_worker_count.positive?

I will change create_thread to ignore the available-threads check when it's triggered from there at the end of the active thread's job.

[bensheldon]

The fix for this is released in v1.7.1. Thank you again @reczy for flagging this 🙏

[reczy]

Happy to help - Thank you for being so responsive.

Confirmed that it's working as expected with max_threads = 1 on v1.7.1