[master] 468a047 Expose is_hit to VCL as resp.is_hit
Federico Schwindt
fgsch at lodoss.net
Wed Jan 7 01:12:41 CET 2015
I must say I'm not very thrilled about having another variable to check
whether it was a hit.
How much of an issue this really is and can we fix the code so we still use
obj.hits?
On Wed, Jan 7, 2015 at 12:10 AM, Federico Schwindt <fgsch at lodoss.net> wrote:
> I must say I'm not very thrilled about having another variable to check
> whether it was a hit.
>
> How much of an issue this really is and can we fix the code so we still
> use obj.hits?
>
> On Tue, Jan 6, 2015 at 9:41 PM, Nils Goroll <nils.goroll at uplex.de> wrote:
>
>>
>> commit 468a0472fa313dbff69269afc085957642011abc
>> Author: Nils Goroll <nils.goroll at uplex.de>
>> Date: Tue Jan 6 22:24:21 2015 +0100
>>
>> Expose is_hit to VCL as resp.is_hit
>>
>> The common method to check obj.hits is racy: While the update to
>> the hits counter happens under a lock, a hit could have happened
>> before the missing request gets to read obj.hits.
>>
>> diff --git a/bin/varnishd/cache/cache.h b/bin/varnishd/cache/cache.h
>> index 2e7fa47..82bf5ed 100644
>> --- a/bin/varnishd/cache/cache.h
>> +++ b/bin/varnishd/cache/cache.h
>> @@ -540,7 +540,7 @@ struct req {
>> int restarts;
>> int esi_level;
>>
>> -#define REQ_FLAG(l, r, w, d) unsigned l:1;
>> +#define REQ_FLAG(l, o, r, w, d) unsigned l:1;
>> #include "tbl/req_flags.h"
>> #undef REQ_FLAG
>>
>> diff --git a/bin/varnishd/cache/cache_panic.c
>> b/bin/varnishd/cache/cache_panic.c
>> index d45458e..9bd99a7 100644
>> --- a/bin/varnishd/cache/cache_panic.c
>> +++ b/bin/varnishd/cache/cache_panic.c
>> @@ -415,7 +415,7 @@ pan_req(const struct req *req)
>> }
>>
>> VSB_printf(pan_vsp, " flags = {\n");
>> -#define REQ_FLAG(l, r, w, d) if(req->l) VSB_printf(pan_vsp, " " #l
>> ",\n");
>> +#define REQ_FLAG(l, o, r, w, d) if(req->l) VSB_printf(pan_vsp, " " #l
>> ",\n");
>> #include "tbl/req_flags.h"
>> #undef REQ_FLAG
>> VSB_printf(pan_vsp, " }\n");
>> diff --git a/bin/varnishd/cache/cache_vrt_var.c
>> b/bin/varnishd/cache/cache_vrt_var.c
>> index 4c6f2c6..5b2e5c2 100644
>> --- a/bin/varnishd/cache/cache_vrt_var.c
>> +++ b/bin/varnishd/cache/cache_vrt_var.c
>> @@ -566,29 +566,29 @@ VRT_r_bereq_xid(VRT_CTX)
>> * req fields
>> */
>>
>> -#define VREQW0(field)
>> -#define VREQW1(field) \
>> +#define VREQW0(obj, field)
>> +#define VREQW1(obj, field) \
>> void \
>> -VRT_l_req_##field(VRT_CTX, unsigned a) \
>> +VRT_l_##obj##_##field(VRT_CTX, unsigned a) \
>> { \
>> CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC); \
>> CHECK_OBJ_NOTNULL(ctx->req, REQ_MAGIC); \
>> ctx->req->field = a ? 1 : 0; \
>> }
>>
>> -#define VREQR0(field)
>> -#define VREQR1(field) \
>> +#define VREQR0(obj, field)
>> +#define VREQR1(obj, field) \
>> unsigned \
>> -VRT_r_req_##field(VRT_CTX) \
>> +VRT_r_##obj##_##field(VRT_CTX) \
>> { \
>> CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC); \
>> CHECK_OBJ_NOTNULL(ctx->req, REQ_MAGIC); \
>> return (ctx->req->field); \
>> }
>>
>> -#define REQ_FLAG(l, r, w, d) \
>> - VREQR##r(l) \
>> - VREQW##w(l)
>> +#define REQ_FLAG(l, o, r, w, d) \
>> + VREQR##r(o, l) \
>> + VREQW##w(o, l)
>> #include "tbl/req_flags.h"
>> #undef REQ_FLAG
>>
>> diff --git a/bin/varnishtest/tests/v00013.vtc
>> b/bin/varnishtest/tests/v00013.vtc
>> index c428add..19e9f26 100644
>> --- a/bin/varnishtest/tests/v00013.vtc
>> +++ b/bin/varnishtest/tests/v00013.vtc
>> @@ -13,6 +13,11 @@ varnish v1 -vcl+backend {
>>
>> sub vcl_deliver {
>> set resp.http.foo = obj.hits;
>> + if (resp.is_hit) {
>> + set resp.http.X-Hit = "yes";
>> + } else {
>> + set resp.http.X-Hit = "no";
>> + }
>> }
>> } -start
>>
>> @@ -21,16 +26,19 @@ client c1 {
>> rxresp
>> expect resp.status == 200
>> expect resp.http.foo == 0
>> + expect resp.http.X-hit == "no"
>>
>> txreq
>> rxresp
>> expect resp.status == 200
>> expect resp.http.foo == 1
>> + expect resp.http.X-hit == "yes"
>>
>> txreq -url /foo
>> rxresp
>> expect resp.status == 200
>> expect resp.http.foo == 0
>> + expect resp.http.X-hit == "no"
>> delay .1
>>
>> txreq
>> @@ -38,4 +46,5 @@ client c1 {
>> expect resp.status == 200
>> expect resp.http.foo == 2
>> expect resp.http.bar >= 0.100
>> + expect resp.http.X-hit == "yes"
>> } -run
>> diff --git a/bin/varnishtest/tests/v00039.vtc
>> b/bin/varnishtest/tests/v00039.vtc
>> index 029e114..c248aa8 100644
>> --- a/bin/varnishtest/tests/v00039.vtc
>> +++ b/bin/varnishtest/tests/v00039.vtc
>> @@ -13,6 +13,11 @@ varnish v1 \
>> -vcl+backend {
>> sub vcl_deliver {
>> set resp.http.hits = obj.hits;
>> + if (resp.is_hit) {
>> + set resp.http.X-Hit = "yes";
>> + } else {
>> + set resp.http.X-Hit = "no";
>> + }
>> }
>> } -start
>>
>> @@ -23,6 +28,7 @@ client c1 {
>> expect resp.status == 200
>> expect resp.bodylen == 6
>> expect resp.http.hits == 0
>> + expect resp.http.X-Hit == "no"
>>
>> # This is a hit -> hits == 1
>> txreq -url "/" -hdr "Bar: 1"
>> @@ -30,6 +36,7 @@ client c1 {
>> expect resp.status == 200
>> expect resp.bodylen == 6
>> expect resp.http.hits == 1
>> + expect resp.http.X-Hit == "yes"
>>
>> # This is a miss on different vary -> hits == 0
>> txreq -url "/" -hdr "Bar: 2"
>> @@ -37,6 +44,7 @@ client c1 {
>> expect resp.status == 200
>> expect resp.bodylen == 4
>> expect resp.http.hits == 0
>> + expect resp.http.X-Hit == "no"
>>
>> # This is a hit -> hits == 1
>> txreq -url "/" -hdr "Bar: 2"
>> @@ -44,6 +52,7 @@ client c1 {
>> expect resp.status == 200
>> expect resp.bodylen == 4
>> expect resp.http.hits == 1
>> + expect resp.http.X-Hit == "yes"
>>
>> } -run
>>
>> diff --git a/include/tbl/req_flags.h b/include/tbl/req_flags.h
>> index 6757d90..55f20f7 100644
>> --- a/include/tbl/req_flags.h
>> +++ b/include/tbl/req_flags.h
>> @@ -29,11 +29,11 @@
>>
>> /*lint -save -e525 -e539 */
>>
>> -/* lower, vcl_r, vcl_w, doc */
>> -REQ_FLAG(disable_esi, 0, 0, "")
>> -REQ_FLAG(hash_ignore_busy, 1, 1, "")
>> -REQ_FLAG(hash_always_miss, 1, 1, "")
>> -REQ_FLAG(is_hit, 0, 0, "")
>> -REQ_FLAG(wantbody, 0, 0, "")
>> -REQ_FLAG(gzip_resp, 0, 0, "")
>> +/* lower, vcl_obj, vcl_r, vcl_w, doc */
>> +REQ_FLAG(disable_esi, none, 0, 0, "")
>> +REQ_FLAG(hash_ignore_busy, req, 1, 1, "")
>> +REQ_FLAG(hash_always_miss, req, 1, 1, "")
>> +REQ_FLAG(is_hit, resp, 1, 0, "")
>> +REQ_FLAG(wantbody, none, 0, 0, "")
>> +REQ_FLAG(gzip_resp, none, 0, 0, "")
>> /*lint -restore */
>> diff --git a/lib/libvcc/generate.py b/lib/libvcc/generate.py
>> index e582c0f..699bab3 100755
>> --- a/lib/libvcc/generate.py
>> +++ b/lib/libvcc/generate.py
>> @@ -564,8 +564,10 @@ sp_variables = [
>> 'INT',
>> ( 'hit', 'deliver',),
>> ( ), """
>> - The count of cache-hits on this object. A value of 0
>> indicates a
>> - cache miss.
>> + The count of cache-hits on this object.
>> +
>> + NB: obj.hits is not a reliable way to determine cache
>> + misses. See resp.is_hit.
>> """
>> ),
>> ('obj.http.',
>> @@ -645,6 +647,13 @@ sp_variables = [
>> The corresponding HTTP header.
>> """
>> ),
>> + ('resp.is_hit',
>> + 'BOOL',
>> + ( 'deliver',),
>> + ( ), """
>> + If this response is from a cache hit.
>> + """
>> + ),
>> ('now',
>> 'TIME',
>> ( 'all',),
>>
>> _______________________________________________
>> varnish-commit mailing list
>> varnish-commit at varnish-cache.org
>> https://www.varnish-cache.org/lists/mailman/listinfo/varnish-commit
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://www.varnish-cache.org/lists/pipermail/varnish-dev/attachments/20150107/a7bf4c0d/attachment.html>
More information about the varnish-dev
mailing list