Add caching headers to all responses #101
Conversation
CHANGELOG.md
Outdated
| @@ -10,6 +10,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 | |||
|
|
|||
| ### Added | |||
|
|
|||
| * Adding Cache-Control max-age headers to all http responses set to 1h. | |||
ruflin
force-pushed
the
caching-headers
branch
from
692e907 to
c660b5e
Compare
| func cacheHeaders(w http.ResponseWriter) { | ||
| w.Header().Add("Cache-Control", "max-age="+cacheTime) | ||
| w.Header().Add("Cache-Control", "public") | ||
| } |
There was a problem hiding this comment.
These are a good start and Good Enough to merge, IMO.
However, I think adding a time-based (Last-Modified) or content-based (ETag) header would be a good improvement because it allows the client to reuse a response which is older than cacheTime but which hasn't changed.
There was a problem hiding this comment.
The Last-Modified will be a bit tricky as the data is generated and it is not clear to me what we would set it to. I was thinking about the ETag quite a bit and was not sure if it will be beneficial if we have a very long cache time. We could use the file sha1 as the etag for example.
There was a problem hiding this comment.
I think there are reasonable answers to those questions, but let's avoid them for now and focus on the max-age value.
I don't think the registry should use the same caching policy for all responses.
/package seems well-suited for a "far future" expiration date. e.g. a year or more. The assets associated with a package-x@major.minor.patch should never change. AFAIK, the registry won't allow force push/republish, so any change should be a new version. That means the expiration should be able to be well into the future without any issues.
/search & /categories are dynamic and should have the expirations set to whatever we feel is Fast Enough and/or doesn't overwhelm the registry.
There was a problem hiding this comment.
SGTM and agree on the different caching times. I expect we will have in the future a CDN in front so the total number of request on the service itself should become pretty low.
The content of the registry only changes if a new package is added or if the registry is updated. Because of this, all responses can be cached. Caching headers are introduced for all requests and the caching time is currently set to 1h. This could be increased in the future especially for the `catchAll` endpoint as these assets basically never change.
ruflin
force-pushed
the
caching-headers
branch
from
c660b5e to
a857ab3
Compare
|
@jfsiii Going to merge this for now but would like to continue the discussion on the Cache headers. Probably depends quite a bit also on how the Kibana Http Client handles cache headers. |
|
@ruflin I'll follow up but the Registry sets the tone here. There's no special/manual intervention required for clients. It's baked into the protocol, browsers, down the line. They can override if need be but it mostly Just Works (based on however the server responds). |
The content of the registry only changes if a new package is added or if the registry is updated. Because of this, all responses can be cached. Caching headers are introduced for all requests and the caching time is currently set to 1h. This could be increased in the future especially for the
catchAllendpoint as these assets basically never change.