Community
OpenEnergyMonitor

Community

Multi-instance service-runner


(Paul) #21

I only suggest the concept is simple and more streamline. I am not entirely sure how easy it would be to implement, I would hope it is pretty simple but I’m put off for the same reasons as Brian, it’s not obvious what the current position is as it’s implemented in different ways in different places.

As I write this, I’m wondering if it has anything to do with user/permission levels. Is it the modules that are located outside www and use root to run that are not global perhaps? (just a guess)

From an uninformed position, I suspect a new connection to the global redis server can be initiated anytime, anywhere, the fact the SR code will use most of the emoncms redis settings but hardcode and overrule a prefix for the SR entries shouldn’t cause any excessive complication,

I do really hope that there is an easy way to do this in emoncms as I do not think there is a big enough user base to justify the time, effort and complexity of where the multi queue route might be leading. ( I really do appreciate your work and feel bad if the work done thus far isn’t needed if a single queue can be implemented) a single queue needs no additional installation or maintenance and is totally unseen to the majority of users.

I also think another settings file should be avoided at all costs.


(Greebo) #22

An environment variable under which userID? A Config file next to which script (there’ll be a script in each instance folder if multiple EmonCMS’s are installed - so why not use the settings.php that is already “next to” a script?)

The closest we’ve come so far is a hardcoded list of prefixes hand edited within the script. If you have multiple EmonCMS instances, you’re one of the 0.5% and you can just go and edit the script yourself.
Likewise if you went in and changed the instance prefix.

I wholeheartedly agree - there’s already sufficient complexity in dealing with locations of settings.

I’ve updated the first post with a hard-coded but easily editable list of instance prefixes.
I’d appreciate if you both could give it a test - @borpin on your “emoncms” named instance and @pb66, anywhere you may have multiple instances…


(Paul) #23

Bit of a chicken and egg situation, I do not have modules that use service-runner installed as service-runner was too emonpi specific and even the update flag is very emonpi specific so I have just swerved the whole lot.

I’m also afraid I have learnt the hard way not to experiment on live servers and currently it’s only the live servers that are multi-instance. If i’m going to take the plunge I will have to set up something specifically to test this with. I just haven’t been inclined to do so before now as a single instance service-runner wasn’t tempting enough.

I have just been looking through the various emoncms modules and I think I may have a stab at creating a service-runner function in Lib that can be used throughout emoncms where ever there is currently a rpush for service-runner flags.

It seems the other places a new redis connection is stsrted is as I suspected, when the connection is initiated outside the www path, therefore the globals are not available so there is no choice but to create a new connection.

What we need is basically the same thing but for a different reason, not because the global redis connection isn’t accesible, but because the global emoncms connection is not configured the way we want it (ie different prefix), so we can just create a new connection on demand of a service-runner call.

It’s not like the buttons will be clicked often so the additional per click connection is not going to be much more processor load. The alternatives are a 2nd global redis just for SR or a connection included in any PHP files that use SR, I think a Lib function is the better approach but it’s open to discussion.

We can just copy and re-purpose code from index.php. I can see this is what has been done for the other additional redis connections as the same “autentication” typo is in both the Sync module and the Postprocess module.

Even the flag content looks “odd” to me and it seems a lot of the flags are hardcoded emonpi specific stuff as well so it may take a while for me to get my head around it all.


(Greebo) #24

Fair enough.

I’ve got a spare Pi kicking about, I’ll chuck the latest EmonSD image onto it and then add a couple of extra EmonCMS instances and test it myself. Definitely going to be a few days before I can sort that out though. Probably wont hurt to have that anyway to test anythign we do come up with :slight_smile:


(Brian Orpin) #25

And I think that is a good thing that works with the 2 different default setings files that exist.

The problem is you have to do it on every update to emoncms as a git pull will complain! A file, in the same place as service-runner, that is ignored by git is still the simplest solution IMHO.

Getting service runner to also check for the presence of a config file and read the contents will just add to the flexibility. Do that alongside a note in the default settings files and I think there is enough flexibility to allow any user the ability to make it work.

Do you mean an OS User? My thought was they (environment variables) were user agnostic - but I don’t know (just know the main settings can be overridden by environment variables).

Yes but you can only use one of the service-runners. I asked a while back if it could be pulled off the settings.php, but that would only work for a single instance. So logically, the config file next to the active service runner script. Perhaps something else for the documentation.


(Paul) #26

So I just had a stab at this and whilst i have had a couple of issues, neither are directly related to implementing this as such.

  1. There is a problem with the process_settings.php file, when ever I try to include/require(_once) I get faults like these pop up momentarily in the emonpi update log window

it would seem require_once is the better option and that just spits out the 2 errors above, they relate to what appear to be the only 2 variables that are handled differently to the others

I messed around with this for a while before noticing they were different, and assuming that is for a reason and assuming it is handled better in other parts of emoncms, I tried everything to change the behavior within my code changes. Once I realised the differences I decided not to get sidetracked further and just hardcopied the redis settings array to my code so I could continue.

  1. Unbeknown to me, there is a “fudging” of the Redis prefix in the process_settings.php file, so by doing the above I also sidestepped this and spend a significant amount of time trying to determine why my prefixes and keys were concatenated without a colon.

Debugging the above took 10x longer than making the actual code changes.

I have had this up and running (using a hardcoded copy of the Redis settings array for now) and have produced 2 options so the code is a bit messy, but can be reduced and tidied once the setting issue is sort and a decision made about which option.

The changes to the existing code are minimal, just the 2 lines switched in emoncms/Modules/admin/admin_controller.php

                    require_once "Lib/service-runner-queue.php";
                    $result = issue_service_runner_flag("$update_script $argument>$update_logfile");
//                    $redis->rpush("service-runner","$update_script $argument>$update_logfile");
//                    $result = "service-runner trigger sent";

and the following added as `emoncms/Lib/service-runner-queue.php

<?php

/*

    All Emoncms code is released under the GNU Affero General Public License.
    See COPYRIGHT.txt and LICENSE.txt.

    ---------------------------------------------------------------------
    Emoncms - open source energy visualisation
    Part of the OpenEnergyMonitor project:
    http://openenergymonitor.org

*/

// no direct access
defined('EMONCMS_EXEC') or die('Restricted access');


function issue_service_runner_flag($flag_content) {
    //require_once "process_settings.php";
    //TEMPORARILY BORROWED SETTINGS AS ABOBE LINE ERRORS
    $redis_server = array('host'=>'localhost','port'=>6379,'auth'=>'','prefix' => 'emoncms');
    // fudging prefix ref https://github.com/emoncms/emoncms/blob/9.9.5/process_settings.php#L83
    if (!empty($redis_server['prefix'])) $redis_server['prefix'] = $redis_server['prefix'] . ":";

    $srq = new Redis();
    $srq_connected = $srq->connect($redis_server['host'], $redis_server['port']);
    if ($srq_connected) {
        if (!empty($redis_server['auth'])) {
            if (!$srq->auth($redis_server['auth'])) {
                return "service-runner failed auth";
            }
        }
        // opt1: No prefix just a common "emoncms-service-runner" key
        //       There is no way to identify the origin other than by the value
        //       The success message will indicate the position in global queue
//        $pos = $srq->rpush("emoncms-service-runner",$flag_content);


        // opt2: Prefixed "emoncms-service-runner" and instance redis prefix as key
        //       This allow the service-runner to identify origin (logs, user, paths???)
        //       The success message will indicate the instance specific position in queue
        $srq->setOption(Redis::OPT_PREFIX, 'emoncms-service-runner:'); // trailing colon on prefix required!!!
        // UNDO fudging prefix ref https://github.com/emoncms/emoncms/blob/9.9.5/process_settings.php#L83
        if (!empty($redis_server['prefix'])) $redis_server['prefix'] = rtrim($redis_server['prefix'], ':');
        $pos = $srq->rpush($redis_server['prefix'],$flag_content);

        return "service-runner trigger sent (queue position:$pos)";
    }
}

As it stands it will publish the exact same flag content to emoncms-service-runner:emoncms where emoncms is the Redis prefix, used as a sub-key so the service-runner can determine who set the flag, useful for log entries or if we want a different user etc. eg

[email protected]:/var/www/test/emoncms$ sudo redis-cli keys emon*
1) "emoncms-service-runner:test"
[email protected]:/var/www/test/emoncms$ sudo redis-cli lpop emoncms-service-runner:test
"/home/pi/emonpi/service-runner-update.sh emonpi>/home/pi/data/emonpiupdate.log"

I have changed the success message to include the position in the queue, if the Redis instance prefix is used as above, the queue will be instance specific.

There is also “opt2” which is to just have a single key for every instance to push flags to, this method will not inform the service-runner where the flag originated and the queue position is global.

[email protected]:/var/www/test/emoncms$ sudo redis-cli keys emon*
1) "emoncms-service-runner"
[email protected]:/var/www/test/emoncms$ sudo redis-cli lpop emoncms-service-runner
"/home/pi/emonpi/service-runner-update.sh emonpi>/home/pi/data/emonpiupdate.log"

to try opt1 just un-comment the last line of the “opt1” block and comment out all 3 lines used in the “opt2” block.

I think I prefer the former as it has more scope to use other users and paths etc and debugging might be easier. But it would involve some minor parsing in service-runner.py.

I can add this as a feature branch to try if you like but it’s a pretty simple edit, it would be good to get rid of the settings workaround and pick a option before I do. Any thoughts?

[edit - obviously the current SR code will need amending to which ever approach we settle on, I have just been working with the raw Redis values today, I still haven’t installed SR]


(Brian Orpin) #27

Great work Paul.

On prefix or no prefix, I think it is likely to be useful debug to know where the flag came from.

As an aside, should there be a check service-runner is actually running (feature creep). It occurs to me you can send the flags all you like, but if the service isn’t running… Pinch the code from the admin_view to check.


(Paul) #28

Good point. I’m sure there is plenty of other refinements needed too.

This is more of a PoC to gauge response before ploughing too much time into it. You specifically seemed resistant to this idea, but your comments above are more positive so maybe we are all close to agreement? (surely not! :smile:)


(Brian Orpin) #29

I didn’t mean to be. Actually pulling all the service-runner into one place makes much more sense and is a good design. They key is specific enough to be extremely unlikely to clash with anything.

Looks good I feel.


(Trystan Lea) #30

Sorry for the absence in the discussion. I’ve read through your discussion above, but probably need to re-read a couple of times. I liked the idea of posting to a common key with perhaps a specific redis prefix. Testing the following placed just below redis connection in emoncms/index.php seems to work:

if ($redis) {
    $redis->setOption(Redis::OPT_PREFIX, "test");
    $redis->set("service-runner","hello");
    $redis->setOption(Redis::OPT_PREFIX, $redis_server['prefix']);
}

If I set the prefix to emoncms in settings.php, it starts with the emoncms prefix, switches to test for the service-runner part and then back to emoncms.

The key generated seems to be missing a colon, it’s listed as:

testservice-runner

This seems like a simpler solution than having a settings file. If we keep the service-runner key in the ‘global’ context with an empty prefix then this change would not make any difference on existing emonSD systems…

Im sorry if im missing something by jumping in here at this point.


(Trystan Lea) #31

Checking if service runner is running in each module that uses it and providing feedback is a good idea too :slight_smile:


(Trystan Lea) #32

Ah sorry @pb66 you have covered much of this in your last post but one! and I see the note about the colon :slight_smile:


(Greebo) #33

Unfortunately, this is no different (in operation) to having multiple prefixes. redis keys is a “non-production usage” operation as it is considered expensive. (reference)

The redis exists command (which is used to determine if we have a flag) requires an exact key match.

This is where we need to be… If we need to track which instance pushed the flag, it could be easily dealt with in the start of the flag itself. if the flag starts with “instanceprefix^” that can be easily stripped off by the service runner and formed appropriately for a log entry. I just chose “^” as the separator as I couldn’t think of any valid bash command that could use it, anything else could do. If there’s no prefix, it can be left off completely and the service runner can just handle that.


(Brian Orpin) #34

Using Paul’s method, it can be checked in that function and the appropriate message returned.


(Greebo) #35

I’ll admit that my only exposure to redis has been this work on service-runner, so maybe there’s other alternatives too, but perhaps we could use a set to store service-runner flags? REDIS Set methods

use SADD to add new flags:
sadd service-runner <prefix> flag
use SCARD to check for any flags (after using exists to check if the set key exists)
scard service-runner
use SPOP to grab a random flag
spop service-runner

The only fly in that ointment is an empty prefix…

BTW, I really like the idea of a single central function controlling this - great stuff @pb66!


(Paul) #36

Is that method safe?

It is something I considered, but as I could not be sure if changing the global prefix setting might effect other Redis transactions that occurred elsewhere during the time it was changed. Small window I know, but unless it definitely cannot happen, i didn’t think it was worth the risk.

This change and then restore methed might end up being used in many places (update/postprocess/backup/sync etc) so that risk could escalate.


(Paul) #37

That sounds good to me, I prefer the idea of a single dedicated queue to simplify the SR end, but also liked passing the origin to the SR whilst trying to keep the flags the same to avoid upsetting the original authers and to hopefully reduce the changes you need to add to the SR. But if changing the flag content is ok, that’s the better way to go indeed.

I think that possibly an array would be better if possible array[0] is instance prefix array[1] is the command flag as it is now, this keeps the command string as a command without needing to be parsed, the separate instance prefix can then be used in log entries etc.

Sounds like somone’s been doing some homework! I do not a clue about any of this functionality so I have stuck to trying to reuse existing code rather than making huge changes.


(Greebo) #38

Is that a redis type? Remember we have to store this in a redis key…

If you call a quick skim of the massive redis docs “homework”… ROFL
Which leads on nicely from my previous comment about types… Here’s the types available in redis, Set being one of them, but perhaps a Hash is appropriate to achieve your suggestion above if we used a single queue…
We could have a hash of “prefix”, “command”, “logpath” which would save SR having to do any string parsing…

[EDIT] just reading about the redis “list” data types - that is what we’re currently using when we push a flag onto the redis queue for service-runner - so the service-runner key is already a list…
This is going to need some testing!

[EDIT #2] I just found reference to BLPOP - which is a blocking version of LPOP. That would mean we no longer need to poll the queue! That would make it infinitely more efficient.
From https://redis-py.readthedocs.io/en/latest/
image

I’ll have to take a look at how that might work in the service-runner code.


(Trystan Lea) #39

I’m pretty sure there is no risk of this. The PHP code in the web view of emoncms is all running on a single thread without asynchronous elements. Its all sequential so I cant see any risk?
It would be nice to avoid creating the second connection if its not needed.


(Brian Orpin) #40

Just as a point on that, I did a search for “new redis” on the emoncms repository and there are a number of occasions where a new connection appears to be made - it may just be my misunderstanding of how this is done though.