Hi,
This is hopefully the closure of work I started during VDD15Q3, with a
design refined during VDD15Q4. Some of the post-Q3 patches have
already been pushed to the master branch, and the remaining have been
split and should be easier to review.
If a release manager happens to look at this mail, here is a list of
commits that would need to be back-ported to 4.1 (I can do that if
asked) along with the attached patches:
* 513aee7 Add a reminder for a VRT_MINOR_VERSION bump
* ae5a342 Polish
* 6a6358f Polish
* 89ce06a Wording
* 502a871 Coverage for the cooling state
* 8bf2b0a Hold a VCL reference in the debug vmod
* 28eec51 Make event handling more readable in vmod-debug
* 57a5b9c Introduce a new "busy" VCL temperature
* 2b6bf4f Start documenting that VCL cool down should go fast
* 3162873 Don't create backends on cooling VCLs, return NULL
* 50aeefc Allow VMODs to hold a reference on a warm VCL
* 16b8375 Clarify VRT_delete_backend a bit
* c503bb7 Avoid connection attempts to ${bad_ip}:80
* efd5dfd VMODs handling of VCL_EVENT_COLD must be failsafe
* fbd1c18 Document that VCL_EVENT_USE is deprecated
* 445854c Reserve space for the "cooling" state in vcl.list
I'm not sure when I can join on IRC to discuss those patches, but I
tried to convey as much information as I could in the commit messages.
If the patches are merged, let me know and I'll write changelog
entries.
Best,
Dridi
From c0768e8a5bf68b83b716ab5b1fb19c45d19e32a6 Mon Sep 17 00:00:00 2001
From: Dridi Boukelmoune <[email protected]>
Date: Fri, 4 Dec 2015 09:48:11 +0100
Subject: [PATCH 1/7] Turn VCL state magic numbers into an enum
Make a clear distinction between (struct vclprog).warm that is used as a
boolean, unlike the second parameter of mgt_vcl_setstate.
---
bin/varnishd/mgt/mgt_vcl.c | 39 ++++++++++++++++++++++++---------------
1 file changed, 24 insertions(+), 15 deletions(-)
diff --git a/bin/varnishd/mgt/mgt_vcl.c b/bin/varnishd/mgt/mgt_vcl.c
index 37359f2..673f9e1 100644
--- a/bin/varnishd/mgt/mgt_vcl.c
+++ b/bin/varnishd/mgt/mgt_vcl.c
@@ -48,11 +48,17 @@
#include "mgt_cli.h"
+enum vclstate {
+ VCL_STATE_AUTO,
+ VCL_STATE_COLD,
+ VCL_STATE_WARM,
+};
+
struct vclprog {
VTAILQ_ENTRY(vclprog) list;
char *name;
char *fname;
- int warm;
+ unsigned warm;
char state[8];
double go_cold;
};
@@ -120,21 +126,24 @@ mgt_has_vcl(void)
}
static void
-mgt_vcl_setstate(struct vclprog *vp, int warm)
+mgt_vcl_setstate(struct vclprog *vp, enum vclstate vs)
{
- unsigned status;
+ unsigned status, warm;
double now;
char *p;
- if (warm == -1) {
+ if (vs == VCL_STATE_AUTO) {
assert(vp != active_vcl);
now = VTIM_mono();
- warm = vp->warm;
+ vs = vp->warm ? VCL_STATE_WARM : VCL_STATE_COLD;
if (vp->go_cold > 0 && !strcmp(vp->state, "auto") &&
vp->go_cold + mgt_param.vcl_cooldown < now)
- warm = 0;
+ vs = VCL_STATE_COLD;
}
+ assert(vs != VCL_STATE_AUTO);
+ warm = vs == VCL_STATE_WARM ? 1 : 0;
+
if (vp->warm == warm)
return;
@@ -230,7 +239,7 @@ mgt_push_vcls_and_start(unsigned *status, char **p)
struct vclprog *vp;
AN(active_vcl);
- mgt_vcl_setstate(active_vcl, 1);
+ mgt_vcl_setstate(active_vcl, VCL_STATE_WARM);
VTAILQ_FOREACH(vp, &vclhead, list) {
if (mgt_cli_askchild(status, p, "vcl.load \"%s\" %s %d%s\n",
vp->name, vp->fname, vp->warm, vp->state))
@@ -321,7 +330,7 @@ mcf_vcl_state(struct cli *cli, const char * const *av, void *priv)
bprintf(vp->state, "%s", "auto");
if (vp != active_vcl) {
vp->go_cold = VTIM_mono();
- mgt_vcl_setstate(vp, -1);
+ mgt_vcl_setstate(vp, VCL_STATE_AUTO);
}
} else if (!strcmp(av[3], "cold")) {
if (vp == active_vcl) {
@@ -330,10 +339,10 @@ mcf_vcl_state(struct cli *cli, const char * const *av, void *priv)
return;
}
bprintf(vp->state, "%s", "auto");
- mgt_vcl_setstate(vp, 0);
+ mgt_vcl_setstate(vp, VCL_STATE_COLD);
} else if (!strcmp(av[3], "warm")) {
bprintf(vp->state, "%s", av[3]);
- mgt_vcl_setstate(vp, 1);
+ mgt_vcl_setstate(vp, VCL_STATE_WARM);
} else {
VCLI_Out(cli, "State must be one of auto, cold or warm.");
VCLI_SetResult(cli, CLIS_PARAM);
@@ -353,20 +362,20 @@ mcf_vcl_use(struct cli *cli, const char * const *av, void *priv)
return;
if (vp == active_vcl)
return;
- mgt_vcl_setstate(vp, 1);
+ mgt_vcl_setstate(vp, VCL_STATE_WARM);
if (child_pid >= 0 &&
mgt_cli_askchild(&status, &p, "vcl.use %s\n", av[2])) {
VCLI_SetResult(cli, status);
VCLI_Out(cli, "%s", p);
vp->go_cold = VTIM_mono();
- mgt_vcl_setstate(vp, -1);
+ mgt_vcl_setstate(vp, VCL_STATE_AUTO);
} else {
VCLI_Out(cli, "VCL '%s' now active", av[2]);
vp2 = active_vcl;
active_vcl = vp;
if (vp2 != NULL) {
vp2->go_cold = VTIM_mono();
- mgt_vcl_setstate(vp2, -1);
+ mgt_vcl_setstate(vp2, VCL_STATE_AUTO);
}
}
free(p);
@@ -388,7 +397,7 @@ mcf_vcl_discard(struct cli *cli, const char * const *av, void *priv)
VCLI_Out(cli, "Cannot discard active VCL program\n");
return;
}
- mgt_vcl_setstate(vp, 0);
+ mgt_vcl_setstate(vp, VCL_STATE_COLD);
if (child_pid >= 0) {
/* If this fails the child is crashing, figure that later */
(void)mgt_cli_askchild(&status, &p, "vcl.discard %s\n", av[2]);
@@ -434,7 +443,7 @@ mgt_vcl_poker(const struct vev *e, int what)
e_poker->timeout = mgt_param.vcl_cooldown * .45;
VTAILQ_FOREACH(vp, &vclhead, list) {
if (vp != active_vcl)
- mgt_vcl_setstate(vp, -1);
+ mgt_vcl_setstate(vp, VCL_STATE_AUTO);
}
return (0);
}
--
2.4.3
From dd43757905baedd21b2d5e9b316ab438c6931266 Mon Sep 17 00:00:00 2001
From: Dridi Boukelmoune <[email protected]>
Date: Fri, 4 Dec 2015 11:14:29 +0100
Subject: [PATCH 2/7] Make vcl_set_state accept a ctx instead of a vcl
This paves the way towards failing VCL_EVENT_WARM events in VMODs. If
setting the temperature fails, we may need to carry a message back to
the CLI and other bits of context.
---
bin/varnishd/cache/cache_vcl.c | 43 +++++++++++++++++++++++++-----------------
1 file changed, 26 insertions(+), 17 deletions(-)
diff --git a/bin/varnishd/cache/cache_vcl.c b/bin/varnishd/cache/cache_vcl.c
index 54effd1..d26c9af 100644
--- a/bin/varnishd/cache/cache_vcl.c
+++ b/bin/varnishd/cache/cache_vcl.c
@@ -429,17 +429,16 @@ vcl_find(const char *name)
}
static void
-vcl_set_state(struct vcl *vcl, const char *state)
+vcl_set_state(VRT_CTX, const char *state)
{
- struct vrt_ctx ctx;
- unsigned hand = 0;
+ struct vcl *vcl;
ASSERT_CLI();
- AN(vcl->temp);
+ CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC);
+ AN(ctx->handling);
- INIT_OBJ(&ctx, VRT_CTX_MAGIC);
- ctx.handling = &hand;
- ctx.vcl = vcl;
+ vcl = ctx->vcl;
+ AN(vcl->temp);
switch(state[0]) {
case '0':
@@ -449,7 +448,7 @@ vcl_set_state(struct vcl *vcl, const char *state)
vcl->temp = vcl->refcount ? vcl_temp_cooling :
vcl_temp_cold;
- AZ(vcl->conf->event_vcl(&ctx, VCL_EVENT_COLD));
+ AZ(vcl->conf->event_vcl(ctx, VCL_EVENT_COLD));
vcl_BackendEvent(vcl, VCL_EVENT_COLD);
}
else if (vcl->busy)
@@ -466,7 +465,7 @@ vcl_set_state(struct vcl *vcl, const char *state)
/* The VCL must first reach a stable cold state */
else if (vcl->temp != vcl_temp_cooling) {
vcl->temp = vcl_temp_warm;
- (void)vcl->conf->event_vcl(&ctx, VCL_EVENT_WARM);
+ (void)vcl->conf->event_vcl(ctx, VCL_EVENT_WARM);
vcl_BackendEvent(vcl, VCL_EVENT_WARM);
}
break;
@@ -529,7 +528,7 @@ VCL_Load(struct cli *cli, const char *name, const char *fn, const char *state)
return (1);
}
VSB_delete(vsb);
- vcl_set_state(vcl, state);
+ vcl_set_state(&ctx, state);
bprintf(vcl->state, "%s", state + 1);
assert(hand == VCL_RET_OK);
VCLI_Out(cli, "Loaded \"%s\" as \"%s\"", fn , name);
@@ -577,13 +576,19 @@ VCL_Nuke(struct vcl *vcl)
void
VCL_Poll(void)
{
+ struct vrt_ctx ctx;
struct vcl *vcl, *vcl2;
+ unsigned hand;
ASSERT_CLI();
VTAILQ_FOREACH_SAFE(vcl, &vcl_head, list, vcl2) {
if (vcl->temp == vcl_temp_busy ||
- vcl->temp == vcl_temp_cooling)
- vcl_set_state(vcl, "0");
+ vcl->temp == vcl_temp_cooling) {
+ INIT_OBJ(&ctx, VRT_CTX_MAGIC);
+ ctx.vcl = vcl;
+ ctx.handling = &hand;
+ vcl_set_state(&ctx, "0");
+ }
if (vcl->discard && vcl->temp == vcl_temp_cold)
VCL_Nuke(vcl);
}
@@ -625,17 +630,21 @@ ccf_config_load(struct cli *cli, const char * const *av, void *priv)
static void __match_proto__(cli_func_t)
ccf_config_state(struct cli *cli, const char * const *av, void *priv)
{
- struct vcl *vcl;
+ struct vrt_ctx ctx;
+ unsigned hand;
+
+ INIT_OBJ(&ctx, VRT_CTX_MAGIC);
+ ctx.handling = &hand;
(void)cli;
AZ(priv);
ASSERT_CLI();
AN(av[2]);
AN(av[3]);
- vcl = vcl_find(av[2]);
- AN(vcl); // MGT ensures this
- vcl_set_state(vcl, av[3]);
- bprintf(vcl->state, "%s", av[3] + 1);
+ ctx.vcl = vcl_find(av[2]);
+ AN(ctx.vcl); // MGT ensures this
+ vcl_set_state(&ctx, av[3]);
+ bprintf(ctx.vcl->state, "%s", av[3] + 1);
}
static void __match_proto__(cli_func_t)
--
2.4.3
From eb653618ea324c32b263ad2bf654e59102f4d305 Mon Sep 17 00:00:00 2001
From: Dridi Boukelmoune <[email protected]>
Date: Fri, 4 Dec 2015 11:45:16 +0100
Subject: [PATCH 3/7] Allow VMODs to fail a warm-up
It is also possible to convey a message to the CLI. In case of a failure
the VCL is cooled down. VCL_Load may now fail with either CLIS_PARAM or
CLIS_CANT.
---
bin/varnishd/cache/cache_vcl.c | 81 +++++++++++++++++++++++++++++-----------
bin/varnishtest/tests/v00044.vtc | 4 ++
2 files changed, 63 insertions(+), 22 deletions(-)
diff --git a/bin/varnishd/cache/cache_vcl.c b/bin/varnishd/cache/cache_vcl.c
index d26c9af..0375974 100644
--- a/bin/varnishd/cache/cache_vcl.c
+++ b/bin/varnishd/cache/cache_vcl.c
@@ -428,14 +428,18 @@ vcl_find(const char *name)
return (NULL);
}
-static void
+static int
vcl_set_state(VRT_CTX, const char *state)
{
struct vcl *vcl;
+ int i = 0;
ASSERT_CLI();
CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC);
AN(ctx->handling);
+ AN(ctx->vcl);
+ AN(state);
+ assert(ctx->msg != NULL || *state == '0');
vcl = ctx->vcl;
AN(vcl->temp);
@@ -465,16 +469,37 @@ vcl_set_state(VRT_CTX, const char *state)
/* The VCL must first reach a stable cold state */
else if (vcl->temp != vcl_temp_cooling) {
vcl->temp = vcl_temp_warm;
- (void)vcl->conf->event_vcl(ctx, VCL_EVENT_WARM);
- vcl_BackendEvent(vcl, VCL_EVENT_WARM);
+ i = vcl->conf->event_vcl(ctx, VCL_EVENT_WARM);
+ if (i == 0)
+ vcl_BackendEvent(vcl, VCL_EVENT_WARM);
+ else
+ AZ(vcl->conf->event_vcl(ctx, VCL_EVENT_COLD));
}
break;
default:
WRONG("Wrong enum state");
}
+ return (i);
}
-static int
+static void
+vcl_unload(VRT_CTX, struct cli *cli, const char *name, const char *step)
+{
+ struct vcl *vcl = ctx->vcl;
+
+ AZ(VSB_finish(ctx->msg));
+ VCLI_SetResult(cli, CLIS_CANT);
+ VCLI_Out(cli, "VCL \"%s\" Failed %s", name, step);
+ if (VSB_len(ctx->msg))
+ VCLI_Out(cli, "\nMessage:\n\t%s", VSB_data(ctx->msg));
+ AZ(vcl->conf->event_vcl(ctx, VCL_EVENT_DISCARD));
+ vcl_KillBackends(vcl);
+ VCL_Close(&vcl);
+ VSB_clear(ctx->msg);
+ VSB_delete(ctx->msg);
+}
+
+static void
VCL_Load(struct cli *cli, const char *name, const char *fn, const char *state)
{
struct vcl *vcl;
@@ -487,8 +512,9 @@ VCL_Load(struct cli *cli, const char *name, const char *fn, const char *state)
vcl = vcl_find(name);
if (vcl != NULL) {
+ VCLI_SetResult(cli, CLIS_PARAM);
VCLI_Out(cli, "Config '%s' already loaded", name);
- return (1);
+ return;
}
vsb = VSB_new_auto();
@@ -497,9 +523,10 @@ VCL_Load(struct cli *cli, const char *name, const char *fn, const char *state)
vcl = VCL_Open(fn, vsb);
if (vcl == NULL) {
AZ(VSB_finish(vsb));
+ VCLI_SetResult(cli, CLIS_PARAM);
VCLI_Out(cli, "%s", VSB_data(vsb));
VSB_delete(vsb);
- return (1);
+ return;
}
vcl->loaded_name = strdup(name);
@@ -516,19 +543,20 @@ VCL_Load(struct cli *cli, const char *name, const char *fn, const char *state)
VSB_clear(vsb);
ctx.msg = vsb;
i = vcl->conf->event_vcl(&ctx, VCL_EVENT_LOAD);
- AZ(VSB_finish(vsb));
if (i) {
- VCLI_Out(cli, "VCL \"%s\" Failed initialization", name);
- if (VSB_len(vsb))
- VCLI_Out(cli, "\nMessage:\n\t%s", VSB_data(vsb));
- AZ(vcl->conf->event_vcl(&ctx, VCL_EVENT_DISCARD));
- vcl_KillBackends(vcl);
- VCL_Close(&vcl);
- VSB_delete(vsb);
- return (1);
+ vcl_unload(&ctx, cli, name, "initialization");
+ return;
+ }
+ VSB_clear(vsb);
+ i = vcl_set_state(&ctx, state);
+ if (i) {
+ assert(*state == '1');
+ vcl_unload(&ctx, cli, name, "warmup");
+ return;
}
+ VSB_clear(vsb);
+ VSB_finish(vsb);
VSB_delete(vsb);
- vcl_set_state(&ctx, state);
bprintf(vcl->state, "%s", state + 1);
assert(hand == VCL_RET_OK);
VCLI_Out(cli, "Loaded \"%s\" as \"%s\"", fn , name);
@@ -539,7 +567,6 @@ VCL_Load(struct cli *cli, const char *name, const char *fn, const char *state)
Lck_Unlock(&vcl_mtx);
VSC_C_main->n_vcl++;
VSC_C_main->n_vcl_avail++;
- return (0);
}
/*--------------------------------------------------------------------
@@ -587,7 +614,7 @@ VCL_Poll(void)
INIT_OBJ(&ctx, VRT_CTX_MAGIC);
ctx.vcl = vcl;
ctx.handling = &hand;
- vcl_set_state(&ctx, "0");
+ (void)vcl_set_state(&ctx, "0");
}
if (vcl->discard && vcl->temp == vcl_temp_cold)
VCL_Nuke(vcl);
@@ -623,8 +650,7 @@ ccf_config_load(struct cli *cli, const char * const *av, void *priv)
AZ(priv);
ASSERT_CLI();
- if (VCL_Load(cli, av[2], av[3], av[4]))
- VCLI_SetResult(cli, CLIS_PARAM);
+ VCL_Load(cli, av[2], av[3], av[4]);
}
static void __match_proto__(cli_func_t)
@@ -634,6 +660,7 @@ ccf_config_state(struct cli *cli, const char * const *av, void *priv)
unsigned hand;
INIT_OBJ(&ctx, VRT_CTX_MAGIC);
+ ctx.msg = VSB_new_auto();
ctx.handling = &hand;
(void)cli;
@@ -643,8 +670,18 @@ ccf_config_state(struct cli *cli, const char * const *av, void *priv)
AN(av[3]);
ctx.vcl = vcl_find(av[2]);
AN(ctx.vcl); // MGT ensures this
- vcl_set_state(&ctx, av[3]);
- bprintf(ctx.vcl->state, "%s", av[3] + 1);
+ if (vcl_set_state(&ctx, av[3]) == 0) {
+ bprintf(ctx.vcl->state, "%s", av[3] + 1);
+ VSB_delete(ctx.msg);
+ return;
+ }
+ VSB_finish(ctx.msg);
+ VCLI_SetResult(cli, CLIS_CANT);
+ VCLI_Out(cli, "Failed <vcl.state %s %s>", ctx.vcl->loaded_name,
+ av[3] + 1);
+ if (VSB_len(ctx.msg))
+ VCLI_Out(cli, "\nMessage:\n\t%s", VSB_data(ctx.msg));
+ VSB_delete(ctx.msg);
}
static void __match_proto__(cli_func_t)
diff --git a/bin/varnishtest/tests/v00044.vtc b/bin/varnishtest/tests/v00044.vtc
index aa79e6a..4b5a59a 100644
--- a/bin/varnishtest/tests/v00044.vtc
+++ b/bin/varnishtest/tests/v00044.vtc
@@ -83,3 +83,7 @@ delay .4
varnish v1 -expect VBE.vcl1.default.happy >= 0
delay 4
varnish v1 -expect !VBE.vcl1.default.happy
+
+# A VMOD's warm-up can fail (not quite there yet)
+varnish v1 -cliok "param.set max_esi_depth 42"
+varnish v1 -cliok "vcl.state vcl1 warm"
--
2.4.3
From 07770b0c52cfae53436f182a451474ce5d686034 Mon Sep 17 00:00:00 2001
From: Dridi Boukelmoune <[email protected]>
Date: Mon, 7 Dec 2015 11:18:33 +0100
Subject: [PATCH 4/7] Make VMODs actually fail warm-ups
The implementation is similar to the load/discard dance when a load
fails. New VGC functions are introduced iff the VCL has at least one
VMOD handling events.
The generated code looks like this:
static unsigned vgc_inistep;
static unsigned vgc_warmupstep;
...
static int
VGC_Load(VRT_CTX)
{
...
}
static int
VGC_Discard(VRT_CTX)
{
...
}
static int
VGC_Warmup(VRT_CTX, enum vcl_event_e ev)
{
vgc_warmupstep = 0;
/* 4 */
if (Vmod_debug_Func._event(ctx, &vmod_priv_debug, ev))
return (1);
vgc_warmupstep = 4;
return (0);
}
static int
VGC_Use(VRT_CTX, enum vcl_event_e ev)
{
/* 4 */
if (Vmod_debug_Func._event(ctx, &vmod_priv_debug, ev))
return (1);
return (0);
}
static int
VGC_Cooldown(VRT_CTX, enum vcl_event_e ev)
{
int retval = 0;
/* 4 */
if (vgc_warmupstep >= 4 &&
Vmod_debug_Func._event(ctx, &vmod_priv_debug, ev) != 0)
retval = 1;
return (retval);
}
static int
VGC_Event(VRT_CTX, enum vcl_event_e ev)
{
if (ev == VCL_EVENT_LOAD)
return(VGC_Load(ctx));
if (ev == VCL_EVENT_WARM)
return(VGC_Warmup(ctx, ev));
if (ev == VCL_EVENT_USE)
return(VGC_Use(ctx, ev));
if (ev == VCL_EVENT_COLD)
return(VGC_Cooldown(ctx, ev));
if (ev == VCL_EVENT_DISCARD)
return(VGC_Discard(ctx));
return (1);
}
However, if there are no VMODs handling events, no new functions shall
be generated, leading to code looking like this:
static unsigned vgc_inistep;
static unsigned vgc_warmupstep;
...
static int
VGC_Load(VRT_CTX)
{
...
}
static int
VGC_Discard(VRT_CTX)
{
...
}
static int
VGC_Event(VRT_CTX, enum vcl_event_e ev)
{
if (ev == VCL_EVENT_LOAD)
return(VGC_Load(ctx));
if (ev == VCL_EVENT_DISCARD)
return(VGC_Discard(ctx));
(void)vgc_warmupstep;
return (0);
}
---
doc/sphinx/reference/vmod.rst | 6 +++
lib/libvcc/vcc_compile.c | 96 +++++++++++++++++++++++++++++++++++++------
lib/libvcc/vcc_vmod.c | 3 +-
3 files changed, 90 insertions(+), 15 deletions(-)
diff --git a/doc/sphinx/reference/vmod.rst b/doc/sphinx/reference/vmod.rst
index 57fa09c..12a199d 100644
--- a/doc/sphinx/reference/vmod.rst
+++ b/doc/sphinx/reference/vmod.rst
@@ -383,6 +383,12 @@ first with a ``VCL_EVENT_WARM`` event. Unless a user decides that a given VCL
should always be warm, an inactive VMOD will eventually become cold and should
manage resources accordingly.
+An event function must return zero upon success. It is therefore possible to
+fail an initialization, the ``VCL_EVENT_LOAD`` or ``VCL_EVENT_WARM`` events.
+Should such a failure happen, a ``VCL_EVENT_DISCARD`` or ``VCL_EVENT_COLD``
+event to the VMODs that succeeded. The VMOD that failed will not receive this
+event, and therefore must not be left half-initialized should a failure occur.
+
If your VMOD is running an asynchronous background job you can hold a reference
to the VCL to prevent it from going cold too soon and get the same guarantees
as backends with ongoing requests for instance. For that, you must acquire the
diff --git a/lib/libvcc/vcc_compile.c b/lib/libvcc/vcc_compile.c
index e79597f..2d08d00 100644
--- a/lib/libvcc/vcc_compile.c
+++ b/lib/libvcc/vcc_compile.c
@@ -325,20 +325,22 @@ EmitCoordinates(const struct vcc *tl, struct vsb *vsb)
/*--------------------------------------------------------------------
* Init/Fini/Event
*
- * We call Fini-s in the opposite order of init-s.
- * Other events are called in same order as init-s, no matter which
- * event it might be.
+ * We call DISCARD and COLD events in the opposite order of LOAD and
+ * WARM.
*/
static void
EmitInitFini(const struct vcc *tl)
{
struct inifin *p;
+ unsigned has_event = 0;
- Fh(tl, 0, "\nstatic unsigned vgc_inistep;\n");
+ Fh(tl, 0, "\n");
+ Fh(tl, 0, "static unsigned vgc_inistep;\n");
+ Fh(tl, 0, "static unsigned vgc_warmupstep;\n");
/*
- * INIT
+ * LOAD
*/
Fc(tl, 0, "\nstatic int\nVGC_Load(VRT_CTX)\n{\n\n");
Fc(tl, 0, "\tvgc_inistep = 0;\n\n");
@@ -349,6 +351,10 @@ EmitInitFini(const struct vcc *tl)
Fc(tl, 0, "\t/* %u */\n%s\n", p->n, VSB_data(p->ini));
Fc(tl, 0, "\tvgc_inistep = %u;\n\n", p->n);
VSB_delete(p->ini);
+
+ AZ(VSB_finish(p->event));
+ if (VSB_len(p->event))
+ has_event = 1;
}
Fc(tl, 0, "\t(void)VGC_function_vcl_init(ctx);\n");
@@ -356,7 +362,7 @@ EmitInitFini(const struct vcc *tl)
Fc(tl, 0, "}\n");
/*
- * FINI
+ * DISCARD
*/
Fc(tl, 0, "\nstatic int\nVGC_Discard(VRT_CTX)\n{\n\n");
@@ -375,6 +381,66 @@ EmitInitFini(const struct vcc *tl)
Fc(tl, 0, "\treturn(0);\n");
Fc(tl, 0, "}\n");
+ if (has_event) {
+ /*
+ * WARM
+ */
+ Fc(tl, 0, "\nstatic int\n");
+ Fc(tl, 0, "VGC_Warmup(VRT_CTX, enum vcl_event_e ev)\n{\n\n");
+
+ Fc(tl, 0, "\tvgc_warmupstep = 0;\n\n");
+ VTAILQ_FOREACH(p, &tl->inifin, list) {
+ assert(p->n > 0);
+ if (VSB_len(p->event)) {
+ Fc(tl, 0, "\t/* %u */\n", p->n);
+ Fc(tl, 0, "\tif (%s)\n", VSB_data(p->event));
+ Fc(tl, 0, "\t\treturn (1);\n");
+ Fc(tl, 0, "\tvgc_warmupstep = %u;\n\n", p->n);
+ }
+ }
+
+ Fc(tl, 0, "\treturn (0);\n");
+ Fc(tl, 0, "}\n");
+
+ /*
+ * USE (deprecated)
+ */
+ Fc(tl, 0, "\nstatic int\n");
+ Fc(tl, 0, "VGC_Use(VRT_CTX, enum vcl_event_e ev)\n{\n\n");
+
+ VTAILQ_FOREACH(p, &tl->inifin, list) {
+ assert(p->n > 0);
+ if (VSB_len(p->event)) {
+ Fc(tl, 0, "\t/* %u */\n", p->n);
+ Fc(tl, 0, "\tif (%s)\n", VSB_data(p->event));
+ Fc(tl, 0, "\t\treturn (1);\n\n");
+ }
+ }
+
+ Fc(tl, 0, "\treturn (0);\n");
+ Fc(tl, 0, "}\n");
+
+ /*
+ * COLD
+ */
+ Fc(tl, 0, "\nstatic int\n");
+ Fc(tl, 0, "VGC_Cooldown(VRT_CTX, enum vcl_event_e ev)\n{\n");
+ Fc(tl, 0, "\tint retval = 0;\n\n");
+
+ VTAILQ_FOREACH_REVERSE(p, &tl->inifin, inifinhead, list) {
+ if (VSB_len(p->event)) {
+ Fc(tl, 0, "\t/* %u */\n", p->n);
+ Fc(tl, 0, "\tif (vgc_warmupstep >= %u &&\n", p->n);
+ Fc(tl, 0, "\t %s != 0)\n", VSB_data(p->event));
+ Fc(tl, 0, "\t\tretval = 1;\n\n");
+ }
+ VSB_delete(p->event);
+ }
+
+ Fc(tl, 0, "\treturn (retval);\n");
+ Fc(tl, 0, "}\n");
+ }
+
/*
* EVENTS
*/
@@ -383,16 +449,20 @@ EmitInitFini(const struct vcc *tl)
Fc(tl, 0, "{\n");
Fc(tl, 0, "\tif (ev == VCL_EVENT_LOAD)\n");
Fc(tl, 0, "\t\treturn(VGC_Load(ctx));\n");
+ if (has_event) {
+ Fc(tl, 0, "\tif (ev == VCL_EVENT_WARM)\n");
+ Fc(tl, 0, "\t\treturn(VGC_Warmup(ctx, ev));\n");
+ Fc(tl, 0, "\tif (ev == VCL_EVENT_USE)\n");
+ Fc(tl, 0, "\t\treturn(VGC_Use(ctx, ev));\n");
+ Fc(tl, 0, "\tif (ev == VCL_EVENT_COLD)\n");
+ Fc(tl, 0, "\t\treturn(VGC_Cooldown(ctx, ev));\n");
+ }
Fc(tl, 0, "\tif (ev == VCL_EVENT_DISCARD)\n");
Fc(tl, 0, "\t\treturn(VGC_Discard(ctx));\n");
Fc(tl, 0, "\n");
- VTAILQ_FOREACH(p, &tl->inifin, list) {
- AZ(VSB_finish(p->event));
- if (VSB_len(p->event))
- Fc(tl, 0, "\t/* %u */\n%s\n", p->n, VSB_data(p->event));
- VSB_delete(p->event);
- }
- Fc(tl, 0, "\treturn (0);\n");
+ if (!has_event)
+ Fc(tl, 0, "\t(void)vgc_warmupstep;\n");
+ Fc(tl, 0, "\treturn (%d);\n", has_event ? 1 : 0);
Fc(tl, 0, "}\n");
}
diff --git a/lib/libvcc/vcc_vmod.c b/lib/libvcc/vcc_vmod.c
index 60d2ed2..6cc2645 100644
--- a/lib/libvcc/vcc_vmod.c
+++ b/lib/libvcc/vcc_vmod.c
@@ -205,8 +205,7 @@ vcc_ParseImport(struct vcc *tl)
VSB_printf(ifp->fin,
"\t\t(void)%s(ctx, &vmod_priv_%.*s,\n"
"\t\t VCL_EVENT_DISCARD);\n", p, PF(mod));
- VSB_printf(ifp->event,
- "\t(void)%s(ctx, &vmod_priv_%.*s, ev);\n",
+ VSB_printf(ifp->event, "\t%s(ctx, &vmod_priv_%.*s, ev)",
p, PF(mod));
} else {
sym = VCC_AddSymbolStr(tl, p, SYM_FUNC);
--
2.4.3
From f50cf619e392ff092c792ac54b6491fe5e567531 Mon Sep 17 00:00:00 2001
From: Dridi Boukelmoune <[email protected]>
Date: Mon, 7 Dec 2015 12:07:27 +0100
Subject: [PATCH 5/7] Catch a vcl.state failure on the manager side
Don't update the state of the VCL to warm if it failed, and don't start
the child if the active VCL failed to warm up.
---
bin/varnishd/mgt/mgt.h | 2 +-
bin/varnishd/mgt/mgt_child.c | 3 ++-
bin/varnishd/mgt/mgt_vcl.c | 49 ++++++++++++++++++++++++----------------
bin/varnishtest/tests/v00044.vtc | 8 +++++--
4 files changed, 38 insertions(+), 24 deletions(-)
diff --git a/bin/varnishd/mgt/mgt.h b/bin/varnishd/mgt/mgt.h
index 5fb12d4..4756fad 100644
--- a/bin/varnishd/mgt/mgt.h
+++ b/bin/varnishd/mgt/mgt.h
@@ -167,7 +167,7 @@ void mgt_vcc_init(void);
void mgt_vcl_init(void);
void mgt_vcc_default(struct cli *, const char *b_arg, const char *vclsrc,
int Cflag);
-int mgt_push_vcls_and_start(unsigned *status, char **p);
+int mgt_push_vcls_and_start(struct cli *, unsigned *status, char **p);
int mgt_has_vcl(void);
extern char *mgt_cc_cmd;
extern const char *mgt_vcl_dir;
diff --git a/bin/varnishd/mgt/mgt_child.c b/bin/varnishd/mgt/mgt_child.c
index 8a26eb4..ae01a91 100644
--- a/bin/varnishd/mgt/mgt_child.c
+++ b/bin/varnishd/mgt/mgt_child.c
@@ -418,7 +418,8 @@ mgt_launch_child(struct cli *cli)
mgt_cli_start_child(child_cli_in, child_cli_out);
child_pid = pid;
- if (mgt_push_vcls_and_start(&u, &p)) {
+ if (mgt_push_vcls_and_start(cli, &u, &p)) {
+ VCLI_SetResult(cli, u);
MGT_complain(C_ERR, "Child (%jd) Pushing vcls failed:\n%s",
(intmax_t)child_pid, p);
free(p);
diff --git a/bin/varnishd/mgt/mgt_vcl.c b/bin/varnishd/mgt/mgt_vcl.c
index 673f9e1..227b9fc 100644
--- a/bin/varnishd/mgt/mgt_vcl.c
+++ b/bin/varnishd/mgt/mgt_vcl.c
@@ -125,12 +125,13 @@ mgt_has_vcl(void)
return (!VTAILQ_EMPTY(&vclhead));
}
-static void
-mgt_vcl_setstate(struct vclprog *vp, enum vclstate vs)
+static int
+mgt_vcl_setstate(struct cli *cli, struct vclprog *vp, enum vclstate vs)
{
unsigned status, warm;
double now;
char *p;
+ int i;
if (vs == VCL_STATE_AUTO) {
assert(vp != active_vcl);
@@ -145,7 +146,7 @@ mgt_vcl_setstate(struct vclprog *vp, enum vclstate vs)
warm = vs == VCL_STATE_WARM ? 1 : 0;
if (vp->warm == warm)
- return;
+ return (0);
vp->warm = warm;
@@ -153,15 +154,19 @@ mgt_vcl_setstate(struct vclprog *vp, enum vclstate vs)
vp->go_cold = 0;
if (child_pid < 0)
- return;
+ return (0);
- /*
- * We ignore the result here so we don't croak if the child did.
- */
- (void)mgt_cli_askchild(&status, &p, "vcl.state %s %d%s\n",
+ i = mgt_cli_askchild(&status, &p, "vcl.state %s %d%s\n",
vp->name, vp->warm, vp->state);
+ if (i) {
+ AN(cli);
+ AN(vp->warm);
+ VCLI_SetResult(cli, status);
+ VCLI_Out(cli, "%s", p);
+ }
free(p);
+ return (i);
}
/*--------------------------------------------------------------------*/
@@ -234,12 +239,15 @@ mgt_vcc_default(struct cli *cli, const char *b_arg, const char *vclsrc,
/*--------------------------------------------------------------------*/
int
-mgt_push_vcls_and_start(unsigned *status, char **p)
+mgt_push_vcls_and_start(struct cli *cli, unsigned *status, char **p)
{
struct vclprog *vp;
AN(active_vcl);
- mgt_vcl_setstate(active_vcl, VCL_STATE_WARM);
+
+ /* The VCL has not been loaded yet, it cannot fail */
+ AZ(mgt_vcl_setstate(cli, active_vcl, VCL_STATE_WARM));
+
VTAILQ_FOREACH(vp, &vclhead, list) {
if (mgt_cli_askchild(status, p, "vcl.load \"%s\" %s %d%s\n",
vp->name, vp->fname, vp->warm, vp->state))
@@ -330,7 +338,7 @@ mcf_vcl_state(struct cli *cli, const char * const *av, void *priv)
bprintf(vp->state, "%s", "auto");
if (vp != active_vcl) {
vp->go_cold = VTIM_mono();
- mgt_vcl_setstate(vp, VCL_STATE_AUTO);
+ (void)mgt_vcl_setstate(cli, vp, VCL_STATE_AUTO);
}
} else if (!strcmp(av[3], "cold")) {
if (vp == active_vcl) {
@@ -339,10 +347,10 @@ mcf_vcl_state(struct cli *cli, const char * const *av, void *priv)
return;
}
bprintf(vp->state, "%s", "auto");
- mgt_vcl_setstate(vp, VCL_STATE_COLD);
+ (void)mgt_vcl_setstate(cli, vp, VCL_STATE_COLD);
} else if (!strcmp(av[3], "warm")) {
- bprintf(vp->state, "%s", av[3]);
- mgt_vcl_setstate(vp, VCL_STATE_WARM);
+ if (mgt_vcl_setstate(cli, vp, VCL_STATE_WARM) == 0)
+ bprintf(vp->state, "%s", av[3]);
} else {
VCLI_Out(cli, "State must be one of auto, cold or warm.");
VCLI_SetResult(cli, CLIS_PARAM);
@@ -362,20 +370,21 @@ mcf_vcl_use(struct cli *cli, const char * const *av, void *priv)
return;
if (vp == active_vcl)
return;
- mgt_vcl_setstate(vp, VCL_STATE_WARM);
+ if (mgt_vcl_setstate(cli, vp, VCL_STATE_WARM))
+ return;
if (child_pid >= 0 &&
mgt_cli_askchild(&status, &p, "vcl.use %s\n", av[2])) {
VCLI_SetResult(cli, status);
VCLI_Out(cli, "%s", p);
vp->go_cold = VTIM_mono();
- mgt_vcl_setstate(vp, VCL_STATE_AUTO);
+ (void)mgt_vcl_setstate(cli, vp, VCL_STATE_AUTO);
} else {
VCLI_Out(cli, "VCL '%s' now active", av[2]);
vp2 = active_vcl;
active_vcl = vp;
if (vp2 != NULL) {
vp2->go_cold = VTIM_mono();
- mgt_vcl_setstate(vp2, VCL_STATE_AUTO);
+ (void)mgt_vcl_setstate(cli, vp2, VCL_STATE_AUTO);
}
}
free(p);
@@ -397,9 +406,9 @@ mcf_vcl_discard(struct cli *cli, const char * const *av, void *priv)
VCLI_Out(cli, "Cannot discard active VCL program\n");
return;
}
- mgt_vcl_setstate(vp, VCL_STATE_COLD);
+ (void)mgt_vcl_setstate(cli, vp, VCL_STATE_COLD);
if (child_pid >= 0) {
- /* If this fails the child is crashing, figure that later */
+ /* XXX If this fails the child is crashing, figure that later */
(void)mgt_cli_askchild(&status, &p, "vcl.discard %s\n", av[2]);
free(p);
}
@@ -443,7 +452,7 @@ mgt_vcl_poker(const struct vev *e, int what)
e_poker->timeout = mgt_param.vcl_cooldown * .45;
VTAILQ_FOREACH(vp, &vclhead, list) {
if (vp != active_vcl)
- mgt_vcl_setstate(vp, VCL_STATE_AUTO);
+ (void)mgt_vcl_setstate(NULL, vp, VCL_STATE_AUTO);
}
return (0);
}
diff --git a/bin/varnishtest/tests/v00044.vtc b/bin/varnishtest/tests/v00044.vtc
index 4b5a59a..c056d6c 100644
--- a/bin/varnishtest/tests/v00044.vtc
+++ b/bin/varnishtest/tests/v00044.vtc
@@ -84,6 +84,10 @@ varnish v1 -expect VBE.vcl1.default.happy >= 0
delay 4
varnish v1 -expect !VBE.vcl1.default.happy
-# A VMOD's warm-up can fail (not quite there yet)
+# A VMOD's warm-up can fail
varnish v1 -cliok "param.set max_esi_depth 42"
-varnish v1 -cliok "vcl.state vcl1 warm"
+varnish v1 -clierr 300 "vcl.state vcl1 warm"
+
+# A warm-up failure can also fail a child start
+varnish v1 -cliok stop
+varnish v1 -clierr 300 start
--
2.4.3
From 3255f3d7f7939b6e4cdf10eea438ee0311669000 Mon Sep 17 00:00:00 2001
From: Dridi Boukelmoune <[email protected]>
Date: Mon, 7 Dec 2015 15:02:13 +0100
Subject: [PATCH 6/7] Simplify `vcl.use` by making it failsafe
By the time we decide to switch to a VCL, it must be warm and usable.
The deprecated VCL_EVENT_USE should not get in the way.
---
bin/varnishd/cache/cache_vcl.c | 23 +++++------------------
lib/libvcc/vcc_compile.c | 3 ++-
lib/libvmod_debug/vmod_debug.c | 1 +
3 files changed, 8 insertions(+), 19 deletions(-)
diff --git a/bin/varnishd/cache/cache_vcl.c b/bin/varnishd/cache/cache_vcl.c
index 0375974..c272a9a 100644
--- a/bin/varnishd/cache/cache_vcl.c
+++ b/bin/varnishd/cache/cache_vcl.c
@@ -711,33 +711,20 @@ ccf_config_use(struct cli *cli, const char * const *av, void *priv)
struct vcl *vcl;
struct vrt_ctx ctx;
unsigned hand = 0;
- struct vsb *vsb;
- int i;
ASSERT_CLI();
+ AN(cli);
AZ(priv);
vcl = vcl_find(av[2]);
AN(vcl); // MGT ensures this
assert(vcl->temp == vcl_temp_warm); // MGT ensures this
INIT_OBJ(&ctx, VRT_CTX_MAGIC);
ctx.handling = &hand;
- vsb = VSB_new_auto();
- AN(vsb);
- ctx.msg = vsb;
ctx.vcl = vcl;
- i = vcl->conf->event_vcl(&ctx, VCL_EVENT_USE);
- AZ(VSB_finish(vsb));
- if (i) {
- VCLI_Out(cli, "VCL \"%s\" Failed to activate", av[2]);
- if (VSB_len(vsb) > 0)
- VCLI_Out(cli, "\nMessage:\n\t%s", VSB_data(vsb));
- VCLI_SetResult(cli, CLIS_CANT);
- } else {
- Lck_Lock(&vcl_mtx);
- vcl_active = vcl;
- Lck_Unlock(&vcl_mtx);
- }
- VSB_delete(vsb);
+ AZ(vcl->conf->event_vcl(&ctx, VCL_EVENT_USE));
+ Lck_Lock(&vcl_mtx);
+ vcl_active = vcl;
+ Lck_Unlock(&vcl_mtx);
}
static void __match_proto__(cli_func_t)
diff --git a/lib/libvcc/vcc_compile.c b/lib/libvcc/vcc_compile.c
index 2d08d00..0c80c3e 100644
--- a/lib/libvcc/vcc_compile.c
+++ b/lib/libvcc/vcc_compile.c
@@ -326,7 +326,8 @@ EmitCoordinates(const struct vcc *tl, struct vsb *vsb)
* Init/Fini/Event
*
* We call DISCARD and COLD events in the opposite order of LOAD and
- * WARM.
+ * WARM. The child will panic if a USE event fails, since a WARM event
+ * leads to a usable state.
*/
static void
diff --git a/lib/libvmod_debug/vmod_debug.c b/lib/libvmod_debug/vmod_debug.c
index 91b49c7..12c97c1 100644
--- a/lib/libvmod_debug/vmod_debug.c
+++ b/lib/libvmod_debug/vmod_debug.c
@@ -278,6 +278,7 @@ event_warm(VRT_CTX)
VSL(SLT_Debug, 0, "%s: VCL_EVENT_WARM", VCL_Name(ctx->vcl));
+ AN(ctx->msg);
if (cache_param->max_esi_depth == 42) {
VSB_printf(ctx->msg, "max_esi_depth is not the answer.");
return (-1);
--
2.4.3
From bfb131db72499a9409d54c869c862676b699f371 Mon Sep 17 00:00:00 2001
From: Dridi Boukelmoune <[email protected]>
Date: Mon, 7 Dec 2015 17:06:36 +0100
Subject: [PATCH 7/7] 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.
---
bin/varnishd/cache/cache_vcl.c | 80 ++++++++++++++++++++++++++++++++--------
bin/varnishtest/tests/v00045.vtc | 12 +++++-
doc/sphinx/reference/vmod.rst | 22 +++++++++++
include/vrt.h | 5 ++-
lib/libvmod_debug/vmod_debug.c | 43 ++++++++++++++++-----
5 files changed, 134 insertions(+), 28 deletions(-)
diff --git a/bin/varnishd/cache/cache_vcl.c b/bin/varnishd/cache/cache_vcl.c
index c272a9a..0ed8fa4 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);
}
/*--------------------------------------------------------------------*/
@@ -428,6 +455,23 @@ vcl_find(const char *name)
return (NULL);
}
+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)
{
@@ -450,16 +494,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;
AZ(vcl->conf->event_vcl(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);
@@ -467,7 +512,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->conf->event_vcl(ctx, VCL_EVENT_WARM);
if (i == 0)
@@ -532,6 +581,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;
@@ -585,7 +635,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;
@@ -701,7 +751,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 12a199d..47cb0ce 100644
--- a/doc/sphinx/reference/vmod.rst
+++ b/doc/sphinx/reference/vmod.rst
@@ -396,6 +396,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 96a3c64..4fe3067 100644
--- a/include/vrt.h
+++ b/include/vrt.h
@@ -298,8 +298,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 12c97c1..6384904 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;
};
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);
}
}
--
2.4.3
_______________________________________________
varnish-dev mailing list
[email protected]
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev
