Commit 17e70e53 authored by Poul-Henning Kamp's avatar Poul-Henning Kamp

Make the trouble-list hold a uintptr_t of the objhead, instead of the

actual objhead, to make it painfully obvious, that no dereference is
allowed.

Various strokes with the polishing cloth along the way.



git-svn-id: http://www.varnish-cache.org/svn/trunk@4514 d4fa192b-c00b-0410-8231-f00ffab90ce4
parent 96bdc84c
......@@ -456,7 +456,8 @@ extern pthread_t VCA_thread;
/* cache_backend.c */
struct vbe_conn *VBE_GetFd(const struct director *, struct sess *sp);
int VBE_Healthy(const struct director *, const struct sess *sp);
int VBE_Healthy(double now, const struct director *, uintptr_t target);
int VBE_Healthy_sp(const struct sess *sp, const struct director *);
void VBE_ClosedFd(struct sess *sp);
void VBE_RecycleFd(struct sess *sp);
void VBE_AddHostHeader(const struct sess *sp);
......
......@@ -243,18 +243,18 @@ vbe_NewConn(void)
*/
static unsigned int
vbe_Healthy(const struct sess *sp, struct backend *backend)
vbe_Healthy(double now, uintptr_t target, struct backend *backend)
{
struct trouble *tr;
struct trouble *tr2;
struct trouble *old = NULL;
unsigned i = 0;
struct trouble *old;
unsigned i = 0, retval;
unsigned int threshold;
CHECK_OBJ_NOTNULL(sp, SESS_MAGIC);
CHECK_OBJ_NOTNULL(backend, BACKEND_MAGIC);
if (!backend->healthy)
return 0;
return (0);
/* VRT/VCC sets threshold to UINT_MAX to mark that it's not
* specified by VCL (thus use param).
......@@ -266,43 +266,47 @@ vbe_Healthy(const struct sess *sp, struct backend *backend)
/* Saintmode is disabled */
if (threshold == 0)
return 1;
return (1);
/* No need to test if we don't have an object head to test against.
* FIXME: Should check the magic too, but probably not assert?
*/
if (!sp->objhead)
return 1;
if (target == 0)
return (1);
old = NULL;
retval = 1;
Lck_Lock(&backend->mtx);
VTAILQ_FOREACH_SAFE(tr, &backend->troublelist, list, tr2) {
CHECK_OBJ_NOTNULL(tr, TROUBLE_MAGIC);
if (tr->timeout < sp->t_req) {
if (tr->timeout < now) {
VTAILQ_REMOVE(&backend->troublelist, tr, list);
old = tr;
retval = 1;
break;
}
if (tr->objhead == sp->objhead) {
Lck_Unlock(&backend->mtx);
return 0;
if (tr->target == target) {
retval = 0;
break;
}
/* If the threshold is at 1, a single entry on the list
* will disable the backend. Since 0 is disable, ++i
* instead of i++ to allow this behavior.
*/
if (++i >=threshold) {
Lck_Unlock(&backend->mtx);
return 0;
if (++i >= threshold) {
retval = 0;
break;
}
}
Lck_Unlock(&backend->mtx);
if (old)
if (old != NULL)
FREE_OBJ(old);
return 1;
return (retval);
}
/*--------------------------------------------------------------------
......@@ -342,7 +346,7 @@ vbe_GetVbe(struct sess *sp, struct backend *bp)
VBE_ClosedFd(sp);
}
if (!vbe_Healthy(sp, bp)) {
if (!vbe_Healthy(sp->t_req, (uintptr_t)sp->objhead, bp)) {
VSL_stats->backend_unhealthy++;
return (NULL);
}
......@@ -423,17 +427,28 @@ VBE_GetFd(const struct director *d, struct sess *sp)
return (d->getfd(d, sp));
}
/* Check health ------------------------------------------------------*/
/* Check health ------------------------------------------------------
*
* The target is really an objhead pointer, but since it can not be
* dereferenced during health-checks, we pass it as uintptr_t, which
* hopefully will make people investigate, before mucking about with it.
*/
int
VBE_Healthy(const struct director *d, const struct sess *sp)
VBE_Healthy_sp(const struct sess *sp, const struct director *d)
{
CHECK_OBJ_NOTNULL(sp, SESS_MAGIC);
if (d == NULL)
d = sp->director;
CHECK_OBJ_NOTNULL(d, DIRECTOR_MAGIC);
return (d->healthy(d, sp));
return (d->healthy(sp->t_req, d, (uintptr_t)sp->objhead));
}
int
VBE_Healthy(double now, const struct director *d, uintptr_t target)
{
CHECK_OBJ_NOTNULL(d, DIRECTOR_MAGIC);
return (d->healthy(now, d, target));
}
/*--------------------------------------------------------------------
......@@ -469,14 +484,13 @@ vdi_simple_getfd(const struct director *d, struct sess *sp)
}
static unsigned
vdi_simple_healthy(const struct director *d, const struct sess *sp)
vdi_simple_healthy(double now, const struct director *d, uintptr_t target)
{
struct vdi_simple *vs;
CHECK_OBJ_NOTNULL(sp, SESS_MAGIC);
CHECK_OBJ_NOTNULL(d, DIRECTOR_MAGIC);
CAST_OBJ_NOTNULL(vs, d->priv, VDI_SIMPLE_MAGIC);
return (vbe_Healthy(sp, vs->backend));
return (vbe_Healthy(now, target, vs->backend));
}
/*lint -e{818} not const-able */
......
......@@ -79,7 +79,8 @@ struct vrt_backend_probe;
typedef struct vbe_conn *vdi_getfd_f(const struct director *, struct sess *sp);
typedef void vdi_fini_f(struct director *);
typedef unsigned vdi_healthy(const struct director *, const struct sess *sp);
typedef unsigned vdi_healthy(double now, const struct director *,
uintptr_t target);
struct director {
unsigned magic;
......@@ -99,7 +100,7 @@ struct director {
struct trouble {
unsigned magic;
#define TROUBLE_MAGIC 0x4211ab21
void *objhead; /* NB: only comparison */
uintptr_t target;
double timeout;
VTAILQ_ENTRY(trouble) list;
};
......
......@@ -137,7 +137,7 @@ vdi_random_getfd(const struct director *d, struct sess *sp)
if (r >= s1)
continue;
d2 = vs->hosts[i].backend;
if (!VBE_Healthy(d2, sp))
if (!VBE_Healthy_sp(sp, d2))
break;
vbe = VBE_GetFd(d2, sp);
if (vbe != NULL)
......@@ -152,7 +152,7 @@ vdi_random_getfd(const struct director *d, struct sess *sp)
for (i = 0; i < vs->nhosts; i++) {
d2 = vs->hosts[i].backend;
/* XXX: cache result of healty to avoid double work */
if (VBE_Healthy(d2, sp))
if (VBE_Healthy_sp(sp, d2))
s1 += vs->hosts[i].weight;
}
......@@ -171,7 +171,7 @@ vdi_random_getfd(const struct director *d, struct sess *sp)
s1 = 0.0;
for (i = 0; i < vs->nhosts; i++) {
d2 = vs->hosts[i].backend;
if (!VBE_Healthy(d2, sp))
if (!VBE_Healthy_sp(sp, d2))
continue;
s1 += vs->hosts[i].weight;
if (r >= s1)
......@@ -187,17 +187,16 @@ vdi_random_getfd(const struct director *d, struct sess *sp)
}
static unsigned
vdi_random_healthy(const struct director *d, const struct sess *sp)
vdi_random_healthy(double now, const struct director *d, uintptr_t target)
{
struct vdi_random *vs;
int i;
CHECK_OBJ_NOTNULL(sp, SESS_MAGIC);
CHECK_OBJ_NOTNULL(d, DIRECTOR_MAGIC);
CAST_OBJ_NOTNULL(vs, d->priv, VDI_RANDOM_MAGIC);
for (i = 0; i < vs->nhosts; i++) {
if (VBE_Healthy(vs->hosts[i].backend, sp))
if (VBE_Healthy(now, vs->hosts[i].backend, target))
return 1;
}
return 0;
......
......@@ -75,7 +75,7 @@ vdi_round_robin_getfd(const struct director *d, struct sess *sp)
for (i = 0; i < vs->nhosts; i++) {
backend = vs->hosts[vs->next_host].backend;
vs->next_host = (vs->next_host + 1) % vs->nhosts;
if (!VBE_Healthy(backend, sp))
if (!VBE_Healthy_sp(sp, backend))
continue;
vbe = VBE_GetFd(backend, sp);
if (vbe != NULL)
......@@ -86,19 +86,18 @@ vdi_round_robin_getfd(const struct director *d, struct sess *sp)
}
static unsigned
vdi_round_robin_healthy(const struct director *d, const struct sess *sp)
vdi_round_robin_healthy(double now, const struct director *d, uintptr_t target)
{
struct vdi_round_robin *vs;
struct director *backend;
int i;
CHECK_OBJ_NOTNULL(sp, SESS_MAGIC);
CHECK_OBJ_NOTNULL(d, DIRECTOR_MAGIC);
CAST_OBJ_NOTNULL(vs, d->priv, VDI_ROUND_ROBIN_MAGIC);
for (i = 0; i < vs->nhosts; i++) {
backend = vs->hosts[i].backend;
if (VBE_Healthy(backend, sp))
if (VBE_Healthy(now, backend, target))
return 1;
}
return 0;
......
......@@ -187,7 +187,7 @@ HSH_AddString(const struct sess *sp, const char *str)
WSP(sp, SLT_Hash, "%s", str);
}
/**********************************************************************
/*---------------------------------------------------------------------
* This is a debugging hack to enable testing of boundary conditions
* in the hash algorithm.
* We trap the first 9 different digests and translate them to different
......@@ -394,15 +394,16 @@ HSH_Lookup(struct sess *sp, struct objhead **poh)
* mode'. Is this entirely wrong, or just ugly? Why isn't objhead
* set here? FIXME:Grace.
*/
sp->objhead = oh;
if (oc == NULL && grace_oc != NULL &&
(busy_oc != NULL || !VBE_Healthy(NULL, sp))) {
if (oc == NULL /* We found no live object */
&& grace_oc != NULL /* There is a grace candidate */
&& (busy_oc != NULL /* Somebody else is already busy */
|| !VBE_Healthy(sp->t_req, sp->director, (uintptr_t)oh))) {
/* Or it is impossible to fetch: */
o = grace_oc->obj;
CHECK_OBJ_NOTNULL(o, OBJECT_MAGIC);
if (o->ttl + HSH_Grace(sp->grace) >= sp->t_req)
oc = grace_oc;
}
sp->objhead = NULL;
if (oc != NULL) {
o = oc->obj;
......
......@@ -308,7 +308,7 @@ VRT_l_beresp_saintmode(const struct sess *sp, double a)
ALLOC_OBJ(new, TROUBLE_MAGIC);
AN(new);
new->objhead = sp->objhead;
new->target = (uintptr_t)sp->objhead;
new->timeout = sp->t_req + a;
/* Insert the new item on the list before the first item with a
......@@ -782,7 +782,7 @@ VRT_r_req_backend_healthy(const struct sess *sp)
{
CHECK_OBJ_NOTNULL(sp, SESS_MAGIC);
CHECK_OBJ_NOTNULL(sp->director, DIRECTOR_MAGIC);
return (VBE_Healthy(NULL, sp));
return (VBE_Healthy_sp(sp, sp->director));
}
/*--------------------------------------------------------------------*/
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment