-
Notifications
You must be signed in to change notification settings - Fork 171
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
Increase private cache duration for live requests. #2066
Comments
Note, please wait for @KyleAMathews to comment and approve/revise/reject the approach before implementing. |
I'm not sure this is correct, if you load a page, it does it initial load, then 20 live polling requests, each of those would be coached for 7 days. Unless I'm missing something, and assuming basic |
Hmm, yes. So there is a downside of long duration caching of live requests: it results in iterating through lots of requests from the file cache often without data. This will slow down fresh online data loading in exchange for support for faster cached data loading and better offline support. Interesting to think about the number of live requests for e.g.: 5, 10, 20, 30, 60 minutes. It's sub millisecond to load a small request from disk cache. Say 10 mins * 3 polls per minute = 30 requests. I wonder if we could work around this in the client. E.g.: conceivably keeping a map of offset -> next offset around to skip all the empty live requests. However, this does feel unnecessary when compared with the "proper" solution of persisted shape data and offset which does the same thing much better. So perhaps in the general case I have to admit defeat here. But maybe we can still support 5 - 10 mins of cache to make active browsing sessions naturally resilient, without too much downside on the cached request trawling? |
We could make live requests stay cached as long as responses that aren't offset=-1 — and make this cache TTL configurable by the app author. |
This is another specific suggestion deriving from the discussion in #2050.
Right now, live requests are served with:
This means they are only cached for 5 seconds. This time is designed to trigger caching to enable request collapsing, whilst also being less than the long poll timeout of 20 seconds. So that repeated long-polling requests don't get stuck receiving stale content from the cache.
However, after landing on this, @KyleAMathews also discovered and worked around some inconsistent caching behaviour between CDNs by adding a
cursor
header and param to requests. This means that clients don't get stuck making the same request: every live request has an increasing cursor, until the offset bumps and the cursor resets.So two things:
When new clients make live requests for an offset starting with the base cursor, we don't want them to have to loop through umpteen cached requests to get one with content. That's why it's useful that the shared cache for live requests is relatively low.
However, when existing clients have received a response from a live request, there's no problem with caching it for longer. They're not going to go back, they're going to move on to a new offset and they've already loaded the data. So we can increase the private max-age to much longer. In tandem with #2064 this will enable offline support out of the box, without compromising anything else.
If this is correct then I propose that we update the header to something like:
The text was updated successfully, but these errors were encountered: