[master] 8756bac Replace the VCL refcount by a self-desribing list
Dridi Boukelmoune
dridi.boukelmoune at gmail.com
Mon Jan 18 22:55:38 CET 2016
commit 8756bacca6550588797eb1c8a9189306b7be86d8
Author: Dridi Boukelmoune <dridi.boukelmoune at gmail.com>
Date: Mon Dec 7 17:06:36 2015 +0100
Replace the VCL refcount by a self-desribing list
Instead of counting the references, the VCL keeps track of them with up
to 31 characters in the description. It is the VMOD's responsibility to
keep track of the opaque struct vclref * and provide a meaningful user-
friendly description.
diff --git a/bin/varnishd/cache/cache_vcl.c b/bin/varnishd/cache/cache_vcl.c
index e10f0c8..694b486 100644
--- a/bin/varnishd/cache/cache_vcl.c
+++ b/bin/varnishd/cache/cache_vcl.c
@@ -62,10 +62,18 @@ struct vcl {
char state[8];
char *loaded_name;
unsigned busy;
- unsigned refcount;
unsigned discard;
const char *temp;
VTAILQ_HEAD(,backend) backend_list;
+ VTAILQ_HEAD(,vclref) ref_list;
+};
+
+struct vclref {
+ unsigned magic;
+#define VCLREF_MAGIC 0x47fb6848
+ const struct vcl *vcl;
+ VTAILQ_ENTRY(vclref) list;
+ char desc[32];
};
/*
@@ -252,7 +260,7 @@ vcl_KillBackends(struct vcl *vcl)
CHECK_OBJ_NOTNULL(vcl, VCL_MAGIC);
AZ(vcl->busy);
- AZ(vcl->refcount);
+ assert(VTAILQ_EMPTY(&vcl->ref_list));
while (1) {
be = VTAILQ_FIRST(&vcl->backend_list);
if (be == NULL)
@@ -375,40 +383,59 @@ VRT_count(VRT_CTX, unsigned u)
ctx->vcl->conf->ref[u].line, ctx->vcl->conf->ref[u].pos);
}
-void
-VRT_ref_vcl(VRT_CTX)
+struct vclref *
+VRT_ref_vcl(VRT_CTX, const char *desc)
{
struct vcl *vcl;
+ struct vclref* ref;
ASSERT_CLI();
CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC);
+ AN(desc);
+ AN(*desc);
vcl = ctx->vcl;
CHECK_OBJ_NOTNULL(vcl, VCL_MAGIC);
xxxassert(vcl->temp == VCL_TEMP_WARM);
+ ALLOC_OBJ(ref, VCLREF_MAGIC);
+ AN(ref);
+ ref->vcl = vcl;
+ snprintf(ref->desc, sizeof ref->desc, "%s", desc);
+
Lck_Lock(&vcl_mtx);
- vcl->refcount++;
+ VTAILQ_INSERT_TAIL(&vcl->ref_list, ref, list);
Lck_Unlock(&vcl_mtx);
+
+ return (ref);
}
void
-VRT_rel_vcl(VRT_CTX)
+VRT_rel_vcl(VRT_CTX, struct vclref **refp)
{
struct vcl *vcl;
+ struct vclref *ref;
+
+ AN(refp);
+ ref = *refp;
+ *refp = NULL;
CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC);
+ CHECK_OBJ_NOTNULL(ref, VCLREF_MAGIC);
vcl = ctx->vcl;
CHECK_OBJ_NOTNULL(vcl, VCL_MAGIC);
+ assert(vcl == ref->vcl);
assert(vcl->temp == VCL_TEMP_WARM || vcl->temp == VCL_TEMP_BUSY ||
vcl->temp == VCL_TEMP_COOLING);
Lck_Lock(&vcl_mtx);
- assert(vcl->refcount > 0);
- vcl->refcount--;
+ assert(!VTAILQ_EMPTY(&vcl->ref_list));
+ VTAILQ_REMOVE(&vcl->ref_list, ref, list);
/* No garbage collection here, for the same reasons as in VCL_Rel. */
Lck_Unlock(&vcl_mtx);
+
+ FREE_OBJ(ref);
}
/*--------------------------------------------------------------------*/
@@ -455,6 +482,23 @@ vcl_failsafe_event(VRT_CTX, enum vcl_event_e ev)
WRONG("A VMOD cannot fail USE, COLD or DISCARD events");
}
+static void
+vcl_print_refs(VRT_CTX)
+{
+ struct vcl *vcl;
+ struct vclref *ref;
+
+ CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC);
+ CHECK_OBJ_NOTNULL(ctx->vcl, VCL_MAGIC);
+ AN(ctx->msg);
+ vcl = ctx->vcl;
+ VSB_printf(ctx->msg, "VCL %s is waiting for:", vcl->loaded_name);
+ Lck_Lock(&vcl_mtx);
+ VTAILQ_FOREACH(ref, &ctx->vcl->ref_list, list)
+ VSB_printf(ctx->msg, "\n\t- %s", ref->desc);
+ Lck_Unlock(&vcl_mtx);
+}
+
static int
vcl_set_state(VRT_CTX, const char *state)
{
@@ -477,16 +521,17 @@ vcl_set_state(VRT_CTX, const char *state)
if (vcl->busy == 0 && (vcl->temp == VCL_TEMP_WARM ||
vcl->temp == VCL_TEMP_BUSY)) {
- vcl->temp = vcl->refcount ? VCL_TEMP_COOLING :
- VCL_TEMP_COLD;
+ vcl->temp = VTAILQ_EMPTY(&vcl->ref_list) ?
+ VCL_TEMP_COLD : VCL_TEMP_COOLING;
vcl_failsafe_event(ctx, VCL_EVENT_COLD);
vcl_BackendEvent(vcl, VCL_EVENT_COLD);
}
else if (vcl->busy)
vcl->temp = VCL_TEMP_BUSY;
+ else if (VTAILQ_EMPTY(&vcl->ref_list))
+ vcl->temp = VCL_TEMP_COLD;
else
- vcl->temp = vcl->refcount ? VCL_TEMP_COOLING :
- VCL_TEMP_COLD;
+ vcl->temp = VCL_TEMP_COOLING;
break;
case '1':
assert(vcl->temp != VCL_TEMP_WARM);
@@ -494,7 +539,11 @@ vcl_set_state(VRT_CTX, const char *state)
if (vcl->temp == VCL_TEMP_BUSY)
vcl->temp = VCL_TEMP_WARM;
/* The VCL must first reach a stable cold state */
- else if (vcl->temp != VCL_TEMP_COOLING) {
+ else if (vcl->temp == VCL_TEMP_COOLING) {
+ vcl_print_refs(ctx);
+ i = -1;
+ }
+ else {
vcl->temp = VCL_TEMP_WARM;
i = vcl_setup_event(ctx, VCL_EVENT_WARM);
if (i == 0)
@@ -558,6 +607,7 @@ VCL_Load(struct cli *cli, const char *name, const char *fn, const char *state)
vcl->loaded_name = strdup(name);
XXXAN(vcl->loaded_name);
VTAILQ_INIT(&vcl->backend_list);
+ VTAILQ_INIT(&vcl->ref_list);
vcl->temp = VCL_TEMP_INIT;
@@ -609,7 +659,7 @@ VCL_Nuke(struct vcl *vcl)
assert(vcl != vcl_active);
assert(vcl->discard);
AZ(vcl->busy);
- AZ(vcl->refcount);
+ assert(VTAILQ_EMPTY(&vcl->ref_list));
VTAILQ_REMOVE(&vcl_head, vcl, list);
ctx.method = VCL_MET_FINI;
ctx.handling = &hand;
@@ -726,7 +776,7 @@ ccf_config_discard(struct cli *cli, const char * const *av, void *priv)
vcl->discard = 1;
Lck_Unlock(&vcl_mtx);
- if (vcl->busy == 0 && vcl->refcount == 0)
+ if (vcl->temp == VCL_TEMP_COLD)
VCL_Nuke(vcl);
}
diff --git a/bin/varnishtest/tests/v00045.vtc b/bin/varnishtest/tests/v00045.vtc
index 1cc1259..936dec9 100644
--- a/bin/varnishtest/tests/v00045.vtc
+++ b/bin/varnishtest/tests/v00045.vtc
@@ -6,7 +6,7 @@ server s1 -start
varnish v1 -vcl+backend {
import ${vmod_debug};
sub vcl_init {
- debug.vcl_release_delay(2s);
+ debug.vcl_release_delay(3s);
}
} -start
@@ -21,9 +21,19 @@ shell {
grep "auto/cooling.*vcl1" >/dev/null
}
+# It can't be warmed up yet
+delay 1
+shell {
+ ${varnishadm} -n ${v1_name} vcl.state vcl1 warm 2>/dev/null |
+ grep "vmod-debug ref on vcl1" >/dev/null
+}
+
# It will eventually cool down
delay 2
shell {
${varnishadm} -n ${v1_name} vcl.list |
grep "auto/cold.*vcl1" >/dev/null
}
+
+# At this point it becomes possible to warm up again
+varnish v1 -cliok "vcl.state vcl1 warm"
diff --git a/doc/sphinx/reference/vmod.rst b/doc/sphinx/reference/vmod.rst
index c6eb274..a5cd8ce 100644
--- a/doc/sphinx/reference/vmod.rst
+++ b/doc/sphinx/reference/vmod.rst
@@ -397,6 +397,28 @@ reference by calling ``VRT_ref_vcl`` when you receive a ``VCL_EVENT_WARM`` and
later calling ``VRT_rel_vcl`` once the background job is over. Receiving a
``VCL_EVENT_COLD`` is your cue to terminate any background job bound to a VCL.
+You can find an example of VCL references in vmod-debug::
+
+ priv_vcl->vclref = VRT_ref_vcl(ctx, "vmod-debug");
+ ...
+ VRT_rel_vcl(&ctx, &priv_vcl->vclref);
+
+In this simplified version, you can see that you need at least a VCL-bound data
+structure like a ``PRIV_VCL`` or a VMOD object to keep track of the reference
+and later release it. You also have to provide a description, it will be printed
+to the user if they try to warm up a cooling VCL::
+
+ $ varnishadm vcl.list
+ available auto/cooling 0 vcl1
+ active auto/warm 0 vcl2
+
+ $ varnishadm vcl.state vcl1 warm
+ Command failed with error code 300
+ Failed <vcl.state vcl1 auto>
+ Message:
+ VCL vcl1 is waiting for:
+ - vmod-debug
+
In the case where properly releasing resources may take some time, you can
opt for an asynchronous worker, either by spawning a thread and tracking it, or
by using Varnish's worker pools.
diff --git a/include/vrt.h b/include/vrt.h
index d3113a8..1f19cd4 100644
--- a/include/vrt.h
+++ b/include/vrt.h
@@ -296,8 +296,9 @@ struct vmod_priv {
typedef int vmod_event_f(VRT_CTX, struct vmod_priv *, enum vcl_event_e);
#endif
-void VRT_ref_vcl(VRT_CTX);
-void VRT_rel_vcl(VRT_CTX);
+struct vclref;
+struct vclref * VRT_ref_vcl(VRT_CTX, const char *);
+void VRT_rel_vcl(VRT_CTX, struct vclref **);
void VRT_priv_fini(const struct vmod_priv *p);
struct vmod_priv *VRT_priv_task(VRT_CTX, void *vmod_id);
diff --git a/lib/libvmod_debug/vmod_debug.c b/lib/libvmod_debug/vmod_debug.c
index 9485268..76287df 100644
--- a/lib/libvmod_debug/vmod_debug.c
+++ b/lib/libvmod_debug/vmod_debug.c
@@ -45,6 +45,8 @@ struct priv_vcl {
#define PRIV_VCL_MAGIC 0x8E62FA9D
char *foo;
uintptr_t exp_cb;
+ struct vcl *vcl;
+ struct vclref *vclref;
};
static VCL_DURATION vcl_release_delay = 0.0;
@@ -248,6 +250,8 @@ priv_vcl_free(void *priv)
EXP_Deregister_Callback(&priv_vcl->exp_cb);
VSL(SLT_Debug, 0, "exp_cb: deregistered");
}
+ AZ(priv_vcl->vcl);
+ AZ(priv_vcl->vclref);
FREE_OBJ(priv_vcl);
AZ(priv_vcl);
}
@@ -273,8 +277,10 @@ event_load(VRT_CTX, struct vmod_priv *priv)
}
static int
-event_warm(VRT_CTX)
+event_warm(VRT_CTX, struct vmod_priv *priv)
{
+ struct priv_vcl *priv_vcl;
+ char buf[32];
VSL(SLT_Debug, 0, "%s: VCL_EVENT_WARM", VCL_Name(ctx->vcl));
@@ -284,7 +290,13 @@ event_warm(VRT_CTX)
return (-1);
}
- VRT_ref_vcl(ctx);
+ CAST_OBJ_NOTNULL(priv_vcl, priv->priv, PRIV_VCL_MAGIC);
+ AZ(priv_vcl->vcl);
+ AZ(priv_vcl->vclref);
+
+ snprintf(buf, sizeof buf, "vmod-debug ref on %s", VCL_Name(ctx->vcl));
+ priv_vcl->vcl = ctx->vcl;
+ priv_vcl->vclref = VRT_ref_vcl(ctx, buf);
return (0);
}
@@ -292,29 +304,40 @@ static void*
cooldown_thread(void *priv)
{
struct vrt_ctx ctx;
+ struct priv_vcl *priv_vcl;
+
+ CAST_OBJ_NOTNULL(priv_vcl, priv, PRIV_VCL_MAGIC);
+ AN(priv_vcl->vcl);
+ AN(priv_vcl->vclref);
- AN(priv);
INIT_OBJ(&ctx, VRT_CTX_MAGIC);
- ctx.vcl = (struct vcl*)priv;
+ ctx.vcl = priv_vcl->vcl;
VTIM_sleep(vcl_release_delay);
- VRT_rel_vcl(&ctx);
+ VRT_rel_vcl(&ctx, &priv_vcl->vclref);
+ priv_vcl->vcl = NULL;
return (NULL);
}
static int
-event_cold(VRT_CTX)
+event_cold(VRT_CTX, struct vmod_priv *priv)
{
pthread_t thread;
+ struct priv_vcl *priv_vcl;
+
+ CAST_OBJ_NOTNULL(priv_vcl, priv->priv, PRIV_VCL_MAGIC);
+ AN(priv_vcl->vcl);
+ AN(priv_vcl->vclref);
VSL(SLT_Debug, 0, "%s: VCL_EVENT_COLD", VCL_Name(ctx->vcl));
if (vcl_release_delay == 0.0) {
- VRT_rel_vcl(ctx);
+ VRT_rel_vcl(ctx, &priv_vcl->vclref);
+ priv_vcl->vcl = NULL;
return (0);
}
- AZ(pthread_create(&thread, NULL, cooldown_thread, ctx->vcl));
+ AZ(pthread_create(&thread, NULL, cooldown_thread, priv_vcl));
AZ(pthread_detach(thread));
return (0);
}
@@ -325,8 +348,8 @@ event_function(VRT_CTX, struct vmod_priv *priv, enum vcl_event_e e)
switch (e) {
case VCL_EVENT_LOAD: return event_load(ctx, priv);
- case VCL_EVENT_COLD: return event_cold(ctx);
- case VCL_EVENT_WARM: return event_warm(ctx);
+ case VCL_EVENT_WARM: return event_warm(ctx, priv);
+ case VCL_EVENT_COLD: return event_cold(ctx, priv);
default: return (0);
}
}
More information about the varnish-commit
mailing list