http: dynamic responses for empty cache entries#603
Merged
Conversation
If the cache entry for any given url was cleared, this would be indicated by a null entry for that url, and vere would serve a 404 for those cases, instead of falling back to the normal, dynamic behavior of injecting the request into eyre. Here, we make eyre serve from the cache only if there is a full cache entry for the url, and have it proceed into normal request injection logic otherwise. This way, setting a cached response for some url no longer means giving up dynamic responses on that url for the rest of time. Recent versions of eyre exhibit the same behavior. As such, in practice, this code won't change the observed behavior without a similarly-patched version of eyre.
This was referenced Feb 8, 2024
Fang-
added a commit
to tloncorp/landscape
that referenced
this pull request
Feb 15, 2024
Globs contain big, static blobs of data, that only get updated periodically. Eyre has an affordance for caching responses at specified urls. Here we update docket to, whenever a glob gets added, updated or removed, manage the eyre response cache to match. We call +new-cache in every place where either +new-docket or +new-chad gets called, making sure to pass in the old charge so that we can clear potentially-stale entries from the cache. Note that docket's dynamic request serving behavior would respond to both /path.ext and /path/ext with the glob entry for the (clay-style) path /path/ext. However, we only add a response for the '/path.ext' url into the cache. This is the common case, and doing so avoids adding twice the amount of cache pressure. For the %groups desk, whose initial pageload requests 32 files, this takes initial load time down from ~0.5s to ~0.1s (on localhost, so no "real networking" overhead). About half of that remaining time is spent on the initial page request, which usually dynamically falls back to serving /index.html. About a fifth is spent on loading desk.js and session.js, which are served by docket independent of glob contents. Adding those into the cache could bring additional performance gains. This should be considered soft-blocked on urbit/urbit#6909 and urbit/vere#603. Without those changes, docket may make /apps/* urls permanently unusable for dynamic binding.
pkova
approved these changes
Mar 6, 2024
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
If the cache entry for any given url was cleared, this would be indicated by a null entry for that url, and vere would serve a 404 for those cases, instead of falling back to the normal, dynamic behavior of injecting the request into eyre.
Here, we make eyre serve from the cache only if there is a full cache entry for the url, and have it proceed into normal request injection logic otherwise.
This way, setting a cached response for some url no longer means giving up dynamic responses on that url for the rest of time.
Recent versions of eyre exhibit the same behavior. As such, in practice, this code won't change the observed behavior without a similarly-patched version of eyre. For that, see the sister PR, urbit/urbit#6909.
The bulk of the updated logic in
_http_cache_respond()comes from_http_rec_accept(), but due to differences in cache checking behavior it isn't a 1:1 match, so it seemed simplest for now to just duplicate the logic, rather than introduce another intermediary request handling function.Resolves urbit/urbit#6702.