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 lock timeout handling #99

Open
eaufavor opened this issue Dec 2, 2020 · 1 comment
Open

Cache lock timeout handling #99

eaufavor opened this issue Dec 2, 2020 · 1 comment

Comments

@eaufavor
Copy link
Contributor

eaufavor commented Dec 2, 2020

In the current implementation, a cache lock timeout is treated as an error.

As mentioned in README, cache lock is "to prevent dog-pile effects". It is an optimization. Cache lock timeout itself shouldn't be a cause of error. A common way of handling cache lock timeout, for example, nginx cache lock timeout, is just to perform uncacheable lookup, similar to elapsed cache lock.

Let's talk about it before I start to try to make a change, since it could also be a breaking change.

@eaufavor eaufavor changed the title [Improvement/Bug] Cache lock timeout is treated as error. [Improvement/Bug] Cache lock timeout is treated as an error. Dec 2, 2020
@thibaultcha
Copy link
Owner

thibaultcha commented Dec 18, 2020

Right, and yet that is not the only the purpose the cache lock serves though; quite often when I have seen this lock expire it has been because of the elected callback itself timing out (i.e. DB timeouts, DNS issues, ...). In such cases, it is much more useful to have an option like resurrect_ttl and early-exit all pending lookups rather than risking cascading failures.

When I hear similar concerns as yours on how to tweak the lookup mechanism to your needs, I think about a configurable retry policy mechanism for this library, a step beyond resurrect_ttl, which could be quite elegant and allow for finer pre-built policies such as the one you described.

@thibaultcha thibaultcha changed the title [Improvement/Bug] Cache lock timeout is treated as an error. [enhancement] Cache lock timeout handling Feb 8, 2021
@thibaultcha thibaultcha changed the title [enhancement] Cache lock timeout handling Cache lock timeout handling Aug 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants