[master] 98dc20f46 Conditional fetch wait for streaming stale object
Dridi Boukelmoune
dridi.boukelmoune at gmail.com
Fri Dec 20 11:21:07 UTC 2019
commit 98dc20f465cb38103a2c78c70a32c5eb162de220
Author: Martin Blix Grydeland <martin at varnish-software.com>
Date: Wed Oct 9 14:01:15 2019 +0200
Conditional fetch wait for streaming stale object
Wait for the stale object to become fully fetched, so that we can catch
fetch errors, before we unbusy the new object. This serves two
purposes. First it helps with request coalescing, and stops long chains of
IMS-updated short-TTL objects all streaming from a single slow body
fetch. Second it makes sure that all the object attributes are complete
when we copy them (this would be an issue for ie OA_GZIPBITS).
This patch OBE's r01646.vtc, and slightly patches r01648.vtc to expect a
503 instead of a 200 and a broken connection on the failing client.
Fixes: #3089
diff --git a/bin/varnishd/cache/cache_fetch.c b/bin/varnishd/cache/cache_fetch.c
index c80f1addf..3a1b91563 100644
--- a/bin/varnishd/cache/cache_fetch.c
+++ b/bin/varnishd/cache/cache_fetch.c
@@ -683,9 +683,37 @@ vbf_objiterator(void *priv, unsigned flush, const void *ptr, ssize_t len)
static enum fetch_step
vbf_stp_condfetch(struct worker *wrk, struct busyobj *bo)
{
+ struct boc *stale_boc;
+ enum boc_state_e stale_state;
CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
CHECK_OBJ_NOTNULL(bo, BUSYOBJ_MAGIC);
+ CHECK_OBJ_NOTNULL(bo->fetch_objcore, OBJCORE_MAGIC);
+ CHECK_OBJ_NOTNULL(bo->stale_oc, OBJCORE_MAGIC);
+
+ stale_boc = HSH_RefBoc(bo->stale_oc);
+ CHECK_OBJ_ORNULL(stale_boc, BOC_MAGIC);
+ if (stale_boc) {
+ /* Wait for the stale object to become fully fetched, so
+ * that we can catch fetch errors, before we unbusy the
+ * new object. This serves two purposes. First it helps
+ * with request coalesching, and stops long chains of
+ * IMS-updated short-TTL objects all streaming from a
+ * single slow body fetch. Second it makes sure that all
+ * the object attributes are complete when we copy them
+ * (this would be an issue for ie OA_GZIPBITS). */
+ ObjWaitState(bo->stale_oc, BOS_FINISHED);
+ stale_state = stale_boc->state;
+ HSH_DerefBoc(bo->wrk, bo->stale_oc);
+ stale_boc = NULL;
+ if (stale_state != BOS_FINISHED) {
+ (void)VFP_Error(bo->vfc, "Template object failed");
+ vbf_cleanup(bo);
+ wrk->stats->fetch_failed++;
+ return (F_STP_FAIL);
+ }
+ }
+ AZ(bo->stale_oc->flags & OC_F_FAILED);
AZ(vbf_beresp2obj(bo));
@@ -707,8 +735,6 @@ vbf_stp_condfetch(struct worker *wrk, struct busyobj *bo)
if (ObjIterate(wrk, bo->stale_oc, bo, vbf_objiterator, 0))
(void)VFP_Error(bo->vfc, "Template object failed");
- if (bo->stale_oc->flags & OC_F_FAILED)
- (void)VFP_Error(bo->vfc, "Template object failed");
if (bo->vfc->failed) {
vbf_cleanup(bo);
wrk->stats->fetch_failed++;
diff --git a/bin/varnishtest/tests/r01646.vtc b/bin/varnishtest/tests/r01646.vtc
deleted file mode 100644
index 10b640ea5..000000000
--- a/bin/varnishtest/tests/r01646.vtc
+++ /dev/null
@@ -1,57 +0,0 @@
-varnishtest "cond/stream race ?"
-
-barrier b1 cond 3
-barrier b2 cond 2
-
-server s1 {
- rxreq
- txresp -nolen -hdr "Transfer-Encoding: chunked" -hdr "Etag: foo"
- chunkedlen 32
- barrier b1 sync
- chunkedlen 32
- barrier b2 sync
- chunkedlen 32
- chunkedlen 0
-} -start
-
-server s2 {
- rxreq
- expect req.http.if-none-match == "foo"
- txresp -status 304
-} -start
-
-varnish v1 -vcl+backend {
- sub vcl_backend_fetch {
- if (bereq.http.foo == "s2") {
- set bereq.backend = s2;
- }
- }
- sub vcl_backend_response {
- set beresp.ttl = 1s;
- set beresp.grace = 0s;
- set beresp.keep = 30s;
- }
-} -start
-
-varnish v1 -cliok "param.set debug +syncvsl"
-
-client c1 {
- txreq
- rxresphdrs
- barrier b1 sync
- rxrespbody
- expect resp.bodylen == 96
-} -start
-
-client c2 {
- barrier b1 sync
- delay 2
- txreq -hdr "foo: s2"
- rxresphdrs
- barrier b2 sync
- rxrespbody
- expect resp.bodylen == 96
-} -start
-
-client c1 -wait
-client c2 -wait
diff --git a/bin/varnishtest/tests/r01648.vtc b/bin/varnishtest/tests/r01648.vtc
index 2780db864..7e565af00 100644
--- a/bin/varnishtest/tests/r01648.vtc
+++ b/bin/varnishtest/tests/r01648.vtc
@@ -62,8 +62,7 @@ client c2 {
txreq -hdr "Client: c2"
barrier b1 sync
rxresphdrs
- expect resp.status == 200
- expect resp.http.transfer-encoding == "chunked"
+ expect resp.status == 503
} -run
delay 1
@@ -76,4 +75,5 @@ client c3 {
expect resp.body == "abcdef"
} -run
+client c1 -wait
varnish v1 -expect fetch_failed == 2
diff --git a/bin/varnishtest/tests/r03089.vtc b/bin/varnishtest/tests/r03089.vtc
new file mode 100644
index 000000000..15d912e0b
--- /dev/null
+++ b/bin/varnishtest/tests/r03089.vtc
@@ -0,0 +1,69 @@
+varnishtest "r03089 - Assert error corrupt OA_GZIPBITS"
+
+barrier b1 cond 2
+
+server s1 {
+ rxreq
+ expect req.url == /included
+ expect req.http.accept-encoding == "gzip"
+ txresp -nolen -hdr "Content-Length: 49" -hdr "Content-Encoding: gzip" -hdr {Etag: "asdf"}
+ barrier b1 sync
+ delay 1
+ sendhex "1f 8b 08 08 ef 00 22 59 02 03 5f 00 0b 2e cd 53"
+ sendhex "f0 4d ac 54 30 32 04 22 2b 03 13 2b 13 73 85 d0"
+ sendhex "10 67 05 23 03 43 73 2e 00 cf 9b db c0 1d 00 00"
+ sendhex "00"
+} -start
+
+server s2 {
+ rxreq
+ expect req.url == /included
+ expect req.http.If-None-Match == {"asdf"}
+ barrier b1 sync
+ txresp -status 304 -nolen -hdr "Etag: asdf"
+} -start
+
+server s3 {
+ rxreq
+ expect req.url == /
+ txresp -gzipbody {<esi:include src="/included"/>}
+} -start
+
+varnish v1 -arg "-p debug=+syncvsl" -vcl+backend {
+ sub vcl_deliver {
+ }
+ sub vcl_backend_fetch {
+ if (bereq.url == "/") {
+ set bereq.backend = s3;
+ } else if (bereq.http.backend == "s1") {
+ set bereq.backend = s1;
+ } else {
+ set bereq.backend = s2;
+ }
+ }
+ sub vcl_backend_response {
+ if (bereq.url == "/") {
+ set beresp.do_esi = true;
+ } else {
+ set beresp.ttl = 0.1s;
+ set beresp.grace = 0s;
+ set beresp.keep = 1m;
+ }
+ }
+} -start
+
+client c1 {
+ txreq -url / -hdr "backend: s1" -hdr "Accept-encoding: gzip"
+ rxresp
+} -start
+
+delay 1
+
+client c2 {
+ txreq -url / -hdr "backend: s2" -hdr "Accept-encoding: gzip"
+ rxresp
+} -start
+
+client c1 -wait
+client c2 -wait
+
More information about the varnish-commit
mailing list