r3872 - trunk/varnish-cache/bin/varnishd

[phk at projects.linpro.no]

Author: phk
Date: 2009-03-04 10:22:47 +0100 (Wed, 04 Mar 2009)
New Revision: 3872

Modified:
   trunk/varnish-cache/bin/varnishd/stevedore.c
Log:
Try quite hard to allocate all storage segments for an object from
the same stevedore as the object was allocated from.

I am not sure why I didn't spot this back when we added multiple
storage ability, but it is surely dumb to have to find the object
structure on one disk and the body of the object on another, or
even several other, disks.

For persistence this changes from a performance issue to problem:
Any object spread over silos will be lost on restart.

Further changes to this stuff is therefore in the pipeline, but I
wanted this fix in a version that can be merged back to 2.0.x.





Modified: trunk/varnish-cache/bin/varnishd/stevedore.c
===================================================================
--- trunk/varnish-cache/bin/varnishd/stevedore.c	2009-03-04 07:38:30 UTC (rev 3871)
+++ trunk/varnish-cache/bin/varnishd/stevedore.c	2009-03-04 09:22:47 UTC (rev 3872)
@@ -46,20 +46,33 @@
 STV_alloc(struct sess *sp, size_t size)
 {
 	struct storage *st;
-	struct stevedore *stv;
+	struct stevedore *stv = NULL;
+	unsigned fail = 0;
 
+	/*
+	 * Always try the stevedore which allocated the object in order to
+	 * not needlessly split an object across multiple stevedores.
+	 */
+	if (sp->obj != NULL) {
+		CHECK_OBJ_NOTNULL(sp->obj, OBJECT_MAGIC);
+		if (sp->obj->objstore != NULL) {
+			stv = sp->obj->objstore->stevedore;
+			CHECK_OBJ_NOTNULL(stv, STEVEDORE_MAGIC);
+		}
+	}
+
 	for (;;) {
+		if (stv == NULL) {
+			/* pick a stevedore and bump the head along */
+			stv = VTAILQ_NEXT(stv_next, list);
+			if (stv == NULL)
+				stv = VTAILQ_FIRST(&stevedores);
+			AN(stv);
+			AN(stv->name);
+			stv_next = stv;
+			fail = 0;
+		}
 
-		/* pick a stevedore and bump the head along */
-		stv = VTAILQ_NEXT(stv_next, list);
-		if (stv == NULL)
-			stv = VTAILQ_FIRST(&stevedores);
-		AN(stv);
-		AN(stv->name);
-
-		 /* XXX: only safe as long as pointer writes are atomic */
-		stv_next = stv;
-
 		/* try to allocate from it */
 		AN(stv->alloc);
 		st = stv->alloc(stv, size);
@@ -69,6 +82,10 @@
 		/* no luck; try to free some space and keep trying */
 		if (EXP_NukeOne(sp) == -1)
 			break;
+
+		/* Enough is enough: try another if we have one */
+		if (++fail == 50)
+			stv = NULL;
 	}
 	CHECK_OBJ_NOTNULL(st, STORAGE_MAGIC);
 	return (st);