varnishcache
cache_range: Bring back VCL control over backend range Requests
The added VTC contains a test case for a (crude) partial caching pattern in VCL, which stopped working with 4ab110047130e3a89936104d74f4729b650676f9 because the Range header to check is taken from bereq0, rather than bereq, which VCL has under control and is also the request sent to the backend.
Ref #3246
You are interfering with http_range_support, your test case passes with this:
varnish v1 -cliok "param.set http_range_support off"
Now regarding http_*_support parameters, it bothers me that they are global, it would be better if they were the default value for some kind of req.*_support variables (yes, a flag-ism) so that you could actually make that decision depending on the nature of the backend or other criteria.
With http_gzip_support, I'd actually do things differently nowadays, but it's off-topic..
Yes, I had also noticed that the test case works with range support disabled.
But I think the test case should also work with it being enabled, and it is, I think, wrong to compare with a header which might not even have been sent to the backend: We are validating the backend response with respect to the backend request, and that is bo->bereq, not bo->bereq0
In that case, what we are missing at fetch time is:
if (cache_param->http_range_support) {
/* reset range header to bereq0 */
}
Regardless of whether we expose a new flag in VCL, I think we should have a [be]req flag for range support to consistently make or avoid range checks over the span of a complete client transaction.
Dridi, I would like to ask you to take a moment to reflect on what you are proposing here. Do you have a secret plan to make VCL useless?
Edit: I agree with the second part to snapshot the parameter for the entirety of a transaction, probably even client+backend combined.
Dridi, I would like to ask you to take a moment to reflect on what you are proposing here.
Ok, so I start with http_range_support that puts Varnish in charge of range requests, and when I'm in charge, I try to do things by the book (or in this case, as mandated by the RFC). So I won't shy away from enforcing strict range checks, in the component that is in charge of range requests.
The problem with such a global flag is that I can only turn it off if somehow my entire Varnish deployment can take care of range requests. If only some parts of the backend interactions should be handled manually, and the rest should be left to automatic range support, then I'm out of luck with this global flag.
In "modern" deployments with one VCL label per "tenant", it becomes even less likely to satisfy range requirements across the board with an all-or-nothing flag. So the moment I need to take action on range requests, it becomes very unlikely that I can just turn off range support globally.
Do you have a secret plan to make VCL useless?
I'm showing you that http_range_support is probably useless as a global parameter in most cases, and suggesting that we instead allow VCL authors to turn it off on a per-req basis, so that both your use case and internal (strict) range support can co-exist.
How is that making VCL useless? Two VDDs ago I laid out a plan to move more of the internal cache-control&co handling in VCL, while at the same time behaving closer to the definition of server-side cache-control directives. I took an active part in #3994 that is also increasing VCL capabilities.
You can't accuse me of that just because you want to take a shortcut to satisfy your use case.
So, to get some things out of the way first:
because you want to take a shortcut to satisfy your use case.
I do not want to take shortcuts, and it seems I have not made clear the reason for my "Do you want to make VCL useless comment". The trigger was specifically this:
if (cache_param->http_range_support) { /* reset range header to bereq0 */ }
Force-undoing changes from VCL is, I think, making VCL useless. This is absolutely what I think we should not do.
My tiny 1-character change in this PR has exactly one purpose: To bring back VCL control. It is not to destroy anyone's use case, it is not to destroy strict RFC behavior and it is not to prevent sensible improvements to VCL.
That said, I appreciate you elaborating on the topic more broadly and in general I feel aligned with you, but I want to emphasize an aspect which is, I believe, vitally important for this project: power of the user through VCL.
Yes, we should do things by the book and we should enforce strict checks. BUT there should always be a way for VCL to override. In another ticket you said "we have vmods for the dangerous stuff" and I would be happy to agree with you (if you could, too?) that there is absolutely a gray area between VCL, vmods and Inline-C as to how many bullets we should allow users to shoot in their feet. BUT there should always be a way for VCL to override, if only for the case where WE make a mistake: in particular when we have to release the next VSV, we will hopefully be able to tell users "and here's the mitigation VCL". The more we depart from the principle that VCL must ultimately be in control, the harder this will become and the less Varnish-Cache will be able to differentiate itself from smart-arse alternatives as a tool which empowers its users.
On http_range_support: I see this as a convenience feature which switches on sensible defaults to have Varnish-Cache handle client range requests. And, if you will, also enables stricter checks. But this feature should not be necessary to handle range requests, it should be possible for VCL to turn Range requests into ordinary requests and explicitly push the range filter on the client side to achieve the same behavior (I did not check and do not have time rn, but this really must work, IMHO).
So it's good that you added stricter checks, but they should not take away VCL control even if the feature is enabled, and I see absolutely no reason why it should.
Regarding the global flag: Yes. I would go so far as to consider allowing VCL to override more parameters (it can already do this for some), and why not override http_range_support?
But, again, this is orthogonal to the point that the feature should still behave in a way which does not conflict with VCL, it should still keep VCL in control.
To close, I do not believe that you actually want to make VCL worse, which is why I asked you to reflect on your suggestion.
Oh and BTW, I might have missed to point out that the issue addressed here is also a regression compared to some version 6.
So, to get some things out of the way first:
because you want to take a shortcut to satisfy your use case.
I do not want to take shortcuts, and it seems I have not made clear the reason for my "Do you want to make VCL useless comment". The trigger was specifically this:
if (cache_param->http_range_support) { /* reset range header to bereq0 */ }Force-undoing changes from VCL is, I think, making VCL useless. This is absolutely what I think we should not do.
I'm getting framing headers vibe here, so I think we can agree to disagree. Just like we shouldn't let VCL fiddle with HTTP framing because the core code is in charge, we shouldn't let VCL fiddle with ranges when we put the core code in charge via the parameter.
My tiny 1-character change in this PR has exactly one purpose: To bring back VCL control. It is not to destroy anyone's use case, it is not to destroy strict RFC behavior and it is not to prevent sensible improvements to VCL.
That said, I appreciate you elaborating on the topic more broadly and in general I feel aligned with you, but I want to emphasize an aspect which is, I believe, vitally important for this project: power of the user through VCL.
This is exactly what I'm advocating for. Making range support a per-req setting to give that kind of power easily and on-demand.
Yes, we should do things by the book and we should enforce strict checks. BUT there should always be a way for VCL to override. In another ticket you said "we have vmods for the dangerous stuff" and I would be happy to agree with you (if you could, too?) that there is absolutely a gray area between VCL, vmods and Inline-C as to how many bullets we should allow users to shoot in their feet. BUT there should always be a way for VCL to override, if only for the case where WE make a mistake: in particular when we have to release the next VSV, we will hopefully be able to tell users "and here's the mitigation VCL". The more we depart from the principle that VCL must ultimately be in control, the harder this will become and the less Varnish-Cache will be able to differentiate itself from smart-arse alternatives as a tool which empowers its users.
And again, my point was to offer this kind of override on a per-req basis instead of the current global override.
On
http_range_support: I see this as a convenience feature which switches on sensible defaults to have Varnish-Cache handle client range requests. And, if you will, also enables stricter checks. But this feature should not be necessary to handle range requests, it should be possible for VCL to turn Range requests into ordinary requests and explicitly push the range filter on the client side to achieve the same behavior (I did not check and do not have time rn, but this really must work, IMHO).
The filter angle is interesting, I need time to ponder this one.
So it's good that you added stricter checks, but they should not take away VCL control even if the feature is enabled, and I see absolutely no reason why it should.
Regarding the global flag: Yes. I would go so far as to consider allowing VCL to override more parameters (it can already do this for some), and why not override
http_range_support?But, again, this is orthogonal to the point that the feature should still behave in a way which does not conflict with VCL, it should still keep VCL in control.
To close, I do not believe that you actually want to make VCL worse, which is why I asked you to reflect on your suggestion.
Oh and BTW, I might have missed to point out that the issue addressed here is also a regression compared to some version 6.
Again, agree to disagree. To me, we are primarily crippled by the global nature of http_range_support.
Hm, OK, I was hoping for a different kind of agreement, but then it is how it is.
So:
-
No, I will not agree to a "force-reset to bereq0". Clear point: This is destroying VCL for no good reason.
-
No, Range processing is not to be compared with framing. I am much more in agreement with your point regarding framing than here. And also with framing, I think we have been restricting VCL power too much.
This PR should go in as is, and vcl override for http_range_support is orthogonal.
On
http_range_support: I see this as a convenience feature which switches on sensible defaults [...] it (is) possible for VCL to [...] push the range filter on the client side to achieve the same behavior
See dd4966e53ea7149095250a2b7471bb139c976438