Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Used Django's builtin redis cache backend #1560

Merged
merged 2 commits into from
Sep 27, 2024

Conversation

bmispelon
Copy link
Member

@bmispelon bmispelon commented Jun 26, 2024

Fix for #1023

Two big questions that I don't have the answer to (yet?):

  • Should we re-populate the cache (somehow transferring the data from memcached to redis) as we deploy the default cache? How big of an impact would it have to clear the site's cache all at once?
  • Will the docs-pages cache change seamlessly work as we change the backend (I think yes, but I haven't tested)
  • Should we consider using hiredis to improve performance? I believe all that would be needed is to use redis[hiredis] in the requirements.

@bmispelon bmispelon force-pushed the redis-cache-use-django-builtin branch from 0d4414d to c218bbe Compare June 26, 2024 16:48
@alexgmin
Copy link
Contributor

Two big questions that I don't have the answer to (yet?):

  • Should we re-populate the cache (somehow transferring the data from memcached to redis) as we deploy the default cache? How big of an impact would it have to clear the site's cache all at once?

On this I see two options:

  • A custom backend that for reads, checks the redis first, and if not, it falls to memcached. And in a future release to remove the custom backend and default to the django one. This would slow the website until the redis one is filled, since for every redis cache miss it would mean hitting memcached and then writing to the redis one before responding.
  • A migration in which data is read from memcached and written into redis. Once the migration is done, change the backend to redis. The migration process could be a Django command that maybe could even have extra arguments to manage the number of cache items to be copied in each execution or to move only the keys that fit some pattern.

This would also depend on what's stored in the default cache, based on a quick look it seems to be some trac stats for every user and a generation key for versioning (?). If that's the only thing all of this preparation could not be needed at all since the volume would be so low.

  • Will the docs-pages cache change seamlessly work as we change the backend (I think yes, but I haven't tested)

Since the backend changes, I feel like it would need testing, since django-redis-cache could use a different serializer/compressor. I know both are supported options, but not sure if they are enabled by default.
I feel like this should be done after the default cache change to avoid risks.

  • Should we consider using hiredis to improve performance? I believe all that would be needed is to use redis[hiredis] in the requirements.

I'd suggest yes, but there's something to consider. Python 3.12 was released in October 2nd. However Python 3.12 wheels for hiredis-py weren't created until December 17th, I haven't checked in previous Python versions, but it could mean having to build the dependency in the future.

@bmispelon
Copy link
Member Author

Considering @alexgmin 's reply above and after giving it some thoughts, I realized that questions 1&2 are not super relevant.

Up until quite recently, we would restart the server regularly (to deal with a memory exhaustion issue that's since been fixed) and the site held up fine with this cold start.

For hiredis, I'd suggest letting the new configuration run for a while to get a sense of the baseline performance, and if we feel the need to improve it we can try hiredis later.

@bmispelon bmispelon merged commit 5c038b8 into django:main Sep 27, 2024
2 checks passed
@bmispelon bmispelon deleted the redis-cache-use-django-builtin branch September 27, 2024 15:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants