Skip to content
This repository has been archived by the owner on May 10, 2019. It is now read-only.

need cache headers on all static resources #620

Closed
lloyd opened this issue Nov 19, 2011 · 17 comments
Closed

need cache headers on all static resources #620

lloyd opened this issue Nov 19, 2011 · 17 comments

Comments

@lloyd
Copy link
Contributor

lloyd commented Nov 19, 2011

We need to figure out GH-226, and once done, we need to ensure cache headers are on all static resources. We should also add an in-tree unit test to verify we don't regress here.

/cc @vmunix @shane-tomlinson

@ghost ghost assigned lloyd Nov 19, 2011
@lloyd
Copy link
Contributor Author

lloyd commented Jan 25, 2012

I've done some research. Here's my conclusions. We have three types of URLs that should be treated differently:

  1. api requests - should be Cache-Control: no-store
  2. static files - should be Cache-Control: public, max-age=0 and should have a proper ETag
  3. rendered views - should be Cache-Control: public, max-age=0 and should have a proper ETag. should include a Vary header, and etag should change based on dynamic content - specifically locale (this can either be done with a checksum over the content (expensive) or baking the locale into the ETag (cheap))

Now for cache busting! we should always allow the client to specify a SHA somewhere in the request path, that will map to the original file. When this is done, we have perma-urls and we can put the max-age at a month or more. So it is up to the client to permute file paths with a version number wherever it can to improve the cacheability of resources. Ideally we'd have one 304 and a bunch of cache hits requiring no additional network traffic on each page load from a browser with a populated cache.

@lloyd
Copy link
Contributor Author

lloyd commented Jan 25, 2012

lloyd added a commit that referenced this issue Jan 25, 2012
… accept-locale, and *only* with an ETag but not last-modified (the two don't mix well, see rfc2616. issue #620
@Seldaek
Copy link

Seldaek commented Jan 25, 2012

  1. I'd put private, no-store and an additional Expires: 2000-00-00 00:00:00 or such for old HTTP/1.0 proxies, to be sure they are not cached anywhere.
  2. Please don't use ETag, it doesn't play well with multiple-servers setups (i.e. scales bad) unless you generate them yourself then it's ok, but it still requires everyone to revalidate, so you don't eliminate http requests altogether. Besides if you do max-age=0, they are basically not cacheable AFAIK. The best for static files is to use uniquely versioned filenames and have public, max-age=31536000 to cache for a long time, then to do cache busting you rename the files. Alternatively you can add a ?v=X to it, but that's supposedly not as reliable. Although I have never seen any cache fail to notice that as a change, maybe some very old ones out there don't.
  3. Same as above, unique resources (i.e. put the locale in there) should have unique URLs as much as possible, then you can cache them for a few minutes/hours at least with s-maxage=... that will apply to Varnish and other proxies, allowing you to scale well in CPU (you're not saving bandwidth though) while retaining an option to bust the cache on your side.

Disclaimer: I'm jumping in just looking at the proposed headers, but I'm not 100% sure what content you're serving and to what purposes, so this may be slightly off.

@lloyd
Copy link
Contributor Author

lloyd commented Jan 25, 2012

@Seldaek thanks a ton for jumping in.

love the with uniquely versioned file names approach, that's the way we want to go. Hadn't thought about the expires header in the past for old proxies, I'll add that.

w.r.t ETags - we generate them and we do so by combining mtime, locale, and file size, so they should be consistent across our production servers.

@Seldaek
Copy link

Seldaek commented Jan 25, 2012

If you do uniquely versioned assets, for static files I would not use etags but rather far-future expires/max-age. For pages this is not practical since you can't really change the links all the time, so using ETag to save a bit on bandwidth might be good.

@vmunix
Copy link

vmunix commented Jan 30, 2012

@fetep can you confirm our deploy scripts will respect/ensure a consistent mtime across servers?

@lloyd
Copy link
Contributor Author

lloyd commented Jan 30, 2012

Confirmed: http://irclog.gr/#show/irc.mozilla.org/identity/48316

--lloyd

On Jan 29, 2012, at 7:20 PM, Mark Mayoreply@reply.github.com wrote:

@fetep can you confirm our deploy scripts will respect/ensure a consistent mtime across servers?


Reply to this email directly or view it on GitHub:
#620 (comment)

@ghost ghost assigned ozten Feb 2, 2012
@ozten
Copy link
Contributor

ozten commented Feb 8, 2012

@ozten
Copy link
Contributor

ozten commented Feb 9, 2012

Re-organized CSS and script tags in dialog_layout.ejs to give a feel for how this could work.
http://git.io/s8NPoA

@ozten
Copy link
Contributor

ozten commented Feb 10, 2012

Issue#226 raises the issue of cache busting image urls in CSS.

The solution for this bug should be compatible with the connect compiler, so a Less based solution or some other solution can build on this bug.

@ozten
Copy link
Contributor

ozten commented Feb 14, 2012

First pull request to fix this issue:
#1114

@lloyd
Copy link
Contributor Author

lloyd commented Feb 16, 2012

Remaining work:

  1. for js/css resources, don't sent ETag or Last-Modified (should fix upstream)
  2. all image resources should be perma urls.

@shane-tomlinson - thoughts on how we get to #2?

@ozten
Copy link
Contributor

ozten commented Feb 16, 2012

  1. for js/css resources, don't sent ETag or Last-Modified (should fix upstream)

Filed Issue 1141.

@ozten
Copy link
Contributor

ozten commented Feb 17, 2012

@shane-tomlinson If you want, I'll grab the images part of this bug.

lloyd has proposed https://gist.github.com/1849619 and/or Spriting our images.

I'll cook up a PR with LESS and we can pick a path. I'm to lazy to sprite and we can always do it later in it's own issue.

@ozten
Copy link
Contributor

ozten commented Mar 27, 2012

Proposed fix
#1341

@ozten
Copy link
Contributor

ozten commented Mar 28, 2012

Go team!

@ozten ozten closed this as completed Mar 28, 2012
floatingatoll added a commit to floatingatoll/browserid that referenced this issue Apr 2, 2012
@jbonacci
Copy link
Contributor

jbonacci commented Apr 7, 2012

Verified older changes in code for Stage.
Verified latest changes in code for Dev.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants