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

Enable immutable caching for S3 objects #9722

Merged
merged 1 commit into from Jan 5, 2019

Conversation

Projects
None yet
3 participants
@nolanlawson
Copy link
Collaborator

nolanlawson commented Jan 5, 2019

Fixes #9721

I also added "public" here, as I can't think of a good reason not to add it. Perhaps it has some marginal benefit in that ISPs (or other proxies) can cache it for all users. The assets are certainly publicly available and the same for all users.

Enable immutable caching for S3 objects
I also added "public" here, as I can't think of a good reason not to add it. Perhaps it has some marginal benefit in that ISPs (or other proxies) can cache it for all users. The assets are certainly publicly available and the same for all users.

@nolanlawson nolanlawson requested a review from nightpool Jan 5, 2019

@nightpool

This comment has been minimized.

Copy link
Collaborator

nightpool commented Jan 5, 2019

@nolanlawson

This comment has been minimized.

Copy link
Collaborator Author

nolanlawson commented Jan 5, 2019

Well they already effectively are, since if you know the URL then you can request it.

The main point of Cache-Control: public is that it tells proxies they are allowed to cache the URL for all users. So e.g. if my router decides to do this, and my wife and I are both using the same router, and I request resource X, and then she requests resource X, the router may decide to serve her the cached version. https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Cache-Control#Cacheability

Otherwise it makes no difference to the browser's cache; the browser can cache regardless of whether it's public or private.

@Gargron

Gargron approved these changes Jan 5, 2019

@nolanlawson

This comment has been minimized.

Copy link
Collaborator Author

nolanlawson commented Jan 5, 2019

Here is another good explanation of cache-control: public: https://stackoverflow.com/a/3492459

@nightpool

This comment has been minimized.

Copy link
Collaborator

nightpool commented Jan 5, 2019

@nolanlawson

This comment has been minimized.

Copy link
Collaborator Author

nolanlawson commented Jan 5, 2019

Yes but it's not cacheable by proxies, since the proxy doesn't know if users A and B see the exact same thing when requesting resource X.

@nightpool

This comment has been minimized.

Copy link
Collaborator

nightpool commented Jan 5, 2019

right, but it's already a cacheable request and intermediaries are free to do whatever, because it has a max-age directive. public is just duplicating that. anyway, whatever, it's not a huge deal

@nightpool nightpool merged commit f05eb67 into master Jan 5, 2019

11 checks passed

ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: check-i18n Your tests passed on CircleCI!
Details
ci/circleci: install Your tests passed on CircleCI!
Details
ci/circleci: install-ruby2.4 Your tests passed on CircleCI!
Details
ci/circleci: install-ruby2.5 Your tests passed on CircleCI!
Details
ci/circleci: install-ruby2.6 Your tests passed on CircleCI!
Details
ci/circleci: test-ruby2.4 Your tests passed on CircleCI!
Details
ci/circleci: test-ruby2.5 Your tests passed on CircleCI!
Details
ci/circleci: test-ruby2.6 Your tests passed on CircleCI!
Details
ci/circleci: test-webui Your tests passed on CircleCI!
Details
codeclimate All good!
Details

@nightpool nightpool deleted the cc-immutable branch Jan 5, 2019

nolanlawson added a commit to nolanlawson/mastodon that referenced this pull request Jan 5, 2019

Enable immutable caching for S3 objects (tootsuite#9722)
I also added "public" here, as I can't think of a good reason not to add it. Perhaps it has some marginal benefit in that ISPs (or other proxies) can cache it for all users. The assets are certainly publicly available and the same for all users.
@nolanlawson

This comment has been minimized.

Copy link
Collaborator Author

nolanlawson commented Jan 5, 2019

It just occurred to me that this may be a moot point for HTTPS anyway. I can't seem to find any info on how proxies handle cache-control: public for HTTPS requests.

Then again, some admins may be using S3 with HTTP rather than HTTPS. And in either case I don't think there's any harm with adding public, yeah.

@nolanlawson

This comment has been minimized.

Copy link
Collaborator Author

nolanlawson commented Jan 5, 2019

BTW for the benefit of Mastodon admins reading this issue who may want to add this header to existing S3 assets, you can use this s3cmd command:

s3cmd --recursive modify --add-header="Cache-Control:public, max-age=315576000, immutable" s3://my-bucket-name/

Note that you will want the latest version of s3cmd from GitHub because apparently there are connection timeout bugs in the Debian version:

git clone https://github.com/s3tools/s3cmd.git
sudo apt install python-dateutil
./s3cmd/s3cmd --help

nolanlawson added a commit to nolanlawson/mastodon that referenced this pull request Jan 11, 2019

Enable immutable caching for S3 objects (tootsuite#9722)
I also added "public" here, as I can't think of a good reason not to add it. Perhaps it has some marginal benefit in that ISPs (or other proxies) can cache it for all users. The assets are certainly publicly available and the same for all users.

nolanlawson added a commit to tootcafe/mastodon that referenced this pull request Jan 11, 2019

Enable immutable caching for S3 objects (tootsuite#9722)
I also added "public" here, as I can't think of a good reason not to add it. Perhaps it has some marginal benefit in that ISPs (or other proxies) can cache it for all users. The assets are certainly publicly available and the same for all users.

nolanlawson added a commit to nolanlawson/cybrespace-meta that referenced this pull request Jan 11, 2019

Tweak S3 command and nginx config
Thanks for merging my last PR! I found a couple more things that folks should do when migrating to S3. This adds the proper caching header, as defined in the latest version of Mastodon: tootsuite/mastodon#9722 . It also ensures that non-custom emoji are properly served with immutable caching via nginx.

It's no fun going back and recursively adding the right cache headers to every object in S3; hopefully this will save some folks that extra step. 😅

chr-1x added a commit to cybrespace/cybrespace-meta that referenced this pull request Jan 13, 2019

Tweak S3 command and nginx config (#3)
Thanks for merging my last PR! I found a couple more things that folks should do when migrating to S3. This adds the proper caching header, as defined in the latest version of Mastodon: tootsuite/mastodon#9722 . It also ensures that non-custom emoji are properly served with immutable caching via nginx.

It's no fun going back and recursively adding the right cache headers to every object in S3; hopefully this will save some folks that extra step. 😅
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment