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

Feature: Graceful shutdown #965

Closed
sallory opened this issue Nov 18, 2023 · 2 comments · Fixed by #1048
Closed

Feature: Graceful shutdown #965

sallory opened this issue Nov 18, 2023 · 2 comments · Fixed by #1048
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@sallory
Copy link

sallory commented Nov 18, 2023

Hi

I would like to use FastStream in my project. However, zero downtime is crucial for me. Here's how I see it:

Handling SIGTERM -> the app should stop receiving new messages.

for handler in broker.handlers.values():
    await handler.close()

But not close the connection (because we need to proceed with all current messages, at least try)

Handling SIGKILL (INT) -> app can close handlers and the connection (now it works the same with SIGTERM and SIGINT)

We can wait for all tasks to be completed (I can kill it with a second signal) or wait for some timeout limit, like stop_grace_period in docker swarm.

In my case, tasks run a maximum of 15 secs, so I can do it like this:

async def on_shutdown():
    for handler in broker.handlers.values():
        await handler.close()
    await asyncio.sleep(15)

app.on_shutdown(on_shutdown)

But it's not a solution I guess

I'm thinking about waiting for all tasks:

tasks = asyncio.all_tasks() # should remove current task
done, pending = asyncio.wait(tasks, stop_grace_period)
# and we can inform that pending tasks are not completed

This code should run before App._shutdown and only if SIGTERM was received

I can try to implement it and create mr, what do you think guys?

@sallory sallory added the enhancement New feature or request label Nov 18, 2023
@davorrunje
Copy link
Collaborator

@sallory that's a great proposal! We would truly appreciate if you could create a PR for this feature.

@Lancetnik
Copy link
Member

Lancetnik commented Nov 22, 2023

Probably, we should just add asyncio.Lock to all handlers, take it in consume and wait for release in stop (with some timeout)

Broker already waits for all handlers shutdown, so we need to just stop them gracefully

@Lancetnik Lancetnik added the good first issue Good for newcomers label Nov 22, 2023
@Lancetnik Lancetnik moved this to Todo in FastStream Nov 22, 2023
@Lancetnik Lancetnik mentioned this issue Dec 13, 2023
13 tasks
@github-project-automation github-project-automation bot moved this from Todo to Done in FastStream Dec 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants