Merge branch 'MDL-82511-cache-cron-MOODLE_500_STABLE' of https://github.com/bwalkerl/moodle into MOODLE_500_STABLE

cache/classes/helper.php

@@ -756,10 +756,16 @@ class helper {
                     debugging('Cache stores used for session definitions should ideally be searchable.', DEBUG_DEVELOPER);
                     continue;
                 }
+
+                // Load all of the keys into memory so we can compare against multiple prefixes.
+                $keys = $store->find_all();
+                sort($keys);
+
                 // Get all of the last access keys.
-                $keys = $store->find_by_prefix(session_cache::LASTACCESS);
+                $lastaccess = array_filter($keys, fn($key) => str_starts_with($key, session_cache::LASTACCESS));
                 $todelete = [];
-                foreach ($store->get_many($keys) as $key => $value) {
+                $prefixtodelete = [];
+                foreach ($store->get_many($lastaccess) as $key => $value) {
                     $expiresvalue = 0;
                     if ($value instanceof ttl_wrapper) {
                         $expiresvalue = $value->data;
@@ -771,11 +777,16 @@ class helper {
                     $expires = (int) $expiresvalue;
 
                     if ($expires > 0 && $expires < $purgetime) {
-                        $prefix = substr($key, strlen(session_cache::LASTACCESS));
-                        $foundbyprefix = $store->find_by_prefix($prefix);
-                        $todelete = array_merge($todelete, [$key], $foundbyprefix);
+                        $todelete[] = $key;
+                        $prefixtodelete[] = substr($key, strlen(session_cache::LASTACCESS));
                     }
                 }
+
+                // Match all of the prefixes to delete to keys to delete.
+                if ($prefixtodelete) {
+                    sort($prefixtodelete);
+                    $todelete = array_merge($todelete, self::filter_sorted_keys_by_prefixes($keys, $prefixtodelete));
+                }
                 if ($todelete) {
                     $outcome = (int) $store->delete_many($todelete);
                     if ($output) {
@@ -788,6 +799,37 @@ class helper {
         }
     }
 
+    /**
+     * Filters a sorted list of keys by a sorted list of prefixes.
+     * This relies on the sorting to reduce the number of comparisons.
+     *
+     * @param array $keys a sorted array of keys
+     * @param array $prefixes a sorted array of prefixes
+     * @return array of keys containing any of the prefixes
+     */
+    public static function filter_sorted_keys_by_prefixes(array $keys, array $prefixes): array {
+        // Reverse the prefixes to allow for simple processing.
+        $prefixes = array_reverse($prefixes);
+
+        $matches = [];
+        $prefix = array_pop($prefixes);
+        foreach ($keys as $key) {
+            // The keys and prefixes are sorted so we only need to compare against one prefix at a time.
+            // This is done inside a loop to check the next prefix at break points.
+            while ($prefix) {
+                if (str_starts_with($key, $prefix)) {
+                    $matches[] = $key;
+                } else if ($prefix < $key) {
+                    // The key has moved past the current prefix alphabetically, so check the next.
+                    $prefix = array_pop($prefixes);
+                    continue;
+                }
+                break;
+            }
+        }
+        return $matches;
+    }
+
     /**
      * Returns an array of stores that would meet the requirements for every definition.
      *

cache/stores/redis/lib.php

@@ -735,13 +735,34 @@ class cachestore_redis extends store implements
         return false;
     }
 
+    /**
+     * Finds all of the keys being used by this cache store instance using a scan.
+     * This is preferred over keys to avoid blocking the server for a long time.
+     *
+     * @param string $prefix
+     * @return array of all matching keys in the hash as a numbered array.
+     */
+    protected function scan_keys($prefix = '') {
+        $return = [];
+        $iterator = null;
+        do {
+            $results = $this->redis->hScan($this->hash, $iterator, "$prefix*", 1000);
+            if ($results !== false) {
+                foreach ($results as $key => $value) {
+                    $return[] = $key;
+                }
+            }
+        } while ($iterator != 0);
+        return $return;
+    }
+
     /**
      * Finds all of the keys being used by this cache store instance.
      *
      * @return array of all keys in the hash as a numbered array.
      */
     public function find_all() {
-        return $this->redis->hKeys($this->hash);
+        return $this->scan_keys();
     }
 
     /**
@@ -752,13 +773,7 @@ class cachestore_redis extends store implements
      * @return array List of keys that match this prefix.
      */
     public function find_by_prefix($prefix) {
-        $return = [];
-        foreach ($this->find_all() as $key) {
-            if (strpos($key, $prefix) === 0) {
-                $return[] = $key;
-            }
-        }
-        return $return;
+        return $this->scan_keys($prefix);
     }
 
     /**

cache/tests/cache_helper_test.php

@@ -60,4 +60,81 @@ final class cache_helper_test extends \advanced_testcase {
             [new \stdClass(), true],
         ];
     }
+
+    /**
+     * Test the filter_sorted_keys_by_prefixes method.
+     *
+     * @param array $keys
+     * @param array $prefixes
+     * @param array $expected
+     * @dataProvider filter_sorted_keys_by_prefixes_provider
+     */
+    public function test_filter_sorted_keys_by_prefixes(array $keys, array $prefixes, array $expected): void {
+        $this->assertEquals($expected, helper::filter_sorted_keys_by_prefixes($keys, $prefixes));
+    }
+
+    /**
+     * Data provider for filter_sorted_keys_by_prefixes tests.
+     *
+     * @return array
+     */
+    public static function filter_sorted_keys_by_prefixes_provider(): array {
+        return [
+            'simple match' => [
+                'keys' => ['aa', 'ab', 'ba', 'bb'],
+                'prefixes' => ['a'],
+                'expected' => ['aa', 'ab'],
+            ],
+            'multiple prefixes match' => [
+                'keys' => ['aa', 'ab', 'ba', 'bb', 'ca', 'cb'],
+                'prefixes' => ['a', 'c'],
+                'expected' => ['aa', 'ab', 'ca', 'cb'],
+            ],
+            'consecutive prefixes match' => [
+                'keys' => ['aa', 'ab', 'ba', 'bb', 'ca', 'cb'],
+                'prefixes' => ['a', 'b'],
+                'expected' => ['aa', 'ab', 'ba', 'bb'],
+            ],
+            'overlapping prefixes' => [
+                'keys' => ['a', 'ab', 'abc', 'abcd'],
+                'prefixes' => ['ab', 'abc'],
+                'expected' => ['ab', 'abc', 'abcd'],
+            ],
+            'exact match' => [
+                'keys' => ['a', 'b', 'c'],
+                'prefixes' => ['a', 'c'],
+                'expected' => ['a', 'c'],
+            ],
+            'duplicate keys' => [
+                'keys' => ['a', 'a', 'b', 'c'],
+                'prefixes' => ['a'],
+                'expected' => ['a', 'a'],
+            ],
+            'duplicate prefixes' => [
+                'keys' => ['a', 'b', 'c'],
+                'prefixes' => ['a', 'a', 'b'],
+                'expected' => ['a', 'b'],
+            ],
+            'unsorted keys boundry' => [
+                'keys' => ['c', 'b', 'a'],
+                'prefixes' => ['a'],
+                'expected' => [],
+            ],
+            'unsorted prefixes boundry' => [
+                'keys' => ['a', 'b', 'c'],
+                'prefixes' => ['d', 'a'],
+                'expected' => [],
+            ],
+            'empty keys' => [
+                'keys' => [],
+                'prefixes' => ['a'],
+                'expected' => [],
+            ],
+            'empty prefixes' => [
+                'keys' => ['a', 'b', 'c'],
+                'prefixes' => [],
+                'expected' => [],
+            ],
+        ];
+    }
 }