Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-78154

Completely remove locking from MUC and reimplement requirelockingwrite

XMLWordPrintable

    • Icon: Improvement Improvement
    • Resolution: Won't Do
    • Icon: Major Major
    • None
    • 4.2
    • Caching
    • MOODLE_402_STABLE
    • Hide

      The only thing that could probably break is modinfo caching due to switching to a different locking implementation.

      Not sure how to test this properly because PHPUnit and Behat are not meant to test concurrent web access.

      Show
      The only thing that could probably break is modinfo caching due to switching to a different locking implementation. Not sure how to test this properly because PHPUnit and Behat are not meant to test concurrent web access.

      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:

      1. code is much much smaller and easier to review, test and maintain
      2. signalling in all cache store types is behaving the same way
      3. 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 
      4. cache purging is guaranteed to release all locks
      5. there can be multiple semaphores on one cache key
      6. each semaphore may define different timeout behaviour and timeout exception message
      7. developers would not be confused by 3 different types of locks in Moodle - there would be core\lock and core_cache\semaphore only

      Drawbacks:

      1. cache definition would need a new option to ignore sub-loaders for cache key with some prefix (or other significant API changes)
      2. 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

            skodak Petr Skoda
            skodak Petr Skoda
            Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

              Created:
              Updated:
              Resolved:

                Error rendering 'clockify-timesheets-time-tracking-reports:timer-sidebar'. Please contact your Jira administrators.