Jobs should have labels

[bensheldon]

I believe GoodJob should have the capability to label jobs with strings for informational purposes (e.g., aggregate the latency of all jobs with a certain label).

This thought came up (again!) as I was writing our my performance recommendations (again!) here: #1091 (reply in thread)

I have observed that developers frequently want to organize their jobs by a service dependency (e.g., everything that sends an email) or some business domain rule (e.g. process a user is separate from process an organization), and these signifiers have to go somewhere... and they end up in the name of the queue. This then makes it harder to performance tune because the signifiers, while meaningful, have little to do with the actual latency characteristics of the jobs.

GoodJob works around this by allowing for mixed-queue threadpools, but it's not particularly explicit.

My hypothesis is that if we created an obvious place for developers to put those non-performance-related signifiers (e.g. labelled jobs), it would be easier for developers to the name and organize their queues more appropriately for purpose.

Some other possible uses for labels:

• They could be used them as concurrency control keys
• They could be used for fuses/breakers (Job fuses / rapid job discard dashboard #749), e.g. to make it easy to temporarily ignore jobs that send emails if the email-sending service is having an incident)

[freesteph]

EDIT: my bad, it is not crashing the worker at all, false alert, sorry.

Has this enforced the presence of labels on jobs? I'm doing support on one of our products and it seems that an upgrade from 3.21.5 to 3.26.1 broke the worker that runs good_job. When it starts, this comes up:

2024-03-12 09:32:54.773764381 +0000 UTC [worker-1] DEPRECATION WARNING: GoodJob has pending database migrations. To create the migration files, run:
2024-03-12 09:32:54.773769197 +0000 UTC [worker-1] rails generate good_job:update
2024-03-12 09:32:54.773770202 +0000 UTC [worker-1] To apply the migration files, run:
2024-03-12 09:32:54.773770827 +0000 UTC [worker-1] rails db:migrate
2024-03-12 09:32:54.773771524 +0000 UTC [worker-1] (called from labels_migrated? at /app/vendor/bundle/ruby/3.3.0/gems/good_job-3.26.1/app/models/good_job/base_execution.rb:63)

However, labels have not been added/enabled on this project (put another way, no code other than the Gemfile.lock was changed for the upgrade). Is this line:

job_data["good_job_labels"] = Array(labels) if self.class.labels_migrated? && labels.present?

a bit too inquisitive?

Thank you.

[bensheldon]

EDIT: my bad, it is not crashing the worker at all, false alert, sorry.

No worries 🤗

For context, I consider deprecation warnings to be a "non-breaking change", but I also do tend to not put much effort into limiting/throttling them as I want folks to be aware they have pending migrations. Managing database schema changes within a Semantic Versioning scheme is unfortunately one of the hardest parts of developing a Rails Engine gem.

[freesteph]

Thanks for you reply and explanation. In the end it turned out the webserver could not reboot without the worker being stopped beforehand. The stacktrace was rather vague:

2024-03-12 12:41:52.748706429 +0100 CET [web-1] #<Thread:0x00007fd92aa46a68 /app/vendor/ruby-3.3.0/lib/ruby/3.3.0/open3.rb:664 run> terminated with exception (report_on_exception is true):
2024-03-12 12:41:52.748710381 +0100 CET [web-1] /app/vendor/ruby-3.3.0/lib/ruby/3.3.0/open3.rb:664:in `read': stream closed in another thread (IOError)

I saw threads and shutdown mentioned in the changelog, so it might have something to do with it but realistically this is a rather edgy case, I'll open an issue if it should ever surface again. Thanks again!

[bensheldon]

In the end it turned out the webserver could not reboot without the worker being stopped beforehand.

That sounds really odd; like I'm not quite sure what that means 😬 That error you posted looks like some kind of subprocess issue, and GoodJob doesn't do anything like that. Though I guess I could imagine the webserver being unhappy about something, but I'm not sure if that something would specifically be GoodJob.

btw, there's some Puma configuration under this heading that you should use if you aren't: https://github.com/bensheldon/good_job/?tab=readme-ov-file#execute-jobs-async--in-process

I'm happy to help if it comes up again 👍