fix ExpiringDict caching solution #1200
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
xlorepdarkhelm
requested changes
Jul 22, 2024
jskinne3
approved these changes
Jul 22, 2024
ccostino
approved these changes
Jul 22, 2024
There was a problem hiding this comment.
Thanks, @terrazoon!
Let's see if this helps alleviate some of the pain at least and buys us more time and space to figure out our next steps with all of this.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
For various reasons we are currently forced to use an in-memory cache to store job information that is used to populate the reports. Although we increased the cache time to 7 days and the number of objects to 20,000, there are still intermittent problems retrieving data being reported.
We found two separate issues:
Sometimes we cannot download a csv file (404) even though we know that csv was successfully uploaded and the message in the csv was successfully sent. This should not happen and the reason why it does is currently a mystery (seems to happen on Fridays!). We are going to ignore this issue for the moment.
Every time the app restarts, the in-memory cache is obviously wiped out, and it seems that the app restarts more frequently with more dire consequences for report generation than we anticipated. In addition to this, no in-memory solution can run across processes so we will need a separate cache for every gunicorn worker.
So the solution proposed here is run a 'repair' task periodically that checks the cache to make sure it has all jobs currently in the bucket.
NOTE: The 'best practice' here would be to use redis or memcached, but until or unless we decide to go that route, ExpiringDict where we rebuild it on a schedule seems like a workable solution. It potentially does require more memory (although how much memory is being used by one instance of the cache is unknown, and probably not much).
Security Considerations
N/A