Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Cache] Prevent stampede at warmup using apcu_entry() for locking#27028
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Closed
Uh oh!
There was an error while loading.Please reload this page.
Conversation
This file contains hidden or 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
0225318 toec2b4c0Compareec2b4c0 to4885cfeCompareContributor
webnet-fr commentedApr 30, 2018
|
MemberAuthor
nicolas-grekas commentedApr 30, 2018 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@webnet-fr that'd be great if it could work, but I don't think that's possible: this code is full of race conditions, and the |
c60f631 to1789e42Comparefabpot added a commit that referenced this pull requestJun 11, 2018
…lculations (nicolas-grekas)This PR was merged into the 4.2-dev branch.Discussion----------[Cache] Use sub-second accuracy for internal expiry calculations| Q | A| ------------- | ---| Branch? | master| Bug fix? | no| New feature? | not really| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets | -| License | MIT| Doc PR | -Embeds#26929,#27009 and#27028, let's focus on the 4th commit for now.This is my last significant PR in the Cache series :)By using integer expiries internally, our current implementations are sensitive to abrupt transitions when time() goes to next second: `$s = time(); sleep(1); echo time() - $s;` *can* display 2 from time to time.This means that we do expire items earlier than required by the expiration settings on items.This also means that there is no way to have a sub-second expiry. For remote backends, that's fine, but for ArrayAdapter, that's a limitation we can remove.This PR replaces calls to `time()` by `microtime(true)`, providing more accurate timing measurements internally.Commits-------08554ea [Cache] Use sub-second accuracy for internal expiry calculations
e31510c to6ed5db7Compare6ed5db7 to54543aeCompareMemberAuthor
nicolas-grekas commentedJun 14, 2018
I'm closing because I'm not convinced we can plug APCu at this central place. |
fabpot added a commit that referenced this pull requestJun 18, 2018
…las-grekas)This PR was merged into the 4.2-dev branch.Discussion----------[Cache] Prevent stampede at warmup using flock()| Q | A| ------------- | ---| Branch? | master| Bug fix? | no| New feature? | yes| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets | -| License | MIT| Doc PR | -Replaces#27028This PR protects against cache stampede by wrapping the computation of items in a pool of locks.For each apps, there can be at most 20 concurrent processes that compute items at the same time and only one per cache-key.Commits-------0ac2777 [Cache] Prevent stampede at warmup using flock()
Sign up for freeto 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.
Uh oh!
There was an error while loading.Please reload this page.
When a cache is cold, all concurrent processes will end up recomputing the same value, which can be slow at best and can create a cascading failure at worst.
By leveraging
apcu_entry(), we can ensure only one process per front is actually computing the value.In theory, you might wonder if this could be implemented in a generic way using e.g. the Lock component. This would allow using distributed locking, thus allow locking the computation across a cluster of fronts.
But in practice,
apcu_entry()is the only primitive that works, because it knows when tonot compute the callback when it waited for a concurrent process, which is something really hard to achieve with other locking mechanisms (if possible at all without all sort of race conditions.) Also, computing only once per-front is already a significant improvement. People having more advanced needs could still create a proxy to handle the distributed lock.For core, relying on
apcu_entry()to provide this behavior is what I propose here.