This repository was archived by the owner on Sep 14, 2020. It is now read-only.
Wait for instantly exiting daemons/timers for more than one asyncio cycle #360
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Generally, no magic constant should be left, everything must be a configurable value — since the configuration structures were added (some can be undocumented). This is just one of the leftovers with the module-level constants.
Previously, handlers were a reactor's module, and importing upper-level
handlers into lower-level structs.containers was not desired. Now, they
are both low-level, so we can import HandlerId into structs.containers.
The daemon_id is not needed also due to usage of self-presentation of
daemons & timers (previously, `f"Daemon {daemon_id}"` was used always).
There is nothing "instant" in the world, everything takes time. We have to specifically define what we consider as "instant" before switching to external resource patching & status polling. These are quite advanced features, so they are not reflected in the configuration docs. A docstring should be enough.
🤖 zincr found 0 problems , 1 warningDetails on how to resolve are provided below Large CommitsChecks all commits for large additions to a single file. Large commits should be reviewed more carefully for potential copyright and licensing issues This file contains a substantial change, please review to determine if the change comes from an external source and if there are any copyright or licensing issues to be aware of
|
This was referenced May 8, 2020
mnarodovitch
approved these changes
May 11, 2020
Closed
5 tasks
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What do these changes do?
Fix an issue in 0.27rc5 when resource deletion hangs for daemons/timers combined with other handlers — due to not allowing them a fair chance to exit instantly, despite they can.
Description
Previously, when a daemon/timer was capable of exiting instantly, it didn't have enough event loop cycles to do that — only one cycle was given with only one
asyncio.sleep(0), while there could be a fewawaitcalls both in the daemon itself and in the framework's wrapping routines (even if it is zero-time,awaitgives control back to the event loop).This is especially important for timers, which spent most of their time in sleeping until the next trigger, so they definitely can exit instantly, but didn't have a chance due to 2-3
awaituses internally.Now, Kopf will give 10 zero-time asyncio cycles to such daemons/timers — i.e. up to 10 potential
awaitstatements since the stopper is set or the task is cancelled — and this is configurable. Alternatively, instead of the cycles, a short timeout can be used (e.g. 0.001-0.01-0.1 seconds or so) — also configurable.This PR fixes the most common case when #356 can happen: in case of daemons and timers with "instant exit" implementation. The issue is still possible in case of actually slowly existing daemons combined with specific operator setup (due to no-op patches) — to be fixed separately in #???.
Additionally, some cleanup is performed in the code:
daemon_idis removed since it is not needed anymore.Issues/PRs
Type of changes
Checklist
Documentation reflects the changesCONTRIBUTORS.txtThe settings are intentionally not documented in the docs, in order to not over-complicate the docs. These settings are extremely advanced, and the defaults should be enough for most or all uses-cases. Those who might ever need to change it, will probably dive deep to the source code to find it themselves.