que-rb/que

Remove unnecessary trigger that notifies about job state transitions

ZimbiX opened this issue · 2 comments

In working on #340, we noted that Que has not just one, but two PostgreSQL notify triggers:

  • que_job_notify trigger
    que_listener_#{locker_pid} notification channel
    job_available notification message
    Source

  • que_state_notify trigger
    que_state notification channel
    job_change notification message
    Source

que_job_notify is used for the job_available notification message, telling a random locker (worker) that there's a new job ready to pick up. This is crucial for Que's listen/notify system (which sits alongside the polling system).

que_state_notify on the other hand, sends a message when a job transitions to another state (expired, finished, errored, scheduled, ready). Whilst this is interesting functionality, it does not appear to be, or have ever been, used for anything:

  • Nothing LISTENs to the que_state notification channel
  • The commit messages on the commits which added que_state_notify functionality offer no insight into why it was added - see the commits around 6729e65

Given that nothing is subscribing to the messages produced, que_state_notify appears to just be wasting database resources in calculating and producing these messages (related: #299).

Perhaps this was part of a feature that was never completed? @chanks, any chance you recall your intention with this?

I can't see a good reason to keep this functionality, so I'm thinking that unless we hear otherwise from anyone, we'll deprecate it in the next release, and remove it at some point after that.

Ha ha, yeah, I had a plan for a UI that would receive job state changes over a websocket so you could have a live view of what all your jobs were currently doing. I never built it out, though. And I think I wanted to get the notify logic into the migration so that people wouldn't need a second migration later.

I think it could still be cool, but feel free to remove it!

Thanks for the quick response, @chanks!

That makes a lot of sense. I've been meaning to check out que-web - that doesn't look like it uses it, but perhaps it could be updated to do so.