Proposal

Tidy up locking options for cache definitions.

Background

MDL-67020 introduced the new cache defintion option `requirelockingbeforewrite`. This checks that a lock already exists for a given cache key before setting the key is attempted. This was added because when rebuilding coursemodinfo cache, the expensive job that you want the lock to protect is rebuilding all the data to store in the cache, before `set()` is called. The existing `requirelockingwrite` only protected actually writing the data into the cache, which is relatively quick.

Rationale

1. `requirelockingwrite` may be worth applying when a cache contains large amounts of data in a single key that may take a long time to write, but in that case it is also likely to take a long time to construct, so `requirelockingbeforewrite` is applicable. Even if the data is quick to construct but slow to write, `requirelockingbeforewrite` can be used in this case to ensure a lock is obtained before `set()` is called.

2. `requirelockingwrite` is not used in any cache definitions in Moodle core. As a result, it is poorly tested, and I have found at least one case where it does not work correctly (`cache_application::set_many`).

3. `requirelockingread` is not implemented consistently. `cache_application::get_many` creates a lock for each key, whereas `cache_application::get_implementation` expects a lock to exist already, as in `requirelockingbeforewrite`. It is also not implemented in any core caches.

Options

1. Create a new `requirelockingbeforeread` option which mirrors `requirelockingbeforewrite` exactly, then deprecate `requirelockingread` and `requirelockingwrite`. This gives us the flexibility we need in the cache API, while minimising the number of options to maintain. Plugin maintainers who have implemented the existing options will need to migrate to the new ones.
2. Create `requirelockingbeforeread`, make `requirelockingread` behave consistently, and fix the bugs with `requirelockingwrite`. Maintain all four options going forwards. This gives us maximum flexibility and convenience for plugin maintainers, but some duplication of functionality. Plugins written to expect the current behaviours will need updating.
3. Fix the issues with `requirelockingwrite` and `requirelockingread` so that they behave correctly and consistently, but don't add `requirelockingbeforewrite`. Plugins written to expect the current behaviours will need updating.
4. Leave everything as it is.

I favour option 1, to give us fewer options to maintain going forward.