From 4daf4526204c733234672a71349f1a9dd9f482bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Behmo?= Date: Thu, 8 Feb 2024 10:37:46 +0100 Subject: [PATCH] fix!: infinite growth of cache when auto eviction is disabled See discussion here: https://github.com/overhangio/tutor/pull/984 This is a breaking change that will explicitely set the timeout of course structure cache entries to 1 week, instead of being unlimited. If you wish to revert to the former behaviour, you should set `course_structure_cache["TIMEOUT"] = None`. The course structure cache keys were previously set with an explicit timeout of `None` which was overriding the cache default timeout of 2h. This was OK in environments where the cache is configured with a maximum memory limit and an automatic key eviction strategy. But in others (such as Tutor), the course structure cache could grow infinitely. It was agreed that course structure cache keys should be long-lived but should respect the default cache structure timeout. Thus, we set here the TTL to 1 week. We can also configure Tutor to use a cache eviction policy. But that means we need to set a `maxmemory` value in Redis. It's not possible to set a value that will be appropriate for everyone: - if it's higher than the total memory (e.g: in small instances), server will crash before the cache is filled. - if it's too low (e.g: in large instances), the whole platform will abruptly slow down as many cache entries are suddenly evicted. That question of whether Tutor should define a cache eviction policy is still under discussion, but it can be resolved independently of this change. --- cms/envs/common.py | 2 +- cms/envs/devstack-experimental.yml | 2 +- lms/envs/common.py | 2 +- lms/envs/devstack-experimental.yml | 2 +- xmodule/modulestore/split_mongo/mongo_connection.py | 5 +++-- 5 files changed, 7 insertions(+), 6 deletions(-) diff --git a/cms/envs/common.py b/cms/envs/common.py index 8f974e73e521..9fd61ec8c76f 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -2193,7 +2193,7 @@ 'KEY_PREFIX': 'course_structure', 'KEY_FUNCTION': 'common.djangoapps.util.memcache.safe_key', 'LOCATION': ['localhost:11211'], - 'TIMEOUT': '7200', + 'TIMEOUT': '604800', # 1 week 'BACKEND': 'django.core.cache.backends.memcached.PyMemcacheCache', 'OPTIONS': { 'no_delay': True, diff --git a/cms/envs/devstack-experimental.yml b/cms/envs/devstack-experimental.yml index 54f6edf8f9f2..c08b19045faa 100644 --- a/cms/envs/devstack-experimental.yml +++ b/cms/envs/devstack-experimental.yml @@ -84,7 +84,7 @@ CACHES: KEY_PREFIX: course_structure LOCATION: - edx.devstack.memcached:11211 - TIMEOUT: '7200' + TIMEOUT: '604800' default: BACKEND: django.core.cache.backends.memcached.PyMemcacheCache OPTIONS: diff --git a/lms/envs/common.py b/lms/envs/common.py index 1b305f005958..bf11453e9826 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -1148,7 +1148,7 @@ 'KEY_PREFIX': 'course_structure', 'KEY_FUNCTION': 'common.djangoapps.util.memcache.safe_key', 'LOCATION': ['localhost:11211'], - 'TIMEOUT': '7200', + 'TIMEOUT': '604800', # 1 week 'BACKEND': 'django.core.cache.backends.memcached.PyMemcacheCache', 'OPTIONS': { 'no_delay': True, diff --git a/lms/envs/devstack-experimental.yml b/lms/envs/devstack-experimental.yml index 63812686e86a..149cf0982132 100644 --- a/lms/envs/devstack-experimental.yml +++ b/lms/envs/devstack-experimental.yml @@ -102,7 +102,7 @@ CACHES: KEY_PREFIX: course_structure LOCATION: - edx.devstack.memcached:11211 - TIMEOUT: '7200' + TIMEOUT: '604800' default: BACKEND: django.core.cache.backends.memcached.PyMemcacheCache OPTIONS: diff --git a/xmodule/modulestore/split_mongo/mongo_connection.py b/xmodule/modulestore/split_mongo/mongo_connection.py index 277aff28d4d7..964bb238c663 100644 --- a/xmodule/modulestore/split_mongo/mongo_connection.py +++ b/xmodule/modulestore/split_mongo/mongo_connection.py @@ -246,9 +246,10 @@ def set(self, key, structure, course_context=None): data_size = len(compressed_pickled_data) tagger.measure('compressed_size', data_size) - # Structures are immutable, so we set a timeout of "never" + # We rely on the course structure cache default timeout, which should be + # high by default (~ a few days). try: - self.cache.set(key, compressed_pickled_data, None) + self.cache.set(key, compressed_pickled_data) except Exception: # pylint: disable=broad-except total_bytes_in_one_mb = 1024 * 1024 chunk_size_in_mbs = round(data_size / total_bytes_in_one_mb, 2)