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

shutdown: Bias towards processing remaining work #15

Merged
merged 1 commit into from
Mar 1, 2022
Merged

Conversation

olix0r
Copy link
Owner

@olix0r olix0r commented Feb 28, 2022

shutdown::CancelOnShutdown wraps a Future or Stream so that the
task completes when shutdown is signaled. Currently, this is done by
stopping all additional work on these tasks as soon as the shutdown
signal is observed. This makes it difficult to ensure that in-flight
work is processed during shutdown.

This change alters the behavior so that CancelOnShutdown polls the
inner Future or Stream before polling the shutdown watch, so that
the shutdown watch is only polled when the inner instance is pending.
This will, in-turn, block the shutdown and runtime futures from
completing until all pending work is processed.

In pathological cases, this could prevent the process from terminating
gracefully. It seems better to the user or kubelet abort the application
if it doesn't complete its work in a timely fashion.

`shutdown::CancelOnShutdown` wraps a `Future` or `Stream` so that the
task completes when shutdown is signaled. Currently, this is done by
stopping all additional work on these tasks as soon as the shutdown
signal is observed. This makes it difficult to ensure that in-flight
work is processed during shutdown.

This change alters the behavior so that `CancelOnShutdown` polls the
inner `Future` or `Stream` before polling the shutdown watch, so that
the shutdown watch is only polled when the inner instance is pending.
This will, in-turn, block the shutdown and runtime futures from
completing until all pending work is processed.

In pathological cases, this could prevent the process from terminating
gracefully. It seems better to the user or kubelet abort the application
if it doesn't complete its work in a timely fashion.
Copy link
Collaborator

@hawkw hawkw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks correct to me!

@olix0r olix0r merged commit a7fcfe9 into main Mar 1, 2022
@olix0r olix0r deleted the ver/cancel-tests branch March 1, 2022 16:56
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 this pull request may close these issues.

2 participants