-
-
Notifications
You must be signed in to change notification settings - Fork 429
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
Add error threshold - better explanation #314
base: master
Are you sure you want to change the base?
Conversation
@niwinz i can move this to an issue, and maybe theres a better solution, but i feel this warrants discussion as it was a serious issue for us and this was our workaround |
@niwinz @jdufresne @jezdez it's been a while since I opened this, have you had a chance to look at this or the other open prs. If maintaining this is no longer a priority, would you be interested in sharing maintenance responsibilities with Jazzband? |
|
||
DJANGO_REDIS_IGNORE_EXCEPTIONS = getattr(settings, "DJANGO_REDIS_IGNORE_EXCEPTIONS", False) | ||
DJANGO_REDIS_LOG_IGNORED_EXCEPTIONS = getattr(settings, "DJANGO_REDIS_LOG_IGNORED_EXCEPTIONS", False) | ||
DJANGO_REDIS_LOGGER = getattr(settings, "DJANGO_REDIS_LOGGER", False) | ||
DJANGO_REDIS_SCAN_ITERSIZE = getattr(settings, "DJANGO_REDIS_SCAN_ITERSIZE", 10) | ||
DJANGO_REDIS_EXCEPTION_THRESHOLD = getattr(settings, "DJANGO_REDIS_EXCEPTION_THRESHOLD", None) | ||
DJANGO_REDIS_EXCEPTION_THRESHOLD_TIME_WINDOW = getattr(settings, "DJANGO_REDIS_EXCEPTION_TIME_WINDOW", 1) | ||
DJANGO_REDIS_EXCEPTION_THRESHOLD_COOLDOWN = getattr(settings, "DJANGO_REDIS_EXCEPTION_THRESHOLD_COOLDOWN", 5) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An alternative way might be a "maximum total exception timeout" and a "cooldown time" where you can set 2 times, say '15 seconds', '5 minutes' (respectively) and if the total time spent on timeouts on back to back requests without a success is 15 seconds, silence exceptions for 5 minutes
Please, add documentation and if possible tests for that part and i will merge it and release a new version ASAP. |
I know you closed the previous request, but I feel like I did not explain the reason I had to implement this which hopefully justifies the complexity. In my case the lack of this feature caused a production level outage.
When you have multiple cache calls in a request, both timeouts and ignore exceptions are set and redis becomes unavailable, the request hangs until all the timeouts resolve. Through multiple requests, each with multiple cache hits (such as someone spamming refresh) this can ended up hanging the whole server.
I want ignore_exceptions to allow the cache to function as if there was no data returned on error, but still allow for occasional network stutters by having a timeout set to nonzero.
The thresholding I suggest allows you to fail fast when you have a redis outtage and prevent hanging.
If you have suggestions to reduce complexity and solve the issue I'd find that useful. I have been using my fork in prod since October.
P.S. Thanks for all the work on this, its really a great lib and great documentation.