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

Graceful shutdowns via Scheduler.wait() #48

Closed
meronym opened this issue Jul 5, 2018 · 4 comments · Fixed by #495
Closed

Graceful shutdowns via Scheduler.wait() #48

meronym opened this issue Jul 5, 2018 · 4 comments · Fixed by #495

Comments

@meronym
Copy link

meronym commented Jul 5, 2018

I'm writing an app that requires a few background jobs to fetch requests from a queue and process them atomically.

When the main app receives a shutdown signal, I'm firing an asyncio.Event to let the jobs know they need to close after they finish processing the task, and I then Job.wait() each of them.

It would be more convenient to do this via the Scheduler class. Currently it only has support for close()ing all the jobs, but not for wait()ing them. Does it make sense to add such a function? A simplified version would look like this:

class Scheduler:
    async def wait(self):
        await asyncio.gather(*[job.wait() for job in jobs])
@strongbugman
Copy link

This is a feature I want too, it is a base feature in my AntNest, I will not recreate wheel if aiojobs can provide this

@asvetlov
Copy link
Member

asvetlov commented Aug 7, 2018

There are design questions:

  1. Should sched.wait() be allowed to call only once or multiple times?
  2. If at time of waiting a new job was added should sched.wait() call wait the job too? Or should it wait for jobs scheduled at sched.wait() call only?
  3. The scheduler has pending jobs count limit. sched.spawn() can pause if too many jobs are scheduled. What is wait behavior for the case?

@strongbugman
Copy link

In my opinion, the wait()(maybe is not a good name) is a graceful close(), it should be same with close() for some behavior:

  1. if close() can be allowed to call only once, wait()should be called only once
  2. No jobs remain after close(), so wait() should wait all jobs, and it is what I need
  3. All jobs include pending jobs, wait() should wait those jobs too

@Dreamsorcerer
Copy link
Member

2. No jobs remain after `close()`, so `wait()` should wait all jobs, and it is what I need

But, close() actually cancels the tasks. If you are waiting on them normally, their normal operation may involve scheduling additional tasks, which may actually keep creating new tasks indefinitely, and then never end.

I think a wait_and_close() method which waits with a timeout before actually cancelling any remaining tasks and closing the scheduler would be a good solution. I've also proposed this for another issue.

If anyone wants to implement this, I'll happily take a look at this.

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

Successfully merging a pull request may close this issue.

4 participants