Plugin reload mechanism#5649
Conversation
- Wrap reload_plugins with mutex lock - Add methods for calculating plugin registry hash
- Background worker will correctly reload registry before performing tasks - Ensures that the background worker plugin regsistry is up to date
SchrodingersGat
added
bug
✅ Deploy Preview for inventree canceled.
|
|
@matmair review on this one please :) Much simpler this time around |
matmair
left a comment
matmair
left a comment
There was a problem hiding this comment.
Much simpler indeed. There are a few points where direct db access is needed, not sure how/if this will affect performance.
| logger.debug("Checking plugin registry hash") | ||
|
|
||
| try: | ||
| reg_hash = InvenTreeSetting.get_setting("_PLUGIN_REGISTRY_HASH", "", create=False, cache=False) |
There was a problem hiding this comment.
Comment: seems expensive to not cache this. We could alternatively override the hash in the cache if we recalculate it.
There was a problem hiding this comment.
The issue with caching is that the various processes have their own cache (not a shared cache) - unless setup with redis. So we run into the same issue again. We have to do a non-cached DB hit to see values updated by the other processes.
There was a problem hiding this comment.
Maybe we should adapt the settings functions in the future to optimize for that
|
Sorry, what do I missed? Why can't we reload the registry after installing a plugin? |
We do reload the registry after installing a plugin - but the background worker does not know that a plugin has been installed. So, this is a mechanism to ensure that the plugin registry is synced across all running workers. |
|
Ah, and there is no way to restart the workers from outside? |
|
Exactly - see further discussion here - django-q2/django-q2#123 |
SchrodingersGat
added
the
enhancement
3 participants
This PR fixes a long-running issue with the plugin ecosystem.
If a new plugin is installed into the registry (i.e. via the web interface) then the registry gets reloaded - but only for the running process.
If the background worker needs to access the new plugin (e.g. responding to an event) then this would fail - as the background worker has the "old" plugin registry.
This PR addresses this by keeping a hash of the plugins loaded into the registry, and updating a value in the database whenever the hash changes.
When any thread needs to call a plugin function, the registry first checks if the hash matches the latest value in the database. If not, the local registry is reloaded. This happens quite quickly and then the worker thread can proceed to perform the task.
This means that the whole server instance can keep running (without requiring a restart) when a new plugin is activated.
This PR also adds a mutex lock around the plugin registry reload procedure, to provide a safer and simpler reload process.
Edit: This is also the reason that the "auto migration" does not function correctly - the background worker does not know about the new app mixins, and thus cannot do the migrations. Once this is merged, I'll submit a fix for that issue.
Closes #5451