Skip to content
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

Use Surrogate-Control across all backends instead of custom X-Reverse-Proxy-TTL #455

Open
andrerom opened this issue Jun 5, 2019 · 10 comments

Comments

@andrerom
Copy link
Contributor

andrerom commented Jun 5, 2019

As discussed in the original Fastly PR in order to generalize TTL handling across backends like Varnish, Fastly, Symfony, it could be an idea to:

  1. abstract it on client side?
  2. to be able to emit Surrogate-Control for fastly
  3. If feasible look into also emit Surrogate-Control on Varnish instead of X-Reverse-Proxy-TTL, VCL could still handle for X-Reverse-Proxy-TTL for BC.

https://www.w3.org/TR/edge-arch/

@dbu
Copy link
Contributor

dbu commented Jun 12, 2019

i assume its more complicated than allowing to change the header name, as surrogate-control will need some additional thing to tell that the number is the ttl on that cache? there seems to be discussions on how varnish itself should support the surrogate-control header: varnishcache/varnish-cache#2893

making the thing more flexible definitely sounds like a good idea. for varnish, we might want to wait for them to decide how to handle it, to avoid having our custom thing that is different from how varnish works. (there is also stuff about handling surrogate names so that the application could target only some of the surrogates and whatnot. not necessarily something we have to support right away, but something to keep in mind with the solution that we come up with.)

i guess ttl should move out of the cache control listener, or be injected as a strategy so we can have the current behaviour or the fastly one or maybe a future thing for varnish with surrogate-control.

@andrerom
Copy link
Contributor Author

andrerom commented Jun 13, 2019

how varnish itself should support the surrogate-control header: varnishcache/varnish-cache#2893

After having read that, I guess it's indeed best to drop the 3 item for now.

The main needs are 1. is to be able to not use s-maxage for ttl across our supported proxies, to leave that for normal internet proxies if people want, and 2. an abstract way to do it so there is one way to set TTL across all supported proxies, in case of Fastly emit Surrogate-Control header.

Adds up on your side? :)

@dbu
Copy link
Contributor

dbu commented Jun 13, 2019 via email

@dbu
Copy link
Contributor

dbu commented May 25, 2021

after discussion at symfony/symfony#41369 , an additional thing to think about is that the Age and Date headers prevent max-age and s-maxage to work as (likely) intended when setting a custom TTL. does the surrogate-control header define anything about that?

@andrerom
Copy link
Contributor Author

andrerom commented May 25, 2021

Depends on what you mean, but I can't see any mention of Age here:
https://www.w3.org/TR/edge-arch/#:~:text=2.2%20Surrogate-Control%20Header

But indeed seems this is a known issue when letting reverse proxy cache for long time:
https://book.varnish-software.com/3.0/HTTP.html#age

Ideally browsers should have some heuristics to prefer other headers then Age if set, but possible fix for this is unsetting Age in VCL, or rename it so you can still see age of cache for debugging issues.

EDIT: Or other client headers should be used here, ref question if stale headers might work better on the symfony PR.

@alexander-schranz
Copy link
Contributor

alexander-schranz commented Aug 11, 2022

Find this very interesting Topic! Do we know if Surrogate-Control is used in other cache service like CloudFront, Cloudflare, ...?

I found some more resources about this topic about another header <targeted>-cache-control header:

Requested the state of the CDN-Cache-Control via Twitter here: https://twitter.com/alex_s_/status/1557679407531171843 some interesting links from fasly yet. They currently supporting Surrogate-Control but not yet the CDN-Cache-Control.

@dbu
Copy link
Contributor

dbu commented Aug 12, 2022

oh, that twitter discussion looks quite interesting. if we add flexibility in this regard, adding Surrogate-Control support additionally seems not wrong and probably not too complicated. but the -cache-control draft looks promising. your tweet only got responses from fastly until now... would indeed be interesting to know if the draft can make it into a proper RFC... implementing something that might still change when the RFC is formalized is a bit unfortunate, but with the necessary disclaimers i think it should be ok to do it.

i wonder if the symfony httpcache itself should consider itself a CDN and implement this as well? possibly also registering SymfonyCDN-Cache-Control? (but that discussion is out of scope here)

@alexander-schranz
Copy link
Contributor

Cloudflare did response with support for the CDN-Cache-Control did got lost in my notifications: https://twitter.com/jgrahamc/status/1558142754034794501

The rfc called targeted-cache-control seems also be published two month ago as RFC9213: https://datatracker.ietf.org/doc/rfc9213/

Cloudflare docs: https://blog.cloudflare.com/cdn-cache-control/

@dbu
Copy link
Contributor

dbu commented Aug 15, 2022

ah, that is excellent news! lets open a new issue to implement RFC-9213 and deprecate x-reverse-proxy-ttl then! we will need to make the header name configurable because it could be a vendor-specific name - when we do that, we can simply allow to specify "Surrogate-Control" as well to cover this story. (though i read the twitter thread as saying that fastly will eventually support the new RFC 9213 as well)

are you interested in working on that?

@alexander-schranz
Copy link
Contributor

I opened an issue in the Symfony Core: symfony/symfony#47288 I think it would be great if the HttpFoundation component allow us easier set and manipulate this headers.

@dbu dbu added this to the 3.0 milestone Mar 23, 2024
@dbu dbu removed this from the 3.0 milestone May 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants