Revisiting `SKIP LOCKED` / advisory locks about good_job HOT 5 OPEN

Just on the query itself, I think priority ordering is what kills performance: Postgres either has to use an index ordered on priority and then filter for the scheduled_at/created_at; or it has to us an index ordered on scheduled_at/created_at and then reorder on priority. Either way, I don't believe it's possible to do a simple index query while supporting priority.

Some other peers don't actually offer priority ordering, and not everyone has this use case -- how about the ability to configure it on or off, if you don't need it, to avoid the performance hit? Is there a way to keep the code maintainable, while allowing opt-out of the performance-significant priority feature?

For many use cases, multiple queues with FIFO are enough, no need for priority within a queue (which after all, makes it no longer exactly a "queue", strictly!)

from good_job.

Some other peers don't actually offer priority ordering, and not everyone has this use case -- how about the ability to configure it on or off, if you don't need it, to avoid the performance hit?

I like that suggestion. It would be fairly simple to offer that as a configuration option, and then slightly change the Active Record query that dequeues jobs if it was set.

I think the most delicate part of providing different job querying patterns is database indexing: GoodJob would need to have migrations that create indexes for both patterns (over-indexing is its own performance problem), or describe that it's not as simple as only setting a configuration option. I'm approaching the "Maybe I should write a book about GoodJob"-stage 😁

from good_job.

❤️ Thank you for looking into this!

This matches up with my expectations of the benefit (database connections), the reality (there's no free lunch for performance), and the elephant in the room (stale locks and heartbeats).

Just on the query itself, I think priority ordering is what kills performance: Postgres either has to use an index ordered on priority and then filter for the scheduled_at/created_at; or it has to us an index ordered on scheduled_at/created_at and then reorder on priority. Either way, I don't believe it's possible to do a simple index query while supporting priority.

(I did make a change in #928 that means that scheduled_at is always populated to remove the OR part of the query).

With GoodJob, I don't think it's worthwhile to build out multiple locking strategies. So I'm working towards a heartbeat (built around the Process record). My thought is to have a lock_by_process_id and locked_at to imply a lock, and then lock_by_process_id will be joined against the Process for dealing with stale/expired.

Your experiment did remind me that Inline mode needs a solution, so that was really helpful 🙌🏻

btw, Cron doesn't do any locking, so you should be golden there.

I think your solution looks good. I can't really go line-by-line, but it seems pretty close to all of the things I'm working through (along with all the deprecation cycles involved).

I'd be happy if you wanted to Zoom pair program on it together and we could try to make the changes in GoodJob itself.

from good_job.

That would be awesome.. could we get in touch via a more real-time medium first? my email should be in bio. It doesn't seem as much about pairing though but about setting the direction first, which we should be aligned on for this to succeed. 🙌

It does seem that a compound index could work here, it has to be configured just right though. TBC.

from good_job.

@julik would you be able to DM me?

• Rails Link Slack (@bensheldon), or
• Rails or Ruby Discords (@bensheldon)

from good_job.