bensheldon / good_job

Multithreaded, Postgres-based, Active Job backend for Ruby on Rails.
https://goodjob-demo.herokuapp.com/
MIT License
2.53k stars 190 forks source link

Whitespace in `queues` configuration can cause issues. #1351

Closed TAGraves closed 2 months ago

TAGraves commented 2 months ago

We had the following queues config for a worker:

"queues": "+workload_5s: 3; +workload_15s,workload_5s:3; +workload_1m,workload_15s,workload_5s:2; +workload_2m,workload_1m,workload_15s,workload_5s:2; +workload_5m,workload_2m,workload_1m,workload_15s,workload_5s:2",

We observed that with this configuration, workload_5m jobs were never being worked. I removed all the spaces from the above configuration:

"queues": "+workload_5s:3;+workload_15s,workload_5s:3;+workload_1m,workload_15s,workload_5s:2;+workload_2m,workload_1m,workload_15s,workload_5s:2;+workload_5m,workload_2m,workload_1m,workload_15s,workload_5s:2",

Then the workload_5m jobs started getting picked up. So it seems like the space between the ; and the +workload_5m, was causing an issue. I don't see it documented anywhere that whitespace is relevant, and indeed the readme even has several examples of whitespace being used in the queue configuration, so I suspect this is a bug.

bensheldon commented 2 months ago

GoodJob should be whitespace tolerant. I'm probably missing some strip's 🤔 Here's where the string gets broken down into a configuration hash:

Also, I love how you've set up your queues 😍

Let me make a test case, but I think it needs more striping

bensheldon commented 2 months ago

Also, I love how you've set up your queues 😍

Unrelated: my one piece of feedback would be not to use the +-queue ordering. That has performance downsides and you likely want to clear faster-SLO jobs first rather than the slower-SLO ones.