-
-
Notifications
You must be signed in to change notification settings - Fork 208
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
Add job-iteration integration #1217
Add job-iteration integration #1217
Conversation
I think it makes more sense for this to live in It's specifically this line that I have a concern because I don't want to be modifying someone's application configuration as a side-effect of installing GoodJob: JobIteration.interruption_adapter = .... |
That's fair, though note that I'll reach out to job-iteration's maintainers to see what they think. Can you let me know your thoughts on my 2nd question? |
We're working to change the way interruption adapters are configured in job-iteration to something that is based on the queue adapter name. In the new version it would look more like JobIteration.register_interruption_adapter(:good_job, ...) That interruption adapter would then automatically be used, but only for jobs that use GoodJob as their queue adapter. Point 2 is important - the interruption adapter should return |
Thanks @Mangara, the new version looks preferable since I think it's technically possible to use different backends on a per-job basis (though that's probably a very uncommon setup). Regarding point 2, I'm happy to submit a PR to GoodJob to make it possible to be notified as soon as a shutdown is requested, if @bensheldon is okay with the idea. |
Oops, sorry for overlooking the 2nd question. You're correct there isn't a It looks like the the GoodJob assigns some thread-local variable when executing jobs ( Caveat: Jobs that are run |
Thanks for the reply @bensheldon. job-iteration would need to check if the GoodJob scheduler is I'm happy to add a EDIT: I missed that |
We ended up using the Closing this PR as I am not planning on working on this any further at this time. |
Closing the loop on this, I added a methods for interrogating graceful shutdown: #1253 |
This PR adds an integration for the job-iteration gem.
job-iteration needs to know when the job worker is shutting down so that it can complete the current iteration and gracefully shut down. The gem ships with integrations for Sidekiq and Resque.
My first approach was to add an integration for GoodJob to the job-iteration gem. However that proved difficult because job-iteration's test harness uses a MySQL database, so I would have to either switch everything to Postgres or set up a multiple databases. Both of these sounded more complex than necessary, so I thought I'd add the integration to GoodJob instead.
I'm opening this PR as a draft to get some early feedback:
GoodJob.shutdown?
will only returntrue
after GoodJob is fully shut down (i.e. when all threads have shut down). If so, that's probably too late for job-iteration's purposes. The interruption adapters for Sidekiq and Resque returntrue
as soon as a shutdown is requested. AFAICT, there is no such hook in GoodJob at the moment. Can you confirm this is the case? If so, I'll update the PR accordingly.