Default concurrency key

[bforma]

First off, great job on creating the GoodJob gem!

I'm using the latest version of GoodJob (3.19).

I've noticed that when I use the following config for concurrency controls, no controls are applied, because the concurrency key becomes nil:

Class MyJob < ApplicationJob
  good_job_control_concurrency_with(
    total_limit: 1,
  )

  def perform
    # perform job
  end
end

The following will enqueue two jobs (when no workers are running in any form):

MyJob.perform_later
MyJob.perform_later

However, when I change the concurrency control config (by providing a key generator) with the following, only a single job is enqueued (as I would expect):

good_job_control_concurrency_with(
  total_limit: 1,
  key: "MyJob-#{arguments.first}"
)

Am I missing something? I find this (default) behavior confusing, and would expect a default concurrency key being generated, when none is supplied. Correct me if I'm wrong, but I can't find anything about this in the docs as well.

I would expect something like this as a default (class name + serialized arguments as concurrency key):

-> { "#{self.class.name}-#{ActiveJob::Arguments.serialize(arguments)}" }

Thanks!

[bensheldon]

Good flag, there is not a default concurrency key assigned:

good_job/lib/good_job/active_job_extensions/concurrency.rb

Lines 92 to 106 in eb65463

	def good_job_concurrency_key
	@good_job_concurrency_key || _good_job_concurrency_key
	end
	# Generates the concurrency key from the configuration
	# @return [Object] concurrency key
	def _good_job_concurrency_key
	key = self.class.good_job_concurrency_config[:key]
	return if key.blank?
	key = instance_exec(&key) if key.respond_to?(:call)
	raise TypeError, "Concurrency key must be a String; was a #{key.class}" unless VALID_TYPES.any? { |type| key.is_a?(type) }
	key
	end

I imagine I deferred this because I couldn't think of a good default: should it be the classname (because it's defined on a class)... but what about when it's defined in ApplicationJob. Should it include arguments? Those can be high cardinality.

I think the simplest path here would be to call out in the Readme that a key must be provided.

[bforma]

I agree that setting a good default is hard. Adding advice about this in the documentation would be helpful 👍 .

Besides that I would still expect the following configuration to issue an error (or warning?), simply because no concurrency limits are applied:

good_job_control_concurrency_with(
  total_limit: 1,
)

[Earlopain]

In #1145 the job class was made the default concurrency key, the docs at https://github.com/bensheldon/good_job?tab=readme-ov-file#concurrency-controls also have been expanded on a bit.

I believe this can be closed now (I totally forgot to link to this issue in the PR 🤦)

[bforma]

Awesome, thanks!