Icingaweb2: Improve Caching

[nilmerg]

Icinga Web 2 does currently not use a standardized way how to serve cachable content to the client.

This applies to gravatars, module images, CSS and JS.

• Gravatar responses do not honor "Cache-Control: no-cache" nor "Cache-Control: max-age=0" which means once a gravatar is cached we'll serve the cached version forever
• Module image responses have an maximum age of one hour but due to another bug (no issue, atm) client's will ignore this and request images with every single subsequent request
• CSS responses are cached, but the client still issues a request everytime instead of using it's cached version right away (Useful when developing, but in a production env we may save a few milliseconds)
• For JS responses applies the same as for CSS responses

I'd propose to fix and streamline this:

• Content generated by Icinga Web 2 (minimized JS, compiled LESS, Gravatars) must honor the client if he does not like to retrieve cached content ("Cache-Control: no-cache" and "Cache-Control: max-age=0")
• All types of cachable content should be served with a maximum age so that clients do not need to issue the same request with every click by the user

If done correctly, only the HTML served for a given url should be loaded from the server for all subsequent clicks until the maximum age expires.

Note that when using a maximum age for CSS, theme changes made by the client may only instantly visible in the window where the change has been made and only after one hour in other windows/tabs if the user does not request a new version. (Ctrl+F5) If this is not desired, CSS responses must not use maximum age.

@lippserd, @Thomas-Gelf What do you think?

[lippserd]

Hi Johannes,

If JavaScript is enabled, clients do not request JavaScript or CSS after the initial page load. Adding a max age here would definitely hurt us because enabling and disabling modules and changing themes modifies the JavaScript and CSS which has to be served. The only issue I see here is, that our controller for static content does not send the appropriate caching headers.

Cheers,
Eric

[Thomas-Gelf]

@nilmerg, some thoughts:

• gravatar: should become a hook and be shipped as one of the available default implementations or completely moved to a dedicated module
• other implementation for such hook might ship user images from Active Directory or similar
• module images: in case there are issues they should be addressed
• discovered at least one issue in the doc module: the wrong content-type is going to be sent, you can easily compare the current behavior with an attempts to place exit; at the end of ModuleController::imageAction(). Guess something changed at postDispatch time, at least the module makes a wrong assumption here. Should be fixed.
• we should touch with special care how "our" CSS and JS are shipped. As far as I know we use standard conforming ETags and respect Cache-Control
• I strongly advocate AGAINST sending expiration headers. That's what we have ETags for, we explicitely want the client to ask every time for the current version, giving us his ETag
• to improve the latter we need a mechanism that add's our tag to the (virtual) file name, shipping something like icinga.hash.css. Makes layout rendering more expensive, as disk I/O is shifted - but saves two async requests. It's worth to invest some time here, but this should happen combined with the long-standing feature request involving our half-finished support for "automagically refresh CSS and JS for all connected browsers"

[lippserd]

Please also note that the Cache-control is part of the HTTP/1.1 specification and broadly supported by "modern" browsers. I think it is worth a shot to remove the Pragma and Expires header.

[Thomas-Gelf]

Created #2918 for the doc module issue

[nilmerg]

Okay, CSS and JS may indeed change too often/easily so that a max age would hurt us.

But static content hardly changes that often/easily that it is strictly necessary to have clients ask every single page load for the content. I can't see a reason that an expiration date would hurt us here.

[Thomas-Gelf]

But static content hardly changes that often/easily that it is strictly necessary to have clients ask every single page load for the content. I can't see a reason that an expiration date would hurt us here.

Absolutely. If I did not miss anything, this should already be in place:

https://github.com/Icinga/icingaweb2/blob/72b02f7f0892c724498dacfa1c237d3f2cb4dc16/application/controllers/StaticController.php#L89-L90

Gravatar-Handling definitively needs improvement, should re-check the source from time to time etc - we should move this at least to a dedicated class, this is way too much logic for a controller action.