Skip to content

Improve caching of assets (project/group avatars/note attachments)

Tim Zallmann requested to merge tz-cache-project-avatars into master

What does this MR do?

This improves the caching of all avatars and note attachments so that further request don't hit our backend anymore. As currently the image caching is the biggest offender in loading and rendering performance.

Removes the pragma:no-cache header by setting it to empty, if we would use delete then it would be added later in the pipeline.

There is also a follow-up issue on improving the loading of user avatars (and most probably also others) in general #33777 (closed).

Important

All these changes are dependent on #32761 (closed) to be effective on .com. This would then enable also in a follow-up finally dynamic image resizing on .com through our CDN.

User Avatars

.com at the moment cache-control: private, max-age=0 (No Caching at all, each public avatar load hits our backend.)

Rails after this change Cache-Control: max-age=300, public (Caching for 5 minutes and also in public proxies etc.)

Competitor Cache-Control: max-age=300

Project + Group Avatars

.com at the moment cache-control: private, max-age=0 (No Caching at all, each private avatar load hits our backend.)

Rails after this change Cache-Control: max-age=300, private, must-revalidate (Caching for 5 minutes)

Notes attachments

.com at the moment cache-control: private, max-age=0 (No Caching at all, each notes image load hits our backend.)

Rails after this change Cache-Control: max-age=15778476, private, must-revalidate (6 months caching as every upload gets anyhow a new address)

Competitor (for private issue upload, no access control at all as the URL is unique with GUID) Cache-Control: max-age=2592000

Screenshots

Before (every image request hits always our backend and performs multiple DB + Redis calls just to deliver each image or avatar)

Bildschirmfoto_2019-10-09_um_18.07.49

After (due to the caching headers the server is not even hit as all assets are only revalidated based on their headers)

Bildschirmfoto_2019-10-09_um_18.08.13

Does this MR meet the acceptance criteria?

Conformity

Performance and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • Label as security and @ mention @gitlab-com/gl-security/appsec
  • The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • Security reports checked/validated by a reviewer from the AppSec team
Edited by Tim Zallmann

Merge request reports