feat: add cache control
Context
- Site local server
- Cloud proxy
Due to latency issues, there needs to be a way to serve static content from the edge
Proposed Solution
- Add Cache-Control together with Etag
- The proxy needs to independently predict the effectively rendered language in order to serve the right cache key
- Therefore: set a
user_langcookie which records the effectively delivered language, this removes too much ambiguity from headers like Accept-Language, already after the first roundtrip
Usage Nginx
At the beginning of the server (?) / location block:
http {
map $cookie_preferred_language $selected_lang {
default $cookie_user_lang;
}
server {
proxy_cache frappe;
proxy_cache_key $scheme$host$request_uri|$selected_lang;
proxy_cache_valid 200 302 1d;
proxy_cache_valid 404 1m;
proxy_cache_lock on;
add_header X-Cache-Status $upstream_cache_status;
}
}
Considerations
Nginx is really quite limited in terms of header and cookie manipulation in relation to its cache subsystem. Better to use Varnish on the edge. rules & example: tbd
no-docs
@ankush with this machinery in place and validated, we could start to Cache-Control: private,max-age=60 some regular GET / POST resources on the hot path, which should make the backend still quite significantly more snappy.
Some candidates:
/api/method/erpnext.accounts.utils.get_fiscal_year # prior to DOM load for some reason
/api/method/frappe.desk.doctype.event.event.get_events # ok to be 1m stale
/api/method/frappe.desk.desktop.get_workspace_sidebar_items # quasi immutable on the private cache
Just private caching these three would take of 250ms overall (~3 resources loaded in parallel blocking subsequent loads) in our case.
There's also a redundand request to get_fiscal_year with added entropy at all:
Oh and /app/home is just a perfectly cachable minimal envelope DOM clocking in at ~700 ms:
see also: https://github.com/frappe/erpnext/pull/41871
Go for it now. The future is promised to no one. - Wayne Dyer
Kindly help move this PR forward. Many thanks from Yours Sincerely!
Fears are nothing more than a state of mind. - Napoleon Hill
Kindly help move this PR forward. Many thanks from Yours Sincerely!
Patience is the mother of all sciences - Unknown ;)
(thanks for your unvaluable work, @blaggacao )
This wisdome stoically spourts Tstalebot — one week :smile:
The important thing is this: to be able at any moment to sacrifice what we are for what we could become. - Charles Dubois
Kindly help move this PR forward. Many thanks from Yours Sincerely!
An invincible determination can accomplish almost anything and in this lies the great distinction between great men and little men. - Thomas Fuller
Kindly help move this PR forward. Many thanks from Yours Sincerely!
@akhilnarang @surajshetty3416 I'd want to proceed to merge and own this, if you havn't got any objections. Maybe there's someone on the team who should have a look at this, before I do?
Things to maybe review:
-
from_cacheflag still useful? - No inadverted significant (performance?) consequences of adding the user language to the
LoginManagerslots - Relation to the design of after request hooks
Otherwise, we've been running this to great effect in production behind a varnish edge cache and static pages, but also delivery of website_script.js is notably sped up, usually from the edge and without any special proxy config.
Note to self, before moving ahead: Flip the switch on the client side for dev servers according to this comment (last paragraph) so that Cache-Control is made ineffective (yet still present / visible) during development for these requests.
@akhilnarang Since no notification about potential problems with https://github.com/frappe/frappe/pull/26752 have reached me in the last 2 weeks since its merge, I'd proceed cautiously with this one, next.
I'm running this successful in production for ever since this PR was opened, so I'm more than ready to stand by and address any potential issue asap should they come up.
@blaggacao this is micro-optimization at best that doesn't work as well as you think.
- You're adding etag computation overhead on EVERY request. >99% of requests that hit python workers are not cacheable.
- You're suggesting that this helps on frequent requests like
erpnext.accounts.utils.get_fiscal_yearbut does it really? All Etags do is tell the client to use cached response if possible. Client still has to wait for a response... you saved what?{"fiscal_year": "2024-25"}26 bytes of transfer? That's less than the MTU size, it's basically free. The real fix would be fix the godawful code in ERPNExt that needs to make this request frequently in the first place.
Easy fix for framework: This should be opt-in, only functions that explicitly enable caching should have this overhead.
cc @akhilnarang @surajshetty3416