Mailing List Archive

Author: phk
Date: 2008-11-05 13:51:51 +0100 (Wed, 05 Nov 2008)
New Revision: 3359

Modified:
trunk/varnish-cache/bin/varnishd/cache_dir_random.c
Log:
Fix the broken logic in the random directors picking routine.

Drop the consistency check for health-changes, at the cost of a
slight bias[1] for hosts before, or after, then one that changed health,
according to how its health changed.

Count all failures to pick against the retry count.

Fixes #361


Modified: trunk/varnish-cache/bin/varnishd/cache_dir_random.c
===================================================================
--- trunk/varnish-cache/bin/varnishd/cache_dir_random.c 2008-11-02 21:39:28 UTC (rev 3358)
+++ trunk/varnish-cache/bin/varnishd/cache_dir_random.c 2008-11-05 12:51:51 UTC (rev 3359)
@@ -66,9 +66,9 @@
static struct vbe_conn *
vdi_random_getfd(struct sess *sp)
{
- int i, j, k;
+ int i, k;
struct vdi_random *vs;
- double r, s1, s2;
+ double r, s1;
struct vbe_conn *vbe;

CHECK_OBJ_NOTNULL(sp, SESS_MAGIC);
@@ -78,43 +78,32 @@
k = 0;
for (k = 0; k < vs->retries; ) {

- r = random() / 2147483648.0; /* 2^31 */
- assert(r >= 0.0 && r < 1.0);
-
+ /* Sum up the weights of healty backends */
s1 = 0.0;
- j = 0;
- for (i = 0; i < vs->nhosts; i++) {
- if (!vs->hosts[i].backend->healthy)
- continue;
- s1 += vs->hosts[i].weight;
- j++;
- }
+ for (i = 0; i < vs->nhosts; i++)
+ if (vs->hosts[i].backend->healthy)
+ s1 += vs->hosts[i].weight;

- if (j == 0) /* No healthy hosts */
+ if (s1 == 0.0)
return (NULL);

+ /* Pick a random threshold in that interval */
+ r = random() / 2147483648.0; /* 2^31 */
+ assert(r >= 0.0 && r < 1.0);
r *= s1;

- s2 = 0;
+ s1 = 0.0;
for (i = 0; i < vs->nhosts; i++) {
if (!vs->hosts[i].backend->healthy)
continue;
- s2 += vs->hosts[i].weight;
- if (r < s2)
- break;
+ s1 += vs->hosts[i].weight;
+ if (r >= s1)
+ continue;
+ vbe = VBE_GetVbe(sp, vs->hosts[i].backend);
+ if (vbe != NULL)
+ return (vbe);
+ break;
}
-
- if (s2 != s1) {
- /*
- * Health bit changed in an unusable way while we
- * worked the problem. Usable changes are any that
- * result in the same sum we prepared for.
- */
- continue;
- }
- vbe = VBE_GetVbe(sp, vs->hosts[i].backend);
- if (vbe != NULL)
- return (vbe);
k++;
}
return (NULL);