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

Remove above-ndb memcaching #6496

Open
BenHenning opened this issue Mar 22, 2019 · 2 comments
Open

Remove above-ndb memcaching #6496

BenHenning opened this issue Mar 22, 2019 · 2 comments
Labels
architectural refactor Architectural refactors that clean up the codebase backend enhancement Label to indicate an issue is a feature/improvement Impact: High Blocks or significantly slows down a core workflow. needs design doc Work: Medium The means to find the solution is clear, but it isn't at good-first-issue level yet.

Comments

@BenHenning
Copy link
Sponsor Member

Per https://cloud.google.com/appengine/docs/standard/python/ndb/cache, NDB performs both in-context and NDB caching for all reads and writes, and does auto-batching for group operations. This is enabled by default, and likely leads to our own memcache operations being completely redundant. Also, NDB properly handles race conditions that can arise when reading and writing to memcache without compare-and-swap operations, which Oppia doesn't currently do.

To summarize, removing our memcaching will likely result in:

  1. Less race conditions since we aren't handling memcache race conditions
  2. More performant caching since NDB includes in-context caching
  3. Simpler code since working with memcache is complex

We can still use memcache for caching outputs of expensive computations, but I don't think we do that anywhere (we generally rely on NDB for this purpose).

@BenHenning
Copy link
Sponsor Member Author

/cc @seanlip

@BenHenning
Copy link
Sponsor Member Author

One other benefit: NDB properly handles memcache updates within transactions (whereas we might not be doing this correctly now, but we rarely use transactions so I doubt we actually see bugs from this--however we should be using more transactions so this could become a worse problem over time).

@vojtechjelinek vojtechjelinek added this to To do in Data handling Jul 3, 2020
@vojtechjelinek vojtechjelinek moved this from TODO (critical) to TODO (improvement/feature) in Data handling Oct 13, 2020
@seanlip seanlip removed this from TODO (improvement/feature) in Data handling Oct 11, 2022
@seanlip seanlip added enhancement Label to indicate an issue is a feature/improvement Impact: High Blocks or significantly slows down a core workflow. Work: Medium The means to find the solution is clear, but it isn't at good-first-issue level yet. labels Dec 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
architectural refactor Architectural refactors that clean up the codebase backend enhancement Label to indicate an issue is a feature/improvement Impact: High Blocks or significantly slows down a core workflow. needs design doc Work: Medium The means to find the solution is clear, but it isn't at good-first-issue level yet.
Projects
Status: Todo
Development

No branches or pull requests

4 participants