[PATCH] Re-arrange the random dir for proper retries

Kristian Lyngstol kristian at varnish-software.com
Mon Aug 15 16:15:04 CEST 2011


The rearrangement consists of the following parts:

1. Creating a vdi_random_seed_init() function for appearances.
   vdi_random_seed_init() returns the initial random value to be used to
   pick the first backend, regardless of which of the random-based
   directors we use.

2. Split most of the SHA256-related code out into vdi_random_sha(). This is
   to allow simple reuse during retries.

3. Remove the hash/client-specific attempt that was made before the for()
   loop with retries. Hash/client now always uses the same algorithm as
   random.

4. Move the re-calculation of the random value below the code that picks a
   backend. This is possible because vdi_random_seed_init() has already
   given us the first value, and makes it easy to re-compute the SHA256 for
   client/hash only when a retry is about to happen.

As a side-effect, this means that .retries param now works correctly too.
The .retries-param was only considered for the for()-loop that the random
director used, meaning that .retries = 1 for random meant "only try once"
while it meant "try once, then only retry once after that" for hash/client.

The test-case (which is not moved into tests/* here) demonstrates the
issue.

See #977 for the origin of this.
---
 bin/varnishd/cache_dir_random.c           |  126 +++++++++++++++++------------
 bin/varnishtest/tests.disabled/r00977.vtc |   28 +++++-
 2 files changed, 97 insertions(+), 57 deletions(-)

diff --git a/bin/varnishd/cache_dir_random.c b/bin/varnishd/cache_dir_random.c
index f85d905..2d13cd6 100644
--- a/bin/varnishd/cache_dir_random.c
+++ b/bin/varnishd/cache_dir_random.c
@@ -77,22 +77,36 @@ struct vdi_random {
 	unsigned		nhosts;
 };
 
-static struct vbc *
-vdi_random_getfd(const struct director *d, struct sess *sp)
+/*
+ * Applies sha256 using the given context and input/length, returning a
+ * usable double.
+ */
+static double
+vdi_random_sha(const struct sess *sp, struct SHA256Context *ctx,
+	uint8_t *sign, char *input, ssize_t len)
 {
-	int i, k;
-	struct vdi_random *vs;
-	double r, s1;
-	unsigned u = 0;
-	struct vbc *vbe;
-	struct director *d2;
-	struct SHA256Context ctx;
-	uint8_t sign[SHA256_LEN], *hp = NULL;
-
-	CHECK_OBJ_NOTNULL(sp, SESS_MAGIC);
-	CHECK_OBJ_NOTNULL(d, DIRECTOR_MAGIC);
-	CAST_OBJ_NOTNULL(vs, d->priv, VDI_RANDOM_MAGIC);
+	double u,r;
+	uint8_t *hp = NULL;
+	SHA256_Init(ctx);
+	AN(input);
+	SHA256_Update(ctx, input, len);
+	SHA256_Final(sign, ctx);
+	hp = sign;
+	AN(hp);
+	u = vle32dec(hp);
+	r = u / 4294967296.0;
+	return r;
+}
 
+/*
+ * Sets up the initial seed for picking a backend.
+ *
+ * First tries the canonical backend for that hash/url.
+ */
+static double
+vdi_random_init_seed(struct vdi_random *vs, struct sess *sp,
+		struct SHA256Context *ctx, uint8_t *sign)
+{
 	if (vs->criteria == c_client) {
 		/*
 		 * Hash the client IP# ascii representation, rather than
@@ -101,50 +115,54 @@ vdi_random_getfd(const struct director *d, struct sess *sp)
 		 * We do not hash the port number, to make everybody behind
 		 * a given NAT gateway fetch from the same backend.
 		 */
-		SHA256_Init(&ctx);
-		AN(sp->addr);
+		
 		if (sp->client_identity != NULL)
-			SHA256_Update(&ctx, sp->client_identity,
-			    strlen(sp->client_identity));
+			return vdi_random_sha(sp, ctx, sign,
+				sp->client_identity,
+				strlen(sp->client_identity));	
 		else
-			SHA256_Update(&ctx, sp->addr, strlen(sp->addr));
-		SHA256_Final(sign, &ctx);
-		hp = sign;
+			return vdi_random_sha(sp, ctx, sign, sp->addr,
+				strlen(sp->addr));	
 	}
+
 	if (vs->criteria == c_hash) {
 		/*
 		 * Reuse the hash-string, the objective here is to fetch the
 		 * same object on the same backend all the time
 		 */
-		hp = sp->digest;
+		AN(sp->digest);
+		return vle32dec(sp->digest) / 4294967296.0;
 	}
 
 	/*
-	 * If we are hashing, first try to hit our "canonical backend"
-	 * If that fails, we fall through, and select a weighted backend
-	 * amongst the healthy set.
+	 * Only c_random left
 	 */
-	if (vs->criteria != c_random) {
-		AN(hp);
-		u = vle32dec(hp);
-		r = u / 4294967296.0;
-		assert(r >= 0.0 && r < 1.0);
-		r *= vs->tot_weight;
-		s1 = 0.0;
-		for (i = 0; i < vs->nhosts; i++)  {
-			s1 += vs->hosts[i].weight;
-			if (r >= s1)
-				continue;
-			d2 = vs->hosts[i].backend;
-			if (!VDI_Healthy(d2, sp))
-				break;
-			vbe = VDI_GetFd(d2, sp);
-			if (vbe != NULL)
-				return (vbe);
-			break;
-		}
-	}
+	return random() / 2147483648.0;	/* 2^31 */
+}
+
+static struct vbc *
+vdi_random_getfd(const struct director *d, struct sess *sp)
+{
+	int i, k;
+	struct vdi_random *vs;
+	double r, s1;
+	unsigned u = 0;
+	struct vbc *vbe;
+	struct director *d2;
+	struct SHA256Context ctx;
+	uint8_t sign[SHA256_LEN];
 
+	CHECK_OBJ_NOTNULL(sp, SESS_MAGIC);
+	CHECK_OBJ_NOTNULL(d, DIRECTOR_MAGIC);
+	CAST_OBJ_NOTNULL(vs, d->priv, VDI_RANDOM_MAGIC);
+	
+	r = vdi_random_init_seed(vs, sp, &ctx, sign);
+
+	/*
+	 * Cycle through N retries. For each retry, re-calculate r, either
+	 * randomly or (with hash/client) using the old value ran through
+	 * sha256 again.
+	 */
 	for (k = 0; k < vs->retries; ) {
 		/* Sum up the weights of healty backends */
 		s1 = 0.0;
@@ -158,12 +176,6 @@ vdi_random_getfd(const struct director *d, struct sess *sp)
 		if (s1 == 0.0)
 			return (NULL);
 
-		if (vs->criteria != c_random) {
-			r = u / 4294967296.0;
-		} else {
-			/* Pick a random threshold in that interval */
-			r = random() / 2147483648.0;	/* 2^31 */
-		}
 		assert(r >= 0.0 && r < 1.0);
 		r *= s1;
 
@@ -180,6 +192,16 @@ vdi_random_getfd(const struct director *d, struct sess *sp)
 				return (vbe);
 			break;
 		}
+
+		/*
+		 * Recompute the hash for the next retry.
+		 */
+		if (vs->criteria != c_random) {
+			r = vdi_random_sha(sp, &ctx, sign, (char *)&r,
+				sizeof(r));
+		} else {
+			r = random() / 2147483648.0;	/* 2^31 */
+		}
 		k++;
 	}
 	return (NULL);
diff --git a/bin/varnishtest/tests.disabled/r00977.vtc b/bin/varnishtest/tests.disabled/r00977.vtc
index e6c61da..d15b55d 100644
--- a/bin/varnishtest/tests.disabled/r00977.vtc
+++ b/bin/varnishtest/tests.disabled/r00977.vtc
@@ -1,7 +1,10 @@
 
 varnishtest "Test proper fallbacks of client director"
 
-server s1 -repeat 1 {
+server s1 {
+	rxreq
+	txresp -status 200
+	accept
 	rxreq
 	txresp -status 200
 } -start
@@ -12,17 +15,32 @@ varnish v1 -vcl+backend {
 		{ .backend = { .host = "${bad_ip}"; .port = "9090"; } .weight = 1; }
 		{ .backend = s1; .weight = 1;}
 	}
+	director bar client{
+		.retries = 1;
+		{ .backend = { .host = "${bad_ip}"; .port = "9090"; } .weight = 1; }
+		{ .backend = s1; .weight = 1;}
+	}
 	sub vcl_recv {
-		set req.backend = foo;
-		set client.identity = "44.452";
+		if (req.url ~ "/one") {
+			set req.backend = foo;
+		} else {
+			set req.backend = bar;
+		}
+		# Carefully chosen seed that'll give us bad backend on
+		# first try and good on second.
+		set client.identity = "1.4";
 		return (pass);
 	}
 } -start
 
 client c1 {
-	txreq
+	txreq -url "/one"
 	rxresp
 	expect resp.status == 200
+
+	txreq -url "/two"
+	rxresp
+	expect resp.status == 503
 } -run
 
-varnish v1 -expect backend_fail == 1
+varnish v1 -expect backend_fail == 2
-- 
1.7.4.1

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 490 bytes
Desc: Digital signature
URL: <https://www.varnish-cache.org/lists/pipermail/varnish-dev/attachments/20110815/53e46f77/attachment-0003.pgp>


More information about the varnish-dev mailing list