Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove unnecessary trigger that notifies about job state transitions #372

Open
ZimbiX opened this issue Aug 23, 2022 · 2 comments
Open

Remove unnecessary trigger that notifies about job state transitions #372

ZimbiX opened this issue Aug 23, 2022 · 2 comments

Comments

@ZimbiX
Copy link
Member

ZimbiX commented Aug 23, 2022

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.

@chanks
Copy link
Collaborator

chanks commented Aug 23, 2022

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!

@ZimbiX
Copy link
Member Author

ZimbiX commented Aug 24, 2022

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants