[master] e8cfcc2 Add beresp.storage

Federico G. Schwindt fgsch at lodoss.net
Mon Oct 17 11:49:05 CEST 2016 

commit e8cfcc22b02dd42e38deefba30c3e733d76896cf
Author: Federico G. Schwindt <fgsch at lodoss.net>
Date:   Thu Oct 13 13:51:05 2016 +0100

    Add beresp.storage
    
    This supersedes beresp.storage_hint but is kept around for backward
    compatibility until the next major release.
    
    Setting beresp.storage_hint to a valid storage will set beresp.storage
    as well. If the storage is invalid, beresp.storage is left untouched.
    
    This also reworks how the storage selection is done when multiple
    storages are defined but none is explicitly set, and what nuke limit we
    use. Previously we will first try with a nuke limit of 0, and on failure
    retry with the configured nuke limit. Furthermore, if we did not specify
    the storage we will go through every one in RR fashion with a nuke limit
    of 0 before retrying.
    
    With this change we initialise beresp.storage with a storage backend from
    the list before we enter v_b_r{}. At the end of v_b_r{} we will use
    whatever storage is in beresp.storage. If the storage is NULL we fail
    the request. In all cases we use the nuke limit.

diff --git a/bin/varnishd/cache/cache.h b/bin/varnishd/cache/cache.h
index 57f260c..9d63a5a 100644
--- a/bin/varnishd/cache/cache.h
+++ b/bin/varnishd/cache/cache.h
@@ -507,6 +507,7 @@ struct busyobj {
 	struct acct_bereq	acct;
 
 	const char		*storage_hint;
+	const struct stevedore	*storage;
 	const struct director	*director_req;
 	const struct director	*director_resp;
 	enum director_state_e	director_state;
@@ -1083,7 +1084,7 @@ void RFC2616_Vary_AE(struct http *hp);
 
 /* stevedore.c */
 int STV_NewObject(struct worker *, struct objcore *,
-    const char *hint, unsigned len);
+    const struct stevedore *, unsigned len);
 
 /*
  * A normal pointer difference is signed, but we never want a negative value
diff --git a/bin/varnishd/cache/cache_fetch.c b/bin/varnishd/cache/cache_fetch.c
index 40664b4..9051689 100644
--- a/bin/varnishd/cache/cache_fetch.c
+++ b/bin/varnishd/cache/cache_fetch.c
@@ -33,6 +33,7 @@
 #include "cache_director.h"
 #include "cache_filter.h"
 #include "hash/hash_slinger.h"
+#include "storage/storage.h"
 #include "vcl.h"
 #include "vtim.h"
 
@@ -46,7 +47,7 @@ static int
 vbf_allocobj(struct busyobj *bo, unsigned l)
 {
 	struct objcore *oc;
-	const char *storage_hint;
+	const struct stevedore *stv;
 	double lifetime;
 
 	CHECK_OBJ_NOTNULL(bo, BUSYOBJ_MAGIC);
@@ -56,16 +57,20 @@ vbf_allocobj(struct busyobj *bo, unsigned l)
 	lifetime = oc->ttl + oc->grace + oc->keep;
 
 	if (bo->uncacheable || lifetime < cache_param->shortlived)
-		storage_hint = TRANSIENT_STORAGE;
+		stv = stv_transient;
 	else
-		storage_hint = bo->storage_hint;
+		stv = bo->storage;
 
+	bo->storage = NULL;
 	bo->storage_hint = NULL;
 
-	if (STV_NewObject(bo->wrk, bo->fetch_objcore, storage_hint, l))
+	if (stv == NULL)
+		return (0);
+
+	if (STV_NewObject(bo->wrk, bo->fetch_objcore, stv, l))
 		return (1);
 
-	if (storage_hint != NULL && !strcmp(storage_hint, TRANSIENT_STORAGE))
+	if (stv == stv_transient)
 		return (0);
 
 	/*
@@ -78,7 +83,7 @@ vbf_allocobj(struct busyobj *bo, unsigned l)
 	oc->grace = 0.0;
 	oc->keep = 0.0;
 	return (STV_NewObject(bo->wrk, bo->fetch_objcore,
-	    TRANSIENT_STORAGE, l));
+	    stv_transient, l));
 }
 
 /*--------------------------------------------------------------------
@@ -169,6 +174,7 @@ vbf_stp_mkbereq(struct worker *wrk, struct busyobj *bo)
 	CHECK_OBJ_NOTNULL(bo->req, REQ_MAGIC);
 
 	assert(bo->fetch_objcore->boc->state == BOS_INVALID);
+	AZ(bo->storage);
 	AZ(bo->storage_hint);
 
 	HTTP_Setup(bo->bereq0, bo->ws, bo->vsl, SLT_BereqMethod);
@@ -232,6 +238,7 @@ vbf_stp_retry(struct worker *wrk, struct busyobj *bo)
 	assert(bo->director_state == DIR_S_NULL);
 
 	/* reset other bo attributes - See VBO_GetBusyObj */
+	bo->storage = NULL;
 	bo->storage_hint = NULL;
 	bo->do_esi = 0;
 	bo->do_stream = 1;
@@ -260,8 +267,11 @@ vbf_stp_startfetch(struct worker *wrk, struct busyobj *bo)
 	CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
 	CHECK_OBJ_NOTNULL(bo, BUSYOBJ_MAGIC);
 
+	AZ(bo->storage);
 	AZ(bo->storage_hint);
 
+	bo->storage = STV_next();
+
 	if (bo->retries > 0)
 		http_Unset(bo->bereq, "\012X-Varnish:");
 
diff --git a/bin/varnishd/cache/cache_priv.h b/bin/varnishd/cache/cache_priv.h
index 415822c..460d01e 100644
--- a/bin/varnishd/cache/cache_priv.h
+++ b/bin/varnishd/cache/cache_priv.h
@@ -127,6 +127,8 @@ void V1P_Init(void);
 /* stevedore.c */
 void STV_open(void);
 void STV_close(void);
+const struct stevedore *STV_find(const char *);
+const struct stevedore *STV_next(void);
 int STV_BanInfoDrop(const uint8_t *ban, unsigned len);
 int STV_BanInfoNew(const uint8_t *ban, unsigned len);
 void STV_BanExport(const uint8_t *banlist, unsigned len);
diff --git a/bin/varnishd/cache/cache_req_body.c b/bin/varnishd/cache/cache_req_body.c
index 2858e0f..891cd4c 100644
--- a/bin/varnishd/cache/cache_req_body.c
+++ b/bin/varnishd/cache/cache_req_body.c
@@ -36,6 +36,7 @@
 #include "cache_filter.h"
 #include "vtim.h"
 #include "hash/hash_slinger.h"
+#include "storage/storage.h"
 
 /*----------------------------------------------------------------------
  * Pull the req.body in via/into a objcore
@@ -60,7 +61,7 @@ vrb_pull(struct req *req, ssize_t maxsize, objiterate_f *func, void *priv)
 
 	req->body_oc = HSH_Private(req->wrk);
 	AN(req->body_oc);
-	XXXAN(STV_NewObject(req->wrk, req->body_oc, TRANSIENT_STORAGE, 8));
+	XXXAN(STV_NewObject(req->wrk, req->body_oc, stv_transient, 8));
 
 	vfc->oc = req->body_oc;
 
diff --git a/bin/varnishd/cache/cache_req_fsm.c b/bin/varnishd/cache/cache_req_fsm.c
index bab15ec..e2eb434 100644
--- a/bin/varnishd/cache/cache_req_fsm.c
+++ b/bin/varnishd/cache/cache_req_fsm.c
@@ -44,6 +44,7 @@
 #include "cache_transport.h"
 
 #include "hash/hash_slinger.h"
+#include "storage/storage.h"
 #include "vcl.h"
 #include "vsha256.h"
 #include "vtim.h"
@@ -194,7 +195,7 @@ cnt_synth(struct worker *wrk, struct req *req)
 	req->objcore = HSH_Private(wrk);
 	CHECK_OBJ_NOTNULL(req->objcore, OBJCORE_MAGIC);
 	szl = -1;
-	if (STV_NewObject(wrk, req->objcore, TRANSIENT_STORAGE, 1024)) {
+	if (STV_NewObject(wrk, req->objcore, stv_transient, 1024)) {
 		szl = VSB_len(synth_body);
 		assert(szl >= 0);
 		sz = szl;
diff --git a/bin/varnishd/cache/cache_vrt_var.c b/bin/varnishd/cache/cache_vrt_var.c
index d7e3be4..7fe8d5a 100644
--- a/bin/varnishd/cache/cache_vrt_var.c
+++ b/bin/varnishd/cache/cache_vrt_var.c
@@ -333,6 +333,7 @@ VRT_r_beresp_storage_hint(VRT_CTX)
 void
 VRT_l_beresp_storage_hint(VRT_CTX, const char *str, ...)
 {
+	const struct stevedore *stv;
 	va_list ap;
 	const char *b;
 
@@ -347,6 +348,27 @@ VRT_l_beresp_storage_hint(VRT_CTX, const char *str, ...)
 		return;
 	}
 	ctx->bo->storage_hint = b;
+	stv = STV_find(b);
+	if (stv != NULL)
+		ctx->bo->storage = stv;
+}
+
+/*--------------------------------------------------------------------*/
+
+VCL_STEVEDORE
+VRT_r_beresp_storage(VRT_CTX)
+{
+	CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC);
+	CHECK_OBJ_NOTNULL(ctx->bo, BUSYOBJ_MAGIC);
+	return (ctx->bo->storage);
+}
+
+void
+VRT_l_beresp_storage(VRT_CTX, VCL_STEVEDORE stv)
+{
+	CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC);
+	CHECK_OBJ_NOTNULL(ctx->bo, BUSYOBJ_MAGIC);
+	ctx->bo->storage = stv;
 }
 
 /*--------------------------------------------------------------------*/
diff --git a/bin/varnishd/storage/stevedore.c b/bin/varnishd/storage/stevedore.c
index f977cdd..072ba26 100644
--- a/bin/varnishd/storage/stevedore.c
+++ b/bin/varnishd/storage/stevedore.c
@@ -49,24 +49,10 @@ static const struct stevedore * volatile stv_next;
  * XXX: trust pointer writes to be atomic
  */
 
-static struct stevedore *
-stv_pick_stevedore(struct vsl_log *vsl, const char **hint)
+const struct stevedore *
+STV_next()
 {
 	struct stevedore *stv;
-
-	AN(hint);
-	if (*hint != NULL && **hint != '\0') {
-		VTAILQ_FOREACH(stv, &stv_stevedores, list) {
-			if (!strcmp(stv->ident, *hint))
-				return (stv);
-		}
-		if (!strcmp(TRANSIENT_STORAGE, *hint))
-			return (stv_transient);
-
-		/* Hint was not valid, nuke it */
-		VSLb(vsl, SLT_Debug, "Storage hint not usable");
-		*hint = NULL;
-	}
 	if (stv_next == NULL)
 		return (stv_transient);
 	/* pick a stevedore and bump the head along */
@@ -87,31 +73,15 @@ stv_pick_stevedore(struct vsl_log *vsl, const char **hint)
 
 int
 STV_NewObject(struct worker *wrk, struct objcore *oc,
-    const char *hint, unsigned wsl)
+    const struct stevedore *stv, unsigned wsl)
 {
-	struct stevedore *stv, *stv0;
-	int j;
-
 	CHECK_OBJ_NOTNULL(oc, OBJCORE_MAGIC);
 	CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
+	CHECK_OBJ_NOTNULL(stv, STEVEDORE_MAGIC);
 	assert(wsl > 0);
 
-	stv = stv0 = stv_pick_stevedore(wrk->vsl, &hint);
 	AN(stv->allocobj);
-	j = stv->allocobj(wrk, stv, oc, wsl, 0);
-	if (j == 0 && hint == NULL) {
-		do {
-			stv = stv_pick_stevedore(wrk->vsl, &hint);
-			AN(stv->allocobj);
-			j = stv->allocobj(wrk, stv, oc, wsl, 0);
-		} while (j == 0 && stv != stv0);
-	}
-	if (j == 0 && cache_param->nuke_limit > 0) {
-		/* no luck; try to free some space and keep trying */
-		j = stv->allocobj(wrk, stv, oc, wsl, cache_param->nuke_limit);
-	}
-
-	if (j == 0)
+	if (stv->allocobj(wrk, stv, oc, wsl, cache_param->nuke_limit) == 0)
 		return (0);
 
 	wrk->stats->n_object++;
@@ -216,8 +186,8 @@ STV_BanExport(const uint8_t *bans, unsigned len)
  * VRT functions for stevedores
  */
 
-static const struct stevedore *
-stv_find(const char *nm)
+const struct stevedore *
+STV_find(const char *nm)
 {
 	const struct stevedore *stv;
 
@@ -233,7 +203,7 @@ int
 VRT_Stv(const char *nm)
 {
 
-	if (stv_find(nm) != NULL)
+	if (STV_find(nm) != NULL)
 		return (1);
 	return (0);
 }
@@ -250,7 +220,7 @@ VRT_STEVEDORE_string(VCL_STEVEDORE s)
 VCL_STEVEDORE
 VRT_stevedore(const char *nm)
 {
-	return (stv_find(nm));
+	return (STV_find(nm));
 }
 
 #define VRTSTVVAR(nm, vtype, ctype, dval)	\
@@ -259,7 +229,7 @@ VRT_Stv_##nm(const char *nm)			\
 {						\
 	const struct stevedore *stv;		\
 						\
-	stv = stv_find(nm);			\
+	stv = STV_find(nm);			\
 	if (stv == NULL)			\
 		return (dval);			\
 	if (stv->var_##nm == NULL)		\
diff --git a/bin/varnishtest/tests/c00044.vtc b/bin/varnishtest/tests/c00044.vtc
index 1519ff5..46bd2b3 100644
--- a/bin/varnishtest/tests/c00044.vtc
+++ b/bin/varnishtest/tests/c00044.vtc
@@ -24,7 +24,6 @@ varnish v1 \
 	-vcl+backend {
 	sub vcl_backend_response {
 		set beresp.do_stream = false;
-		set beresp.storage_hint = "invalid";
 		# Unset Date header to not change the object sizes
 		unset beresp.http.Date;
 	}
diff --git a/bin/varnishtest/tests/c00045.vtc b/bin/varnishtest/tests/c00045.vtc
index 022e904..4d3850f 100644
--- a/bin/varnishtest/tests/c00045.vtc
+++ b/bin/varnishtest/tests/c00045.vtc
@@ -1,4 +1,4 @@
-varnishtest	"Object/LRU/Stevedores with hinting"
+varnishtest	"Object/LRU/Stevedores with storage set"
 
 server s1 {
 	rxreq
@@ -16,7 +16,7 @@ varnish v1 \
 	-vcl+backend {
 	sub vcl_backend_response {
 		set beresp.do_stream = false;
-		set beresp.storage_hint = "s0";
+		set beresp.storage = storage.s0;
 		# Unset Date header to not change the object sizes
 		unset beresp.http.Date;
 	}
diff --git a/bin/varnishtest/tests/c00046.vtc b/bin/varnishtest/tests/c00046.vtc
index 52872d7..87963dc 100644
--- a/bin/varnishtest/tests/c00046.vtc
+++ b/bin/varnishtest/tests/c00046.vtc
@@ -1,4 +1,4 @@
-varnishtest	"Object/LRU/Stevedores with hinting and body alloc failures"
+varnishtest	"Object/LRU/Stevedores with storage set and body alloc failures"
 
 server s1 {
 	rxreq
@@ -11,7 +11,7 @@ varnish v1 \
 	-arg "-smalloc,1m" \
 	-vcl+backend {
 	sub vcl_backend_response {
-		set beresp.storage_hint = "s0";
+		set beresp.storage = storage.s0;
 	}
 } -start
 
diff --git a/bin/varnishtest/tests/c00078.vtc b/bin/varnishtest/tests/c00078.vtc
new file mode 100644
index 0000000..e7047ff
--- /dev/null
+++ b/bin/varnishtest/tests/c00078.vtc
@@ -0,0 +1,42 @@
+varnishtest "Stevedores RR, beresp.storage and beresp.storage_hint"
+
+server s1 -repeat 6 {
+	rxreq
+	txresp
+} -start
+
+varnish v1 -arg "-smalloc,1m" -arg "-smalloc,1m" \
+    -arg "-smalloc,1m" -vcl+backend {
+	import debug;
+	sub vcl_backend_response {
+		if (bereq.url == "/1") {
+			set beresp.storage_hint = "invalid";
+		} else if (bereq.url == "/2") {
+			set beresp.storage_hint = "s1";
+		} else if (bereq.url == "/6") {
+			set beresp.storage = debug.no_stevedore();
+		}
+		set beresp.http.storage = beresp.storage;
+	}
+} -start
+
+client c1 {
+	txreq -url /1
+	rxresp
+	expect resp.http.storage == "storage.s1"
+	txreq -url /2
+	rxresp
+	expect resp.http.storage == "storage.s1"
+	txreq -url /3
+	rxresp
+	expect resp.http.storage == "storage.s0"
+	txreq -url /4
+	rxresp
+	expect resp.http.storage == "storage.s1"
+	txreq -url /5
+	rxresp
+	expect resp.http.storage == "storage.s2"
+	txreq -url /6
+	rxresp
+	expect resp.http.storage == <undef>
+} -run
diff --git a/bin/varnishtest/tests/p00008.vtc b/bin/varnishtest/tests/p00008.vtc
index c7a144d..fb55acb 100644
--- a/bin/varnishtest/tests/p00008.vtc
+++ b/bin/varnishtest/tests/p00008.vtc
@@ -20,9 +20,9 @@ varnish v1 \
 	-arg "-sper2=deprecated_persistent,${tmpdir}/_.per2,10m" \
 	-vcl+backend {
 		sub vcl_backend_response {
-			set beresp.storage_hint = "per1";
+			set beresp.storage = storage.per1;
 			if (bereq.url ~ "silo2") {
-				set beresp.storage_hint = "per2";
+				set beresp.storage = storage.per2;
 			}
 		}
 	} -start
diff --git a/bin/varnishtest/tests/r00962.vtc b/bin/varnishtest/tests/r00962.vtc
index 2b6d03e..9b280ec 100644
--- a/bin/varnishtest/tests/r00962.vtc
+++ b/bin/varnishtest/tests/r00962.vtc
@@ -16,7 +16,7 @@ varnish v1 \
 	-arg "-sdeprecated_persistent,${tmpdir}/_.per2,10m" \
 	-vcl+backend {
 	sub vcl_backend_response {
-		set beresp.storage_hint = "s0";
+		set beresp.storage = storage.s0;
 	}
 } -start
 
diff --git a/bin/varnishtest/tests/r01175.vtc b/bin/varnishtest/tests/r01175.vtc
index 5624c20..42a87a3 100644
--- a/bin/varnishtest/tests/r01175.vtc
+++ b/bin/varnishtest/tests/r01175.vtc
@@ -7,7 +7,7 @@ server s1 {
 
 varnish v1 -arg "-s test=malloc,1M" -vcl+backend {
 	sub vcl_backend_response {
-		set beresp.storage_hint = "test";
+		set beresp.storage = storage.test;
 		set beresp.do_stream = false;
 	}
 } -start
diff --git a/bin/varnishtest/tests/r01284.vtc b/bin/varnishtest/tests/r01284.vtc
index 7c0502d..dd8c65f 100644
--- a/bin/varnishtest/tests/r01284.vtc
+++ b/bin/varnishtest/tests/r01284.vtc
@@ -15,7 +15,7 @@ varnish v1 \
 	-vcl+backend {
 	sub vcl_backend_response {
 		set beresp.do_stream = false;
-		set beresp.storage_hint = "Transient";
+		set beresp.storage = storage.Transient;
 		# Unset Date header to not change the object sizes
 		unset beresp.http.Date;
 	}
diff --git a/bin/varnishtest/tests/v00020.vtc b/bin/varnishtest/tests/v00020.vtc
index 1fa3891..513f56c 100644
--- a/bin/varnishtest/tests/v00020.vtc
+++ b/bin/varnishtest/tests/v00020.vtc
@@ -300,3 +300,11 @@ varnish v1 -errvcl {INT * BLOB not possible.} {
 		set resp.status = 100 * debug.str2blob("a");
 	}
 }
+
+# XXX: should spot nonexistent storage
+varnish v1 -errvcl {Symbol not found: 'storage.foo' (expected type STEVEDORE):} {
+	backend b { .host = "127.0.0.1"; }
+	sub vcl_backend_response {
+		set beresp.storage = storage.foo;
+	}
+}
diff --git a/bin/varnishtest/tests/v00025.vtc b/bin/varnishtest/tests/v00025.vtc
index 62c51e6..3e1acd3 100644
--- a/bin/varnishtest/tests/v00025.vtc
+++ b/bin/varnishtest/tests/v00025.vtc
@@ -42,6 +42,7 @@ varnish v1 -arg "-i J.F.Nobody" -vcl+backend {
 		set beresp.http.bereq_backend = bereq.backend;
 		set beresp.http.beresp_backend = beresp.backend;
 		set beresp.http.keep = beresp.keep;
+		set beresp.http.stv = beresp.storage;
 		set beresp.http.hint = beresp.storage_hint;
 		set beresp.http.be_ip = beresp.backend.ip;
 		set beresp.http.be_nm = beresp.backend.name;
diff --git a/bin/varnishtest/tests/v00033.vtc b/bin/varnishtest/tests/v00033.vtc
index ca51ece..b370dd7 100644
--- a/bin/varnishtest/tests/v00033.vtc
+++ b/bin/varnishtest/tests/v00033.vtc
@@ -15,7 +15,7 @@ varnish v1 -vcl+backend {
 		    storage.Transient.used_space +
 		    1 B + 1 KB + 1 MB + 1GB + 1TB;
 		if (bereq.url == "/foo") {
-			set beresp.storage_hint = "Transient";
+			set beresp.storage = storage.Transient;
 		}
 	}
 	sub vcl_deliver {
diff --git a/lib/libvcc/generate.py b/lib/libvcc/generate.py
index 92bed74..6c79950 100755
--- a/lib/libvcc/generate.py
+++ b/lib/libvcc/generate.py
@@ -620,12 +620,20 @@ sp_variables = [
 		IP of the backend this response was fetched from.
 		"""
 	),
+	('beresp.storage',
+		'STEVEDORE',
+		('backend_response', 'backend_error'),
+		('backend_response', 'backend_error'), """
+		The storage backend to use to save this object.
+		"""
+	),
 	('beresp.storage_hint',
 		'STRING',
 		('backend_response', 'backend_error'),
 		('backend_response', 'backend_error'), """
-		Hint to Varnish that you want to save this object to a
-		particular storage backend.
+		Deprecated. Hint to Varnish that you want to
+		save this object to a particular storage backend.
+		Use beresp.storage instead.
 		"""
 	),
 	('obj.proto',

More information about the varnish-commit mailing list