[master] fca5b0e4e Add a VRT_HashStrands32 utility

Dridi Boukelmoune dridi.boukelmoune at gmail.com
Fri Oct 18 15:20:05 UTC 2019


commit fca5b0e4edcd3e56da2b210817e4eb258845c056
Author: Dridi Boukelmoune <dridi.boukelmoune at gmail.com>
Date:   Wed Jul 3 19:18:44 2019 +0200

    Add a VRT_HashStrands32 utility
    
    This de-duplicates logic between the two hashing directors, and in
    the case of vmod-shard adds a missing check for empty components in
    the strands that should be skipped.
    
    This breaks the hashing of the hash director, but unlike the shard
    director it offers no stability and results may vary depending on the
    health of individual clusters. For this reason only d00003.vtc needed
    a bit of reshuffling.

diff --git a/bin/varnishd/cache/cache_vrt.c b/bin/varnishd/cache/cache_vrt.c
index d19e85b90..07587008b 100644
--- a/bin/varnishd/cache/cache_vrt.c
+++ b/bin/varnishd/cache/cache_vrt.c
@@ -37,8 +37,10 @@
 #include "vav.h"
 #include "vcl.h"
 #include "vct.h"
+#include "vend.h"
 #include "vrt_obj.h"
 #include "vsa.h"
+#include "vsha256.h"
 #include "vtcp.h"
 #include "vtim.h"
 #include "vcc_interface.h"
@@ -250,6 +252,35 @@ VRT_Strands2Bool(VCL_STRANDS s)
 	return (0);
 }
 
+/*--------------------------------------------------------------------
+ * Hash a STRANDS
+ */
+
+uint32_t
+VRT_HashStrands32(VCL_STRANDS s)
+{
+	struct VSHA256Context sha_ctx;
+	unsigned char sha256[VSHA256_LEN];
+	const char *p;
+	int i;
+
+	AN(s);
+	VSHA256_Init(&sha_ctx);
+	for (i = 0; i < s->n; i++) {
+		p = s->p[i];
+		if (p != NULL && *p != '\0')
+			VSHA256_Update(&sha_ctx, p, strlen(p));
+	}
+	VSHA256_Final(sha256, &sha_ctx);
+
+	/* NB: for some reason vmod_director's shard director specifically
+	 * relied on little-endian decoding of the last 4 octets. In order
+	 * to maintain a stable hash function to share across consumers we
+	 * need to stick to that.
+	 */
+	return (vle32dec(sha256 + VSHA256_LEN - 4));
+}
+
 /*--------------------------------------------------------------------
  * Collapse a STRING_LIST in the space provided, or return NULL
  */
diff --git a/bin/varnishtest/tests/d00003.vtc b/bin/varnishtest/tests/d00003.vtc
index 7f12096b2..30c73cfb5 100644
--- a/bin/varnishtest/tests/d00003.vtc
+++ b/bin/varnishtest/tests/d00003.vtc
@@ -2,22 +2,22 @@ varnishtest "Test hash director"
 
 server s1 {
 	rxreq
-	txresp -hdr "Foo: 1" -body "1"
+	txresp -hdr "Foo: 2" -body "2"
 	rxreq
 	txresp -hdr "Foo: 3" -body "3"
 	rxreq
-	txresp -hdr "Foo: 9" -body "9"
+	txresp -hdr "Foo: 6" -body "6"
+	rxreq
+	txresp -hdr "Foo: 8" -body "8"
 } -start
 
 server s2 {
 	rxreq
-	txresp -hdr "Foo: 2" -body "2"
+	txresp -hdr "Foo: 1" -body "1"
 	rxreq
 	txresp -hdr "Foo: 4" -body "4"
 	rxreq
-	txresp -hdr "Foo: 6" -body "6"
-	rxreq
-	txresp -hdr "Foo: 8" -body "8"
+	txresp -hdr "Foo: 9" -body "9"
 } -start
 
 varnish v1 -vcl+backend {
@@ -67,15 +67,15 @@ client c1 {
 	rxresp
 	expect resp.http.foo == "4"
 
-	txreq -url /emptystring
+	txreq -url "/emptystring"
 	rxresp
 	expect resp.http.foo == "6"
 
-	txreq -url /nohdr
+	txreq -url "/nohdr"
 	rxresp
 	expect resp.http.foo == "8"
 
-	txreq -url /ip
+	txreq -url "/ip"
 	rxresp
 	expect resp.http.foo == "9"
 } -run
@@ -89,7 +89,7 @@ client c1 {
 
 	txreq
 	rxresp
-	expect resp.http.foo == "2"
+	expect resp.http.foo == "1"
 
 	txreq
 	rxresp
@@ -97,9 +97,5 @@ client c1 {
 
 	txreq
 	rxresp
-	expect resp.http.foo == "6"
-
-	txreq
-	rxresp
-	expect resp.http.foo == "8"
+	expect resp.http.foo == "9"
 } -run
diff --git a/include/vrt.h b/include/vrt.h
index 556425298..6eaa34690 100644
--- a/include/vrt.h
+++ b/include/vrt.h
@@ -53,6 +53,7 @@
  *
  * unreleased (planned for 2020-03-15)
  *	New prefix_{ptr|len} fields in vrt_backend
+ *	VRT_HashStrands32() added
  * 10.0 (2019-09-15)
  *	VRT_UpperLowerStrands added.
  *	VRT_synth_page now takes STRANDS argument
@@ -546,6 +547,7 @@ VCL_STEVEDORE VRT_stevedore(const char *nm);
 
 int VRT_CompareStrands(VCL_STRANDS a, VCL_STRANDS b);
 VCL_BOOL VRT_Strands2Bool(VCL_STRANDS);
+uint32_t VRT_HashStrands32(VCL_STRANDS);
 char *VRT_Strands(char *, size_t, VCL_STRANDS);
 VCL_STRING VRT_StrandsWS(struct ws *, const char *, VCL_STRANDS);
 VCL_STRING VRT_CollectStrands(VRT_CTX, VCL_STRANDS);
diff --git a/lib/libvmod_directors/hash.c b/lib/libvmod_directors/hash.c
index 897c5a567..d1f7fd6b3 100644
--- a/lib/libvmod_directors/hash.c
+++ b/lib/libvmod_directors/hash.c
@@ -33,9 +33,6 @@
 
 #include "cache/cache.h"
 
-#include "vend.h"
-#include "vsha256.h"
-
 #include "vdir.h"
 
 #include "vcc_if.h"
@@ -110,26 +107,15 @@ vmod_hash_remove_backend(VRT_CTX,
 VCL_BACKEND v_matchproto_()
 vmod_hash_backend(VRT_CTX, struct vmod_directors_hash *rr, VCL_STRANDS s)
 {
-	struct VSHA256Context sha_ctx;
-	const char *p;
-	unsigned char sha256[VSHA256_LEN];
 	VCL_BACKEND be;
 	double r;
-	int i;
 
 	CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC);
 	CHECK_OBJ_ORNULL(ctx->bo, BUSYOBJ_MAGIC);
-
 	CHECK_OBJ_NOTNULL(rr, VMOD_DIRECTORS_HASH_MAGIC);
-	VSHA256_Init(&sha_ctx);
-	for (i = 0; i < s->n; i++) {
-		p = s->p[i];
-		if (p != NULL && *p != '\0')
-			VSHA256_Update(&sha_ctx, p, strlen(p));
-	}
-	VSHA256_Final(sha256, &sha_ctx);
-
-	r = vbe32dec(sha256);
+	AN(s);
+
+	r = VRT_HashStrands32(s);
 	r = scalbn(r, -32);
 	assert(r >= 0 && r <= 1.0);
 	be = vdir_pick_be(ctx, rr->vd, r);
diff --git a/lib/libvmod_directors/shard_cfg.c b/lib/libvmod_directors/shard_cfg.c
index 532b0d9e8..cb67915c6 100644
--- a/lib/libvmod_directors/shard_cfg.c
+++ b/lib/libvmod_directors/shard_cfg.c
@@ -268,7 +268,7 @@ shardcfg_hashcircle(struct sharddir *shardd, VCL_INT replicas)
 			ssp[1] = s;
 			ss->p = ssp;
 			shardd->hashcircle[i * replicas + j].point =
-				sharddir_sha256(ss);
+			    VRT_HashStrands32(ss);
 			shardd->hashcircle[i * replicas + j].host = i;
 		}
 		/* not used in current interface */
diff --git a/lib/libvmod_directors/shard_dir.c b/lib/libvmod_directors/shard_dir.c
index db2c5be78..40472e677 100644
--- a/lib/libvmod_directors/shard_dir.c
+++ b/lib/libvmod_directors/shard_dir.c
@@ -40,8 +40,6 @@
 
 #include "vbm.h"
 #include "vrnd.h"
-#include "vsha256.h"
-#include "vend.h"
 
 #include "vcc_if.h"
 #include "shard_dir.h"
@@ -89,25 +87,6 @@ sharddir_err(VRT_CTX, enum VSL_tag_e tag,  const char *fmt, ...)
 	va_end(ap);
 }
 
-uint32_t
-sharddir_sha256(VCL_STRANDS s)
-{
-	struct VSHA256Context sha256;
-	unsigned char digest[VSHA256_LEN];
-	int i;
-
-	AN(s);
-	VSHA256_Init(&sha256);
-	for (i = 0; i < s->n; i++) {
-		if (s->p[i] != NULL)
-			VSHA256_Update(&sha256, s->p[i], strlen(s->p[i]));
-	}
-	VSHA256_Final(digest, &sha256);
-
-	/* The low 32 bits are as good as any. */
-	return (vle32dec(&digest[VSHA256_LEN - 4]));
-}
-
 static int
 shard_lookup(const struct sharddir *shardd, const uint32_t key)
 {
diff --git a/lib/libvmod_directors/shard_dir.h b/lib/libvmod_directors/shard_dir.h
index 6d70a7603..1888aaf4b 100644
--- a/lib/libvmod_directors/shard_dir.h
+++ b/lib/libvmod_directors/shard_dir.h
@@ -98,7 +98,6 @@ sharddir_backend(const struct sharddir *shardd, int id)
 
 void sharddir_debug(struct sharddir *shardd, const uint32_t flags);
 void sharddir_err(VRT_CTX, enum VSL_tag_e tag,  const char *fmt, ...);
-uint32_t sharddir_sha256(VCL_STRANDS s);
 void sharddir_new(struct sharddir **sharddp, const char *vcl_name,
     const struct vmod_directors_shard_param *param);
 void sharddir_set_param(struct sharddir *shardd,
diff --git a/lib/libvmod_directors/vmod_shard.c b/lib/libvmod_directors/vmod_shard.c
index a5e07e14e..09a547faa 100644
--- a/lib/libvmod_directors/vmod_shard.c
+++ b/lib/libvmod_directors/vmod_shard.c
@@ -252,7 +252,7 @@ vmod_shard_key(VRT_CTX, struct vmod_directors_shard *vshard, VCL_STRANDS s)
 	(void)ctx;
 	(void)vshard;
 
-	return ((VCL_INT)sharddir_sha256(s));
+	return ((VCL_INT)VRT_HashStrands32(s));
 }
 
 VCL_VOID v_matchproto_(td_directors_set_warmup)
@@ -376,7 +376,7 @@ shard_get_key(VRT_CTX, const struct vmod_directors_shard_param *p)
 		sp[0] = http->hd[HTTP_HDR_URL].b;
 		s->n = 1;
 		s->p = sp;
-		return (sharddir_sha256(s));
+		return (VRT_HashStrands32(s));
 	}
 	WRONG("by enum");
 }


More information about the varnish-commit mailing list