Skip to content

Commit

Permalink
fix!: infinite growth of cache when auto eviction is disabled
Browse files Browse the repository at this point in the history
See discussion here: overhangio/tutor#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.
  • Loading branch information
regisb authored and ormsbee committed Feb 14, 2024
1 parent 16e0333 commit 4daf452
Show file tree
Hide file tree
Showing 5 changed files with 7 additions and 6 deletions.
2 changes: 1 addition & 1 deletion cms/envs/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion cms/envs/devstack-experimental.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion lms/envs/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion lms/envs/devstack-experimental.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
5 changes: 3 additions & 2 deletions xmodule/modulestore/split_mongo/mongo_connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit 4daf452

Please sign in to comment.