[experimental-ims] 42654fd Bugfixes in experimental-ims: - TTL was set to -1 after a 304 response - Operation with do_stream = true experimental-ims now passes (my version of) the rapid expire stress test
Geoff Simmons
geoff at varnish-cache.org
Mon Jan 16 17:40:52 CET 2012
commit 42654fd13f6f5427bbc57553dc50486f4ed38fa4
Author: Geoff Simmons <geoff at uplex.de>
Date: Mon Jan 16 17:36:21 2012 +0100
Bugfixes in experimental-ims:
- TTL was set to -1 after a 304 response
- Operation with do_stream = true
experimental-ims now passes (my version of) the rapid expire stress test
diff --git a/bin/varnishd/cache/cache.h b/bin/varnishd/cache/cache.h
index 898f3ba..c7924a9 100644
--- a/bin/varnishd/cache/cache.h
+++ b/bin/varnishd/cache/cache.h
@@ -810,7 +810,7 @@ void http_FilterHeader(const struct sess *sp, unsigned how);
/* Check if a refresh should be done */
void http_CheckRefresh(struct sess *sp);
/* Check if we got 304 response */
-void http_Check304(struct sess *sp);
+void http_Check304(struct sess *sp, struct busyobj *busyobj);
void http_PutProtocol(struct worker *w, unsigned vsl_id, const struct http *to,
const char *protocol);
diff --git a/bin/varnishd/cache/cache_center.c b/bin/varnishd/cache/cache_center.c
index 04a2444..3a91016 100644
--- a/bin/varnishd/cache/cache_center.c
+++ b/bin/varnishd/cache/cache_center.c
@@ -73,6 +73,8 @@ DOT acceptor -> first [style=bold,color=green]
#include "vtcp.h"
#include "vtim.h"
+#include "storage/storage.h"
+
#ifndef HAVE_SRANDOMDEV
#include "compat/srandomdev.h"
#endif
@@ -633,6 +635,15 @@ cnt_fetch(struct sess *sp, struct worker *wrk, struct req *req)
*/
wrk->busyobj->body_status = RFC2616_Body(sp);
+ /*
+ * If we found a candidate for conditional backend
+ * request, check now if the backend responded with 304,
+ * and merge headers from stale_obj into the busyobj if so.
+ * Any other response is handled as usual.
+ */
+ if (sp->stale_obj)
+ http_Check304(sp, wrk->busyobj);
+
req->err_code = http_GetStatus(wrk->busyobj->beresp);
/*
@@ -651,6 +662,12 @@ cnt_fetch(struct sess *sp, struct worker *wrk, struct req *req)
VCL_fetch_method(sp);
+ /* Cancel streaming if a stale object was validated */
+ /* XXX: But not if original beresp.status != 304 */
+ /* See also AZ(sp->stale_obj) in cnt_streambody() */
+ if (sp->stale_obj)
+ wrk->busyobj->do_stream = 0;
+
switch (req->handling) {
case VCL_RET_HIT_FOR_PASS:
if (req->objcore != NULL)
@@ -888,23 +905,6 @@ cnt_fetchbody(struct sess *sp, struct worker *wrk, struct req *req)
http_FilterFields(wrk, sp->vsl_id, hp2, hp,
pass ? HTTPH_R_PASS : HTTPH_A_INS);
- /*
- * If we found a candidate for conditional backend request, attempt it
- * now. If backend responds with 304, http_Check304() merges stale_obj
- * into wrk->obj, any other response is handled as usual. In either case,
- * the stale_obj is no longer needed in the cache, so discard it.
- */
- if (sp->stale_obj) {
- http_Check304(sp);
- if (wrk->busyobj->beresp->status == 304) {
- assert(req->obj->http->status == 200);
- wrk->busyobj->do_stream = 0;
- }
- EXP_Clr(&sp->stale_obj->exp);
- EXP_Rearm(sp->stale_obj);
- HSH_Deref(sp->wrk, NULL, &sp->stale_obj);
- AZ(sp->stale_obj);
- }
http_CopyHome(wrk, sp->vsl_id, hp2);
if (http_GetHdr(hp, H_Last_Modified, &b)
@@ -935,6 +935,25 @@ cnt_fetchbody(struct sess *sp, struct worker *wrk, struct req *req)
/* Use unmodified headers*/
i = FetchBody(wrk, req->obj);
+ /*
+ * If a stale_obj was found, dup its storage into the new obj,
+ * reset Content-Length from the size of the storage, and discard
+ * the stale_obj.
+ */
+ if (sp->stale_obj) {
+ STV_dup(sp, sp->stale_obj, req->obj);
+ assert(sp->stale_obj->len == req->obj->len);
+
+ http_Unset(req->obj->http, H_Content_Length);
+ http_PrintfHeader(sp->wrk, sp->fd, req->obj->http,
+ "Content-Length: %u", req->obj->len);
+
+ EXP_Clr(&sp->stale_obj->exp);
+ EXP_Rearm(sp->stale_obj);
+ HSH_Deref(sp->wrk, NULL, &sp->stale_obj);
+ AZ(sp->stale_obj);
+ }
+
http_Setup(wrk->busyobj->bereq, NULL);
http_Setup(wrk->busyobj->beresp, NULL);
wrk->busyobj->vfp = NULL;
@@ -985,6 +1004,7 @@ cnt_streambody(struct sess *sp, struct worker *wrk, struct req *req)
CHECK_OBJ_NOTNULL(sp, SESS_MAGIC);
CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
CHECK_OBJ_NOTNULL(req, REQ_MAGIC);
+ AZ(sp->stale_obj);
CHECK_OBJ_NOTNULL(wrk->busyobj, BUSYOBJ_MAGIC);
memset(&sctx, 0, sizeof sctx);
diff --git a/bin/varnishd/cache/cache_fetch.c b/bin/varnishd/cache/cache_fetch.c
index 98b88c0..7070612 100644
--- a/bin/varnishd/cache/cache_fetch.c
+++ b/bin/varnishd/cache/cache_fetch.c
@@ -509,12 +509,7 @@ FetchBody(struct worker *wrk, struct object *obj)
AssertObjCorePassOrBusy(obj->objcore);
AZ(bo->vgz_rx);
-
- /* If we've freshened from another object and got a "Not Modified"
- * response, then we have already duped the other object's body.
- */
- if (bo->beresp->status != 304)
- AZ(VTAILQ_FIRST(&obj->store));
+ AZ(VTAILQ_FIRST(&obj->store));
bo->fetch_obj = obj;
bo->fetch_failed = 0;
diff --git a/bin/varnishd/cache/cache_hash.c b/bin/varnishd/cache/cache_hash.c
index 6ae193d..827d6af 100644
--- a/bin/varnishd/cache/cache_hash.c
+++ b/bin/varnishd/cache/cache_hash.c
@@ -307,7 +307,7 @@ HSH_Lookup(struct sess *sp, struct objhead **poh)
CHECK_OBJ_NOTNULL(sp->req->http, HTTP_MAGIC);
AN(sp->req->director);
AN(hash);
- AZ(sp->stale_obj);
+ AZ(sp->stale_obj);
wrk = sp->wrk;
HSH_Prealloc(sp);
diff --git a/bin/varnishd/cache/cache_http.c b/bin/varnishd/cache/cache_http.c
index 4f48d52..7ea85f2 100644
--- a/bin/varnishd/cache/cache_http.c
+++ b/bin/varnishd/cache/cache_http.c
@@ -34,7 +34,6 @@
#include <stdio.h>
#include "cache.h"
-#include "storage/storage.h"
#include "vct.h"
@@ -951,52 +950,46 @@ http_CheckRefresh(struct sess *sp)
*/
void
-http_Check304(struct sess *sp)
+http_Check304(struct sess *sp, struct busyobj *busyobj)
{
- struct object *o, *o_stale;
+ struct object *o_stale;
char *p;
CHECK_OBJ_NOTNULL(sp, SESS_MAGIC);
+ CHECK_OBJ_NOTNULL(busyobj, BUSYOBJ_MAGIC);
o_stale = sp->stale_obj;
CHECK_OBJ_NOTNULL(o_stale, OBJECT_MAGIC);
- o = sp->req->obj;
- CHECK_OBJ_NOTNULL(o, OBJECT_MAGIC);
- if (sp->wrk->busyobj->beresp->status != 304) {
+ if (busyobj->beresp->status != 304) {
/*
* IMS/INM headers may have been removed in VCL, so only count a
* non-validating response if they were present in the request.
*/
- if (http_GetHdr(sp->wrk->busyobj->bereq, H_If_Modified_Since, &p)
- || http_GetHdr(sp->wrk->busyobj->bereq, H_If_None_Match, &p))
+ if (http_GetHdr(busyobj->bereq, H_If_Modified_Since, &p)
+ || http_GetHdr(busyobj->bereq, H_If_None_Match, &p))
sp->wrk->stats.fetch_not_validated++;
+
+ HSH_Deref(sp->wrk, NULL, &sp->stale_obj);
+ AZ(sp->stale_obj);
return;
}
/*
* Copy headers we need from the stale object into the 304 response
*/
- http_FilterMissingFields(sp->wrk, sp->fd, o->http, o_stale->http);
-
- /*
- * Dup the stale object's storage in to the new object
- * and reset Content-Length from the size of the storage.
- */
- STV_dup(sp, o_stale, o);
- http_Unset(o->http, H_Content_Length);
- http_PrintfHeader(sp->wrk, sp->fd, o->http, "Content-Length: %u",
- o->len);
+ http_FilterMissingFields(sp->wrk, sp->fd, busyobj->beresp,
+ o_stale->http);
- http_SetResp(o->http, "HTTP/1.1", 200, "Ok Not Modified");
- http_SetH(o->http, HTTP_HDR_REQ, "GET");
- http_copyh(o->http, sp->wrk->busyobj->bereq, HTTP_HDR_URL);
+ http_SetResp(busyobj->beresp, "HTTP/1.1", 200, "Ok Not Modified");
+ http_SetH(busyobj->beresp, HTTP_HDR_REQ, "GET");
+ http_copyh(busyobj->beresp, busyobj->bereq, HTTP_HDR_URL);
/*
* XXX: Are we copying all the necessary fields from stale_obj?
* Should we copy o_stale->hits into o->hits?
+ * What about do_esi and do_g(un)zip?
*/
- o->response = 200;
- o->gziped = o_stale->gziped;
+ busyobj->is_gzip = o_stale->gziped;
AZ(o_stale->objcore->flags & OC_F_BUSY);
}
diff --git a/bin/varnishtest/tests/i00001.vtc b/bin/varnishtest/tests/i00001.vtc
index 6383a1f..1d25a8f 100644
--- a/bin/varnishtest/tests/i00001.vtc
+++ b/bin/varnishtest/tests/i00001.vtc
@@ -19,10 +19,10 @@ server s1 {
txresp -status 304
} -start
-varnish v1 -vcl+backend {
- sub vcl_fetch {
- set beresp.ttl = 0.5s;
- }
+varnish v1 -arg "-t 0.5" -vcl+backend {
+ sub vcl_fetch {
+ set beresp.http.X-TTL = beresp.ttl;
+ }
} -start
client c1 {
@@ -32,6 +32,7 @@ client c1 {
expect resp.http.content-length == 6
expect resp.bodylen == 6
expect resp.http.Age == 0
+ expect resp.http.X-TTL == 0.500
} -run
delay 0.6
@@ -40,6 +41,7 @@ client c1 -run
varnish v1 -expect fetch_304 == 1
varnish v1 -expect fetch_not_validated == 0
+varnish v1 -expect cache_hitpass == 0
server s1 -wait
varnish v1 -stop
diff --git a/bin/varnishtest/tests/i00003.vtc b/bin/varnishtest/tests/i00003.vtc
index fb66266..b4220e1 100644
--- a/bin/varnishtest/tests/i00003.vtc
+++ b/bin/varnishtest/tests/i00003.vtc
@@ -52,6 +52,47 @@ varnish v1 -expect fetch_not_validated == 0
server s1 -wait
varnish v1 -stop
+## Verify that backend conditionals work correctly with streaming
+## i.e. streaming is cancelled if a stale object is delivered
+
+server s1 {
+ rxreq
+ expect req.url == "/foo"
+ txresp -status 200 \
+ -hdr "Last-Modified: Thu, 26 Jun 2008 12:00:01 GMT" \
+ -hdr "ETag: foo" \
+ -body "abcdefghijklmonpqrstuvwxyz"
+
+ rxreq
+ expect req.url == "/foo"
+ expect req.http.If-Modified-Since == "Thu, 26 Jun 2008 12:00:01 GMT"
+ expect req.http.If-None-Match == "foo"
+ txresp -status 304
+} -start
+
+varnish v1 -vcl {
+ backend s1 {
+ .host = "${s1_addr}"; .port = "${s1_port}";
+ }
+
+ sub vcl_fetch {
+ set beresp.ttl = 0.5s;
+ set beresp.do_stream = true;
+ }
+} -start
+
+client c1 -run
+
+delay 0.6
+
+client c1 -run
+
+varnish v1 -expect fetch_304 == 1
+varnish v1 -expect fetch_not_validated == 0
+
+server s1 -wait
+varnish v1 -stop
+
## Verify that if a client sends a conditional request to Varnish, then Varnish
## can return a 304 response to the client after it got 304 from the backend
@@ -99,9 +140,8 @@ varnish v1 -expect fetch_not_validated == 0
server s2 -wait
varnish v1 -stop
-##
-## If stale_obj has a gzipped body, make sure it interacts correctly with clients
-##
+## If stale_obj has a gzipped body, make sure it interacts correctly
+## with clients
server s2 {
rxreq
@@ -144,3 +184,6 @@ client c2 {
varnish v1 -expect fetch_304 == 1
varnish v1 -expect fetch_not_validated == 0
+
+server s2 -wait
+varnish v1 -stop
diff --git a/bin/varnishtest/tests/i00004.vtc b/bin/varnishtest/tests/i00004.vtc
index 77fb9a6..a173e29 100644
--- a/bin/varnishtest/tests/i00004.vtc
+++ b/bin/varnishtest/tests/i00004.vtc
@@ -49,6 +49,7 @@ client c1 -run
varnish v1 -expect fetch_304 == 1
varnish v1 -expect fetch_not_validated == 0
+varnish v1 -expect cache_hitpass == 0
server s1 -wait
varnish v1 -stop
More information about the varnish-commit
mailing list