-
Notifications
You must be signed in to change notification settings - Fork 15
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
Fetching stale values can be slow #33
Comments
It seems like if the computation is expensive enough to cause the event loop to stall for a non-trivial amount of time, that would cause problems anyhow. "Best case" once the app is in production and exposed to a certain level of concurrency..? |
Yeah, so for my case each refresh is just a single network call, but each call to the server fetches a bunch of values from the cache. (Between 20-200.) I'm using the memory backend, so it's not a huge deal, but the requests seemed to be clobbering me. I profiled my code, and it spends most of its time in the node innards. I think issue really is that a value has to bubble its way past a bunch of var i = 0;
var ptimes = new Array(5000);
function ptest() { ptimes[i] = process.hrtime(); Promise.resolve(i).then((ii) => {ptimes[ii] = process.hrtime(ptimes[ii])[1]/1000000;})}
// u = lodash
for ( i = 0; i < 50; i++) ptest()
u.mean(ptimes)
// 0.384245
for ( i = 0; i < 500; i++) ptest();
u.mean(ptimes)
// 1.9938009480000003 So the more promises are scheduled at the same time, the longer they take, since they just stack on top of the event loop. Since I don't expect you guys to fix this, since it obviously isn't your use case. Anyway it might not be possible without rewriting most of the cache, now that I know what the problem is. At this point I just hope you found this interesting. 😄 You're totally right though, it does work like a dream when it's under serious load. Now all I need is the users. |
When a stale value is fetched, because of the way promise code is scheduled (
Promise.then
just sticks the function on the even loop), theval()
function passed togetOrElse()
is executed before returning the stale value. Ifval()
is computationally expensive, or if many stale values are queried a the same time, it can significantly delay the response.Just stupidly delaying the content refresh solves this problem (as long as you don't make another request 500ms later):
This is obviously not the correct way to do it, but it does demonstrate that this is where the problem is.
Having some kind of scheduler that delays the content refresh logic until all the cache calls have resolved seems like the way to go. (Naturally it would have to avoid starvation.) Let me know what you think, I'd be happy to look into it.
The text was updated successfully, but these errors were encountered: