Originally the idea was to support cache stores that cannot do atomic data updates, this caused unnecessary complexity and confusion in MUC implementation. All current supported stores do atomic read/write operations.
Later the MUC locking was repurposed to implement locks for modinfo generation code - the idea was to lock local cache stores instead of using global lock to improve performance.
It should be possible and relatively easy to remove all MUC locking code. The deprecation can be done in the cache class with debugging messages.
Unfortunately MDL-67020 will have to be reworked, but we could at least fix all known linked problems at the same time. The idea is to move modinfo build caching outside of MUC and using a second caching key with prefix to indicate if modinfo cache is being built. This can be implemented with a new "cache semaphore" abstraction which could be later reused elsewhere for other expensive data stored in local caches.
Benefits of proposed cache semaphores compared to current MUC requirelockingbeforewrite:
- code is much much smaller and easier to review, test and maintain
- signalling in all cache store types is behaving the same way
- there are two different timeouts - shorter one that expires when waiting for one build, longer one that expires when waiting for other builds that won the lock - this could help with silent recovery from timeouts; exceptions could prevent some server overloading
- cache purging is guaranteed to release all locks
- there can be multiple semaphores on one cache key
- each semaphore may define different timeout behaviour and timeout exception message
- developers would not be confused by 3 different types of locks in Moodle - there would be core\lock and core_cache\semaphore only
Drawbacks:
- cache definition would need a new option to ignore sub-loaders for cache key with some prefix (or other significant API changes)
- static caches are not compatible with semaphores - not a problem for modinfo because it has external static caching
Prototype patch: https://github.com/moodle/moodle/compare/master...skodak:moodle:MDL-78154
Looking back adding external semaphore abstraction might not be the best idea due to the required sub-loader support. So instead here is another prototype that removes all locking except the cache::acquire_local() and friends, the whole semaphore idea is now hidden in cache class itself:
https://github.com/moodle/moodle/compare/master...skodak:moodle:MDL-78154b