From 737c159e2d38d5139ed4ea976ba063432663e7a6 Mon Sep 17 00:00:00 2001 From: Eileen McNaughton Date: Tue, 22 Aug 2023 09:22:24 +1200 Subject: [PATCH 1/3] Only delete contact from sub-Redis keys if the 'all' key contains the contact --- CRM/Core/PrevNextCache/Redis.php | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/CRM/Core/PrevNextCache/Redis.php b/CRM/Core/PrevNextCache/Redis.php index 5b70cf74dc56..3705cef53c08 100644 --- a/CRM/Core/PrevNextCache/Redis.php +++ b/CRM/Core/PrevNextCache/Redis.php @@ -173,9 +173,12 @@ public function deleteItem($id = NULL, $cacheKey = NULL) { } elseif ($id !== NULL && $cacheKey !== NULL) { // Delete a specific contact, within a specific cache. - $this->redis->zRem($this->key($cacheKey, 'all'), $id); - $this->redis->zRem($this->key($cacheKey, 'sel'), $id); - $this->redis->hDel($this->key($cacheKey, 'data'), $id); + $deleted = $this->redis->zRem($this->key($cacheKey, 'all'), $id); + if ($deleted) { + // If they were in the 'all' key they might be in the more specific 'sel' and 'data' keys. + $this->redis->zRem($this->key($cacheKey, 'sel'), $id); + $this->redis->hDel($this->key($cacheKey, 'data'), $id); + } } elseif ($id !== NULL && $cacheKey === NULL) { // Delete a specific contact, across all prevnext caches. From fa54ce73b8b1979676391ef76ee96be0ba2130ef Mon Sep 17 00:00:00 2001 From: Eileen McNaughton Date: Tue, 22 Aug 2023 10:47:29 +1200 Subject: [PATCH 2/3] [REF] use function to get ttl Since this stuff needs a re-jig it seems good to switch to a function first --- CRM/Core/PrevNextCache/Redis.php | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/CRM/Core/PrevNextCache/Redis.php b/CRM/Core/PrevNextCache/Redis.php index 3705cef53c08..826049b7acd5 100644 --- a/CRM/Core/PrevNextCache/Redis.php +++ b/CRM/Core/PrevNextCache/Redis.php @@ -23,7 +23,7 @@ */ class CRM_Core_PrevNextCache_Redis implements CRM_Core_PrevNextCache_Interface { - const TTL = 21600; + private const TTL = 21600; /** * @var Redis @@ -37,15 +37,27 @@ class CRM_Core_PrevNextCache_Redis implements CRM_Core_PrevNextCache_Interface { /** * CRM_Core_PrevNextCache_Redis constructor. + * * @param array $settings */ - public function __construct($settings) { + public function __construct(array $settings) { $this->redis = CRM_Utils_Cache_Redis::connect($settings); $this->prefix = $settings['prefix'] ?? ''; $this->prefix .= \CRM_Utils_Cache::DELIMITER . 'prevnext' . \CRM_Utils_Cache::DELIMITER; } - public function fillWithSql($cacheKey, $sql, $sqlParams = []) { + /** + * Get the time-to-live. + * + * This is likely to be made configurable in future. + * + * @return int + */ + public function getTTL() : int { + return self::TTL; + } + + public function fillWithSql($cacheKey, $sql, $sqlParams = []): bool { $dao = CRM_Core_DAO::executeQuery($sql, $sqlParams, FALSE); [$allKey, $dataKey, , $maxScore] = $this->initCacheKey($cacheKey); @@ -89,7 +101,7 @@ public function markSelection($cacheKey, $action, $ids = NULL) { } elseif ($action === 'unselect' && $ids === NULL) { $this->redis->del($selKey); - $this->redis->expire($selKey, self::TTL); + $this->redis->expire($selKey, $this->getTTL()); } elseif ($action === 'unselect' && $ids !== NULL) { foreach ((array) $ids as $id) { @@ -231,9 +243,9 @@ private function initCacheKey($cacheKey) { $selKey = $this->key($cacheKey, 'sel'); $dataKey = $this->key($cacheKey, 'data'); - $this->redis->expire($allKey, self::TTL); - $this->redis->expire($dataKey, self::TTL); - $this->redis->expire($selKey, self::TTL); + $this->redis->expire($allKey, $this->getTTL()); + $this->redis->expire($dataKey, $this->getTTL()); + $this->redis->expire($selKey, $this->getTTL()); $maxScore = 0; foreach ($this->redis->zRange($allKey, -1, -1, TRUE) as $lastElem => $lastScore) { From 1a00aeccb414f5f8d89acdc76208b19a832cb459 Mon Sep 17 00:00:00 2001 From: Eileen McNaughton Date: Tue, 22 Aug 2023 11:26:47 +1200 Subject: [PATCH 3/3] Expire prev-next cache keys --- CRM/Core/PrevNextCache/Redis.php | 38 +++++++++++++++++++++----------- 1 file changed, 25 insertions(+), 13 deletions(-) diff --git a/CRM/Core/PrevNextCache/Redis.php b/CRM/Core/PrevNextCache/Redis.php index 826049b7acd5..0295db9dc270 100644 --- a/CRM/Core/PrevNextCache/Redis.php +++ b/CRM/Core/PrevNextCache/Redis.php @@ -37,10 +37,9 @@ class CRM_Core_PrevNextCache_Redis implements CRM_Core_PrevNextCache_Interface { /** * CRM_Core_PrevNextCache_Redis constructor. - * * @param array $settings */ - public function __construct(array $settings) { + public function __construct($settings) { $this->redis = CRM_Utils_Cache_Redis::connect($settings); $this->prefix = $settings['prefix'] ?? ''; $this->prefix .= \CRM_Utils_Cache::DELIMITER . 'prevnext' . \CRM_Utils_Cache::DELIMITER; @@ -57,16 +56,23 @@ public function getTTL() : int { return self::TTL; } - public function fillWithSql($cacheKey, $sql, $sqlParams = []): bool { + public function fillWithSql($cacheKey, $sql, $sqlParams = []) { $dao = CRM_Core_DAO::executeQuery($sql, $sqlParams, FALSE); [$allKey, $dataKey, , $maxScore] = $this->initCacheKey($cacheKey); - + $first = TRUE; while ($dao->fetch()) { [, $entity_id, $data] = array_values($dao->toArray()); $maxScore++; $this->redis->zAdd($allKey, $maxScore, $entity_id); + if ($first) { + $this->redis->expire($allKey, $this->getTTL()); + } $this->redis->hSet($dataKey, $entity_id, $data); + if ($first) { + $this->redis->expire($dataKey, $this->getTTL()); + } + $first = FALSE; } return TRUE; @@ -74,11 +80,18 @@ public function fillWithSql($cacheKey, $sql, $sqlParams = []): bool { public function fillWithArray($cacheKey, $rows) { [$allKey, $dataKey, , $maxScore] = $this->initCacheKey($cacheKey); - + $first = TRUE; foreach ($rows as $row) { $maxScore++; $this->redis->zAdd($allKey, $maxScore, $row['entity_id1']); + if ($first) { + $this->redis->expire($allKey, $this->getTTL()); + } $this->redis->hSet($dataKey, $row['entity_id1'], $row['data']); + if ($first) { + $this->redis->expire($dataKey, $this->getTTL()); + } + $first = FALSE; } return TRUE; @@ -94,9 +107,14 @@ public function markSelection($cacheKey, $action, $ids = NULL) { $selKey = $this->key($cacheKey, 'sel'); if ($action === 'select') { + $first = TRUE; foreach ((array) $ids as $id) { $score = $this->redis->zScore($allKey, $id); $this->redis->zAdd($selKey, $score, $id); + if ($first) { + $this->redis->expire($selKey, $this->getTTL()); + } + $first = FALSE; } } elseif ($action === 'unselect' && $ids === NULL) { @@ -124,16 +142,14 @@ public function getSelection($cacheKey, $action = 'get') { } return [$cacheKey => $result]; } - elseif ($action === 'getall') { + if ($action === 'getall') { $result = []; foreach ($this->redis->zRange($allKey, 0, -1) as $entity_id) { $result[$entity_id] = 1; } return [$cacheKey => $result]; } - else { - throw new \CRM_Core_Exception("Unrecognized action: $action"); - } + throw new \CRM_Core_Exception("Unrecognized action: $action"); } public function getPositions($cacheKey, $id1) { @@ -243,10 +259,6 @@ private function initCacheKey($cacheKey) { $selKey = $this->key($cacheKey, 'sel'); $dataKey = $this->key($cacheKey, 'data'); - $this->redis->expire($allKey, $this->getTTL()); - $this->redis->expire($dataKey, $this->getTTL()); - $this->redis->expire($selKey, $this->getTTL()); - $maxScore = 0; foreach ($this->redis->zRange($allKey, -1, -1, TRUE) as $lastElem => $lastScore) { $maxScore = $lastScore;