volumio / Volumio2

Volumio 2 - Audiophile Music Player
http://volumio.org
Other
1.37k stars 315 forks source link

Plugin manager: improve load order priority management #1114

Open macmpi opened 7 years ago

macmpi commented 7 years ago

Plugins may have a set boot priority order number in package.json. They are then loaded by such boot priority order here: loading is essentially about invoking each pluginInstance.onVolumioStart() initialization routine.

However current implementation does NOT guaranties plugin(s) with boot priority 2 will be initialized after all plugin(s) with boot priority 1 initialization are completed. Therefore, this may lead to unpredictable results if a boot priority 2 plugin depends on higher priority boot priority 1 plugin initialization been completed. I took examples of 2 adjacent priorities here, but this could actually span over several priority numbers.

It is expected onVolumioStart() should not take too long to complete (good practice recommendation probably), but nothing can actually guaranty timing: it is at plugin developer's discretion to ensure.

Therefore, it would be very valuable if pluginmanager could ensure that plugins do complete their initialization by boot priority number batches at least, meaning: all the # 1 start & complete, then all # 2 start & complete, then...

I'm not familiar enough with promises programming to deal with such type of launch queues concept, but I hope this will be considered.

Thanks for feedback.

macmpi commented 7 years ago

It seems to turn-out that currently pluginmanager does not handle promises chains during plugin load (onVolumioStart()).

While trying manage that as per PR https://github.com/volumio/Volumio2/pull/1145, it seems some plugins reveal initialization issues:

EDIT: update in PR https://github.com/volumio/Volumio2/pull/1145 resolved items 2&3

macmpi commented 7 years ago

@volumio would you have some insight on following log, which looks a bit unexpected:

Apr 18 16:34:25 volumio volumio[1116]: info: Loading plugin "system"...
Apr 18 16:34:25 volumio volumio[1116]: info: Loading plugin "appearance"...
Apr 18 16:34:27 volumio volumio[1116]: info: Loading plugin "network"...
[...]
Apr 18 16:34:39 volumio volumio[1116]: info: Loading plugin "rest_api"...
Apr 18 16:34:39 volumio volumio[1116]: info: Loading plugin "websocket"...
Apr 18 16:34:39 volumio volumio[1116]: info: Plugin volspotconnect2 is not enabled
Apr 18 16:34:39 volumio volumio[1116]: info: ___________ START PLUGINS ___________
Apr 18 16:34:39 volumio volumio[1116]: PLUGIN START: appearance
Apr 18 16:34:39 volumio volumio[1116]: PLUGIN START: last_100
[...]
Apr 18 16:34:40 volumio volumio[1116]: info: BOOT COMPLETED
[...]
Apr 18 16:34:41 volumio volumio[1116]: Volumio Calling Home
[...]
Apr 18 16:34:44 volumio sudo[1173]: volumio : TTY=unknown ; PWD=/ ; USER=root ; COMMAND=/usr/sbin/i2cdetect -y 1
[...]
Apr 18 16:34:44 volumio volumio[1116]: info: Setting Device type: Raspberry PI

Last 3 log entries pertain to onVolumiostart() of system and i2s_dac plugins. Aren't plugin loads (onVolumiostart()) supposed to be all finished before ___________ START PLUGINS ___________ and everything finished before BOOT COMPLETED?

Should Volumio2/app/index.js better chain the various actions? Appreciate your views on this.

macmpi commented 7 years ago

https://github.com/volumio/Volumio2/pull/1162 intends to improve load/start/stop infrastructure. Boot priority number batches may be implemented after that PR is accepted. Scheduling issues (from previous post) can also be addressed after that same PR.

fideleinratio commented 7 years ago

Would like to contribute a solution for this. First, though, need to clarify the scope and requirement :

Please confirm: am assuming that the plugins should only be started after they have finished loading.

Question 1. What should boot priority apply to:

Question2: would there be a need to complete the priority1 onVolumioStart() and then onStart() before moving onto priority 2 plugins? (that is, Load AND Start all plugins by priority before moving on to the next plugins)

@macmpi, @fanciulli , @volumio , thanks in advance for your comments.

fideleinratio commented 7 years ago

And are there any other cases where the order of plugin changes could be important? For example should we execute onVolumioShutdown and onVolumioReboot in the reverse priority order?

fideleinratio commented 7 years ago

@macmpi have started looking at this.

In stepping through there are some parts of Volumio startup in app/index.js that are dependant on completion of at least the higher priority plugin loads.

Also, there are some plugin onStart functions that are dependant on those non-plugin parts of Volumio startup.

So can mostly ignore my previous comments and questions. The changes are grouped here: https://github.com/fideleinratio/Volumio2/pull/1 , which :

@macmpi , @fanciulli , @volumio please let me know if any comments or issues.