r4099 - in trunk/varnish-cache: bin/varnishd include lib/libvcl

phk at projects.linpro.no phk at projects.linpro.no
Mon Jun 8 23:40:49 CEST 2009


Author: phk
Date: 2009-06-08 23:40:48 +0200 (Mon, 08 Jun 2009)
New Revision: 4099

Modified:
   trunk/varnish-cache/bin/varnishd/cache_expire.c
   trunk/varnish-cache/bin/varnishd/default.vcl
   trunk/varnish-cache/include/vcl.h
   trunk/varnish-cache/include/vcl_returns.h
   trunk/varnish-cache/lib/libvcl/vcc_fixed_token.c
   trunk/varnish-cache/lib/libvcl/vcc_gen_fixed_token.tcl
   trunk/varnish-cache/lib/libvcl/vcc_gen_obj.tcl
   trunk/varnish-cache/lib/libvcl/vcc_obj.c
Log:
Remove the vcl_discard{} facility, it does not seem to bring benefits
which outweigh the trouble it causes.

The original idea was to allow intelligent purging for space reason
in a size-constrained storage.

In practice, storage is seldomly constrained.

In the cases where it is, I have yet to see any documented benefit
from using vcl_discard{}, likely because of the fragmentation such
use would cause.

The implementation of the vcl_discard callback had severe costs in
code complexity (not fully unrolled by this commit) and locking
activity.

The straw that breaks the camels back, is that faithfull implementation
of vcl_discard{} in -spersistence would force us to relocate objects,
at a cost likely higher than picking them up from the backend.



Modified: trunk/varnish-cache/bin/varnishd/cache_expire.c
===================================================================
--- trunk/varnish-cache/bin/varnishd/cache_expire.c	2009-06-06 18:43:28 UTC (rev 4098)
+++ trunk/varnish-cache/bin/varnishd/cache_expire.c	2009-06-08 21:40:48 UTC (rev 4099)
@@ -309,12 +309,6 @@
 			break;
 	}
 	if (oc != NULL) {
-		/*
-		 * We hazzard the guess that the object is more likely to
-		 * be tossed than kept, and forge ahead on the work to save
-		 * a lock cycle.  If the object is kept, we reverse these
-		 * actions below.
-		 */
 		VTAILQ_REMOVE(&lru, oc, lru_list);
 		oc->flags &= ~OC_F_ONLRU;
 		binheap_delete(exp_heap, oc->timer_idx);
@@ -326,35 +320,9 @@
 	if (oc == NULL)
 		return (-1);
 
-	/*
-	 * Ask VCL in the context of the clients session, in order to allow
-	 * client QoS considerations to inform the decision. Temporarily
-	 * substitute the object we want to nuke for the sessions own object.
-	 */
-	o = sp->obj;
-	sp->obj = oc->obj;
-	VCL_discard_method(sp);
-	sp->obj = o;
-	o = oc->obj;
-
-	if (sp->handling == VCL_RET_DISCARD) {
-		WSL(sp->wrk, SLT_ExpKill, 0, "%u LRU", o->xid);
-		HSH_Deref(sp->wrk, &o);
-		return (1);
-	}
-
-	assert(sp->handling == VCL_RET_KEEP);
-
-	/* Insert in binheap and lru again */
-	Lck_Lock(&exp_mtx);
-	VSL_stats->n_lru_nuked--;		/* It was premature */
-	VSL_stats->n_lru_saved++;
-	binheap_insert(exp_heap, oc);
-	assert(oc->timer_idx != BINHEAP_NOIDX);
-	VTAILQ_INSERT_TAIL(&lru, oc, lru_list);
-	oc->flags |= OC_F_ONLRU;
-	Lck_Unlock(&exp_mtx);
-	return (0);
+	WSL(sp->wrk, SLT_ExpKill, 0, "%u LRU", o->xid);
+	HSH_Deref(sp->wrk, &o);
+	return (1);
 }
 
 /*--------------------------------------------------------------------

Modified: trunk/varnish-cache/bin/varnishd/default.vcl
===================================================================
--- trunk/varnish-cache/bin/varnishd/default.vcl	2009-06-06 18:43:28 UTC (rev 4098)
+++ trunk/varnish-cache/bin/varnishd/default.vcl	2009-06-08 21:40:48 UTC (rev 4099)
@@ -110,11 +110,6 @@
     return (deliver);
 }
 
-sub vcl_discard {
-    /* XXX: Do not redefine vcl_discard{}, it is not yet supported */
-    return (discard);
-}
-
 sub vcl_timeout {
     /* XXX: Do not redefine vcl_timeout{}, it is not yet supported */
     return (discard);

Modified: trunk/varnish-cache/include/vcl.h
===================================================================
--- trunk/varnish-cache/include/vcl.h	2009-06-06 18:43:28 UTC (rev 4098)
+++ trunk/varnish-cache/include/vcl.h	2009-06-08 21:40:48 UTC (rev 4099)
@@ -23,10 +23,9 @@
 #define VCL_MET_FETCH		(1 << 6)
 #define VCL_MET_DELIVER		(1 << 7)
 #define VCL_MET_TIMEOUT		(1 << 8)
-#define VCL_MET_DISCARD		(1 << 9)
-#define VCL_MET_ERROR		(1 << 10)
+#define VCL_MET_ERROR		(1 << 9)
 
-#define VCL_MET_MAX		11
+#define VCL_MET_MAX		10
 
 /* VCL Returns */
 #define VCL_RET_ERROR		0
@@ -71,6 +70,5 @@
 	vcl_func_f	*fetch_func;
 	vcl_func_f	*deliver_func;
 	vcl_func_f	*timeout_func;
-	vcl_func_f	*discard_func;
 	vcl_func_f	*error_func;
 };

Modified: trunk/varnish-cache/include/vcl_returns.h
===================================================================
--- trunk/varnish-cache/include/vcl_returns.h	2009-06-06 18:43:28 UTC (rev 4098)
+++ trunk/varnish-cache/include/vcl_returns.h	2009-06-08 21:40:48 UTC (rev 4099)
@@ -64,10 +64,6 @@
      ((1 << VCL_RET_FETCH)
     | (1 << VCL_RET_DISCARD)
 ))
-VCL_MET_MAC(discard,DISCARD,
-     ((1 << VCL_RET_DISCARD)
-    | (1 << VCL_RET_KEEP)
-))
 VCL_MET_MAC(error,ERROR,
      ((1 << VCL_RET_RESTART)
     | (1 << VCL_RET_DELIVER)

Modified: trunk/varnish-cache/lib/libvcl/vcc_fixed_token.c
===================================================================
--- trunk/varnish-cache/lib/libvcl/vcc_fixed_token.c	2009-06-06 18:43:28 UTC (rev 4098)
+++ trunk/varnish-cache/lib/libvcl/vcc_fixed_token.c	2009-06-08 21:40:48 UTC (rev 4099)
@@ -175,9 +175,8 @@
 	vsb_cat(sb, "#define VCL_MET_FETCH\t\t(1 << 6)\n");
 	vsb_cat(sb, "#define VCL_MET_DELIVER\t\t(1 << 7)\n");
 	vsb_cat(sb, "#define VCL_MET_TIMEOUT\t\t(1 << 8)\n");
-	vsb_cat(sb, "#define VCL_MET_DISCARD\t\t(1 << 9)\n");
-	vsb_cat(sb, "#define VCL_MET_ERROR\t\t(1 << 10)\n");
-	vsb_cat(sb, "\n#define VCL_MET_MAX\t\t11\n\n");
+	vsb_cat(sb, "#define VCL_MET_ERROR\t\t(1 << 9)\n");
+	vsb_cat(sb, "\n#define VCL_MET_MAX\t\t10\n\n");
 	vsb_cat(sb, "/* VCL Returns */\n#define VCL_RET_ERROR\t\t0\n");
 	vsb_cat(sb, "#define VCL_RET_LOOKUP\t\t1\n#define VCL_RET_HASH\t\t2");
 	vsb_cat(sb, "\n#define VCL_RET_PIPE\t\t3\n#define VCL_RET_PASS\t\t4");
@@ -199,8 +198,7 @@
 	vsb_cat(sb, "\tvcl_func_f\t*miss_func;\n\tvcl_func_f\t*hit_func;\n");
 	vsb_cat(sb, "\tvcl_func_f\t*fetch_func;\n\tvcl_func_f\t*deliver_fun");
 	vsb_cat(sb, "c;\n\tvcl_func_f\t*timeout_func;\n");
-	vsb_cat(sb, "\tvcl_func_f\t*discard_func;\n\tvcl_func_f\t*error_fun");
-	vsb_cat(sb, "c;\n};\n");
+	vsb_cat(sb, "\tvcl_func_f\t*error_func;\n};\n");
 
 	/* ../../include/vrt.h */
 

Modified: trunk/varnish-cache/lib/libvcl/vcc_gen_fixed_token.tcl
===================================================================
--- trunk/varnish-cache/lib/libvcl/vcc_gen_fixed_token.tcl	2009-06-06 18:43:28 UTC (rev 4098)
+++ trunk/varnish-cache/lib/libvcl/vcc_gen_fixed_token.tcl	2009-06-08 21:40:48 UTC (rev 4099)
@@ -43,7 +43,6 @@
 	{fetch		{error restart pass deliver}}
 	{deliver	{restart deliver}}
 	{timeout	{fetch discard}}
-	{discard	{discard keep}}
 	{error		{restart deliver}}
 }
 

Modified: trunk/varnish-cache/lib/libvcl/vcc_gen_obj.tcl
===================================================================
--- trunk/varnish-cache/lib/libvcl/vcc_gen_obj.tcl	2009-06-06 18:43:28 UTC (rev 4098)
+++ trunk/varnish-cache/lib/libvcl/vcc_gen_obj.tcl	2009-06-08 21:40:48 UTC (rev 4099)
@@ -233,17 +233,17 @@
     }
     { obj.ttl
 	RW TIME
-	{                         hit               discard timeout error}
+	{                         hit               timeout error}
 	"const struct sess *"
     }
     { obj.grace
 	RW TIME
-	{                         hit               discard timeout error}
+	{                         hit               timeout error}
 	"const struct sess *"
     }
     { obj.lastuse
 	RO TIME
-	{                         hit       deliver discard timeout error}
+	{                         hit       deliver timeout error}
 	"const struct sess *"
     }
     { obj.hash
@@ -280,11 +280,11 @@
     # XXX: or delta times in VCL programs, so this shouldn't be needed /phk
     { now
 	    RO TIME
-	    {recv pipe pass hash miss hit fetch deliver discard timeout}
+	    {recv pipe pass hash miss hit fetch deliver timeout}
 	    "const struct sess *"
     }
     { req.backend.healthy	RO BOOL
-	    {recv pipe pass hash miss hit fetch deliver discard timeout}
+	    {recv pipe pass hash miss hit fetch deliver timeout}
 	    "const struct sess *"
     }
 

Modified: trunk/varnish-cache/lib/libvcl/vcc_obj.c
===================================================================
--- trunk/varnish-cache/lib/libvcl/vcc_obj.c	2009-06-06 18:43:28 UTC (rev 4098)
+++ trunk/varnish-cache/lib/libvcl/vcc_obj.c	2009-06-08 21:40:48 UTC (rev 4099)
@@ -215,18 +215,17 @@
 	{ "obj.ttl", TIME, 7,
 	    "VRT_r_obj_ttl(sp)",	    "VRT_l_obj_ttl(sp, ",
 	    V_RW,	    0,
-	    VCL_MET_HIT | VCL_MET_DISCARD | VCL_MET_TIMEOUT | VCL_MET_ERROR
+	    VCL_MET_HIT | VCL_MET_TIMEOUT | VCL_MET_ERROR
 	},
 	{ "obj.grace", TIME, 9,
 	    "VRT_r_obj_grace(sp)",	    "VRT_l_obj_grace(sp, ",
 	    V_RW,	    0,
-	    VCL_MET_HIT | VCL_MET_DISCARD | VCL_MET_TIMEOUT | VCL_MET_ERROR
+	    VCL_MET_HIT | VCL_MET_TIMEOUT | VCL_MET_ERROR
 	},
 	{ "obj.lastuse", TIME, 11,
 	    "VRT_r_obj_lastuse(sp)",	    NULL,
 	    V_RO,	    0,
-	    VCL_MET_HIT | VCL_MET_DELIVER | VCL_MET_DISCARD | VCL_MET_TIMEOUT
-	     | VCL_MET_ERROR
+	    VCL_MET_HIT | VCL_MET_DELIVER | VCL_MET_TIMEOUT | VCL_MET_ERROR
 	},
 	{ "obj.hash", STRING, 8,
 	    "VRT_r_obj_hash(sp)",	    NULL,
@@ -258,14 +257,14 @@
 	    V_RO,	    0,
 	    VCL_MET_RECV | VCL_MET_PIPE | VCL_MET_PASS | VCL_MET_HASH
 	     | VCL_MET_MISS | VCL_MET_HIT | VCL_MET_FETCH | VCL_MET_DELIVER
-	     | VCL_MET_DISCARD | VCL_MET_TIMEOUT
+	     | VCL_MET_TIMEOUT
 	},
 	{ "req.backend.healthy", BOOL, 19,
 	    "VRT_r_req_backend_healthy(sp)",	    NULL,
 	    V_RO,	    0,
 	    VCL_MET_RECV | VCL_MET_PIPE | VCL_MET_PASS | VCL_MET_HASH
 	     | VCL_MET_MISS | VCL_MET_HIT | VCL_MET_FETCH | VCL_MET_DELIVER
-	     | VCL_MET_DISCARD | VCL_MET_TIMEOUT
+	     | VCL_MET_TIMEOUT
 	},
 	{ NULL }
 };



More information about the varnish-commit mailing list