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

Cache serialized version of Attribute to save on space #20

Merged
merged 1 commit into from
Jul 14, 2017

Conversation

alukach
Copy link
Contributor

@alukach alukach commented Jul 13, 2017

Cadasta/cadasta-platform#1579 occurs when attempting to cache a large number of Attribute instances (407 in the case of the ticket's example). When serialized by the Python memcached library (via Pickle, I believe), this large number of model instances exceeded the 1MB limit of memcached This PR helps us avoid the failure by serializing/deserializing Attribute instances as lighter-weight dictionary objects rather than entire Django Model instances (along with their FK related model instances, I believe) when inserting/retrieving them to/from the cache.

@alukach alukach force-pushed the bugfix/caching-issues branch from 40119ba to 1448328 Compare July 13, 2017 04:22
@alukach
Copy link
Contributor Author

alukach commented Jul 13, 2017

Partially resolves Cadasta/cadasta-platform#1579. This change avoids errors without needing Cadasta/cadasta-platform#1650, however both are recommended as a) we want to use caching when we can as it was likely added for a good reason; and b) eventually someone will try to put a huge object into cache.

@alukach alukach changed the title Cache serialized version of Attribute, handle cache failures, legibility Cache serialized version of Attribute to save on space Jul 13, 2017
@amplifi amplifi requested a review from oliverroick July 13, 2017 13:01
@oliverroick oliverroick merged commit 9e7ee0a into master Jul 14, 2017
@oliverroick oliverroick deleted the bugfix/caching-issues branch July 14, 2017 09:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants