catalyst / moodle-tool_webanalytics

A Moodle admin tool adding Web Analytics to your Moodle site.
https://moodle.org/plugins/tool_webanalytics
7 stars 11 forks source link

Remove all overhead db / muc cache from bootstrap #34

Closed brendanheywood closed 3 years ago

brendanheywood commented 4 years ago

There is 2 application scope MUC hits

tool_webanalytics\records_manager::is_ready

dmitriim commented 3 years ago

Hi @brendanheywood can you please explain a bit more what do you mean by "be cached to $CFG or some combination of localizing this and then it can be mapped to APCu"? Any examples?

brendanheywood commented 3 years ago

There is spectrum of performance options, in order of easiest to best:

1) do nothing, it hits the db each time

2) create a new normal cache definition against the tool_webanalytics plugin. Easy but this also means we hit the shared redis cache and its another round trip outside the front end for data that is effectively static

We could also be slightly dodgy and just make a normal cache item as above, but in our inf map this to a local cache like apcu even though it technically isn't localizable. It would mean changes to the plugin conf might take a while to take affect but it would be fairly fine generally.

3) we store a summary of the config in a core config item rather than a plugin cache definition. This means it will be loaded for free on every bootstrap and its just waiting to be used with no extra overhead. Generally I wouldn't recommend polluting the main config space but this case adds overhead to every single page I think it is warranted. This is the way to go here because the data is very small. Just make sure it gets set again on every data mutation.

set_config('tool_webanalytics', json_encode($config));

4) If the data payload was very large but we still wanted it to be a localizable cache (eg apcu or local_file) then instead you'd just store a version key somewhere central such as $CFG and then use that key to load it from a local cache.This is the more proper way to do it but this is overkill for this specific use case.

dmitriim commented 3 years ago
  1. We don't hit DB each time as we have everything in shared cache https://github.com/catalyst/moodle-tool_webanalytics/blob/master/db/caches.php#L30
  2. It's done already.
  3. I can't get the difference between core config and the existing plugin cache.
  4. That could be an option. Do you have a good example?

With this landed https://github.com/catalyst/moodle-tool_webanalytics/pull/41 we won't have any DB hits, but just application cache to get our records as we are removing all set_config calls.

brendanheywood commented 3 years ago

The 4 options are mutually exclusive alternatives, they are not a checklist.

3 is the best way for this exact use case. The difference between using core and the plugin is that the core one is loaded automatically as part of the bootstrap so there is no extra overhead at all. So I want to undo 1/2 and replace it with 3.

4 is the best generic way for large and intermittently used cache items. But in this case it is actually a slight step backwards because you would have to do a lookup to get a version key and then use the version key to grab it from a local cache. It would make more sense if the payload was large and it wasn't used on every request, but it fails both of those rules of thumb. Sorry for presenting it in a confusing order above.

dmitriim commented 3 years ago

@brendanheywood Ok. I'll implement 3. We will need to cache records and not js code as we have "a track admins option" per analytics. Also this will help to avoid rewriting too much code. And will help to make supporting Totara branch easier. Thoughts?

brendanheywood commented 3 years ago

Yes this should just be at the data loading layer and nothing else should be aware of where the config was loaded from.