Release memory when a deamon exits #953
Conversation
|
The original discussion: #951 |
kopf/_core/engines/daemons.py
Outdated
| this_daemon = daemons[handler.id] | ||
| for running_daemon in memory.running_daemons.values(): | ||
| if running_daemon is not this_daemon: | ||
| if running_daemon.handler.selector.check(cause.resource): |
There was a problem hiding this comment.
Can you please remind me: what does this if condition do? Or, better phrased, what would be the case when the running daemons of this resource object X will not match the definition of the handler?
There was a problem hiding this comment.
running_daemon is the other daemon. If running_daemon and this_daemon are referencing the same Kubernetes object, then we can't free the body since it's still in-use by the other daemon.
Editing: I'm using this particular if just to confirm if they're pointing at the same Kubernetes object. If there's a better check, I'm happy to change it.
There was a problem hiding this comment.
Thanks for the clarification. I think this 2nd "if" is not needed.
The cross-object isolation happens much earlier, a few architectural levels above, i.e. when the individual objects are split into separate processing streams:
- First, in
watcher()/worker()using the(resource, uid)as the key (also seeget_uid()there). - Second, where the singular
memoryis extracted from the pluralmemoriesviamemories.recall().
When it comes to the memory & memory.running_daemons, it is already dedicated to a specific object. All other objects (of the same and other kinds) have their own memory struct.
So, to my understanding, the check for "other daemons except for the current one" is sufficient (which is done on the line above). The selector check is redundant — it will always be True because of such a split.
So, this 2nd "if" should be removed. Otherwise, the PR is clear, I am ready to merge and release it ASAP.
There was a problem hiding this comment.
if removed, new push coming shortly.
|
What are the chances of this change getting into the next release of kopf? We've been running with a patched version of kopf with these changes for months now. We'd really like to be able to go back to using the standard release without having to patch. |
|
Thank you! |
Release local cache of Kubernetes objects when a demon exits
If a daemon finishes, the live_fresh_body copy of the Kubernetes object won't be used again, as
long as no other daemons are referencing the same object. But it live_fresh_copy is never set
to None, so it will not be garbage collected.
This patch will mark live_fresh_body for garbage collection when a deamon exits, as long as
no other running daemon is referencing the same object.
Test suite completed without errors (log attached)
kopf-test-log.txt