Is the documentation in the BigPipe README correct?
It advises:
vcl_fetch {
if (beresp.Surrogate-Control ~ "BigPipe/1.0") {
set beresp.do_stream = true;
set beresp.ttl = 0;
}
}
I am uncertain under what circumstances BigPipe might switch from using BigPipe/1.0
to BigPipe/1.1
or BigPipe/2.0
. It is also unclear whether the 1.0 here is the exact version of the BigPipe module, or, what seems more likely, it might be a form of protocol version that changes less frequently. That is, might BigPipe version 1.3 continue to use "BigPipe/1.0", or would it report "BigPipe/1.3"? If BigPipe is adopted in Drupal 8 core, would it report "BigPipe/1.0", or "BigPipe/8.0"?
Clearly, it is possible that future versions might require different varnish settings; also, other clients might in the future use the Surrogate-Control for some purpose other than Varnish cache control, which might require updating the reported version number for other unrelated purposes. I don't really know; this is a question.
If the reported Surrogate-Control value does get bumped up to BigPipe/1.1, would it be better if clients that are not yet savvy to the change stop using the modified varnish settings (which definitely breaks), or continue using the old settings (might break)? If the former, it might be better to recommend testing for "BigPipe/1" or "BigPipe/", as a contingency. Individual users who build their own servers can be expected to update their configuration before they upgrade their BigPipe implementation, but service providers that set up varnish for clients will want to use a strategy that is best suited to long-term stability. Also, even for individual users, it would be best to minimize the need to update varnish configuration.
It would be great if the README could be updated to explain what the plan / policy for changing the Surrogate-Control value is, and what the best possible configuration tests are.
Comment | File | Size | Author |
---|---|---|---|
#16 | update-readme-2644146.patch | 399 bytes | kostajh |
#4 | 2644146-4.patch | 1.06 KB | Wim Leers |
Comments
Comment #2
Wim LeersThese are excellent questions! Let me think about that some time. Perhaps @Fabianx also has thoughts on this.
Comment #3
Fabianx CreditAttribution: Fabianx at Tag1 Consulting commentedI don't think there will ever be a 1.1 atm. ESI is also still at ESI/1.0 since ages.
So I would say:
BigPipe/1.0 just means BigPipe as streaming technology, not as advertised version.
Thoughts?
Comment #4
Wim LeersI think I agree with @Fabianx. Even more so because we do not expect anything special of proxies: the only thing we expect is that they don't perform buffering. That's it. We can change implementation details of BigPipe, but the key requirement of BigPipe is just that: no buffering.
Consequently, I also can't see any need to update this version number in the future.
Finally, note that ESI has far more requirements for proxies claiming to support it. They haven't had to (or simply chose not to) increase their version number.
Patch attached that adds this information to the README.
Comment #5
Fabianx CreditAttribution: Fabianx at Tag1 Consulting commentedSounds perfect!
Comment #6
Wim LeersPer #5, committed & pushed #4.
Comment #7
Wim LeersAdjusting issue credit.
Comment #9
Anonymous (not verified) CreditAttribution: Anonymous commentedfyi,
for current Varnish v4, vcl_fetch() has been replaced by vcl_backend_response(), cref: https://www.varnish-cache.org/docs/4.0/whats-new/upgrading.html#vcl-fetc...
and,
throws a VCL compile error
should be, instead,
Comment #10
Wim LeersThanks, will fix.
Comment #11
Wim LeersCreated a separate issue + patch for that: #2652692: BigPipe's Varnish documentation does not apply to Varnish 4. Please review.
Comment #12
Wim LeersGiving @bugsonly issue credit here because his comment is not on #2652692: BigPipe's Varnish documentation does not apply to Varnish 4, which means I can't give him issue credit there.
Thanks again!
Comment #13
Wim LeersWhich means… congrats with your first credit on your d.o profile: https://www.drupal.org/u/bugsonly :)
Comment #15
x610 CreditAttribution: x610 at FFW commented@Wim
We were trying to apply VCL changes on acquia, and found out, that for Varnish 3 `beresp.Surrogate-Control` is not correct, instead it should be `beresp.http.Surrogate-Control`.
README.md still says `beresp.Surrogate-Control`.
Comment #16
kostajh CreditAttribution: kostajh at Savas Labs commented@x610 is right. Here's a patch.
Comment #17
Fabianx CreditAttribution: Fabianx at Tag1 Consulting commentedIndeed.
Comment #19
Wim LeersHeh, https://www.drupal.org/documentation/modules/big_pipe/environment is already correct.
I think it may be better to just remove this from the README and point to the online docs.
Comment #20
kostajh CreditAttribution: kostajh at Savas Labs commentedUp to ya'll. I personally find README docs more reliable/trustworthy than the d.o. documentation pages, but so long as it's clear where to look, then removing from the README is fine.
Comment #22
Wim LeersHah! You're not alone!
However, since this module now also lives in Drupal core, it just makes more sense to let that be the sole point of documentation.
Did exactly that.