catalyst / moodle-tool_forcedcache

Moodle MUC config managed deterministicly in code using rulesets
https://moodle.org/plugins/tool_forcedcache
9 stars 9 forks source link

Siteidentifier issues on master branch #54

Closed bwalkerl closed 2 months ago

bwalkerl commented 2 months ago

The latest siteidentifier changes on the master branch from https://github.com/catalyst/moodle-tool_forcedcache/pull/52 requires the setup.php changes from https://tracker.moodle.org/browse/MDL-71014 to work correctly.

Without this $CFG->siteidentifier won't be set so the cache stores will be disabled and caching won't be used. This can be seen on the /cache/admin.php page where the store mappings will show 'None', and caching won't be used.

Ideally we want to keep this fix, so we'd be looking to either add some extra logic to handle this scenario or explicitly add this backport to the list of required core patches.

matthewhilton commented 2 months ago

The reason we did #52 was because Moodle uses the siteidentifier as part of the cache, but the cache was used as part of querying the DB to get the siteidentifier - a chicken and egg situation.

The siteidentifier changes IIRC shouldn't require the code from MDL-71014 (but the tracker does help)

It should be being set (if not bootstrapped by MDL-71014) from here https://github.com/moodle/moodle/blob/d559cde424aad74cb9a5cd864fbd4f3f6ba5da15/lib/moodlelib.php#L1029

Is this not the case in your env ?

bwalkerl commented 2 months ago

Hi @matthewhilton most of my testing for this issue has been on Totara sites, which don't have that code specifically. When debugging the issue I did try adding that in, but without the rest of MDL-71014 it was still failing. I'll share some examples of this with you.