On Thu, Sep 10, 2015 at 7:38 PM, Dridi Boukelmoune <[email protected]> wrote: > Hi, > > This is Varnish 4.1 material. > > I have finally done and tested the changes we had decided to make > during the last VDD: > - removing the USE event > - allowing VMODs to fail a WARM event > - VCL reference counting for VMODs > - relaxing the "cold" restrictions on cooling VCLs > > I have found and fixed more issues than anticipated after I updated my > DNS director draft to comply to the new rules. I'm happy to say that > the director is actually simpler to implement now. I have also updated > the documentation along with the code, but I may have missed some > changes and I'm running late for tonight's rehearsal :) > > Best Regards, > Dridi
Following today's discussion on IRC, there's an error I introduced when cleaning up the patch set before submission, and did not notice. Attached is the fix for the 10th patch. Best Regards, Dridi
From 51212454a36c337b0eb08078ebd8833bc92aed9b Mon Sep 17 00:00:00 2001 From: Dridi Boukelmoune <[email protected]> Date: Thu, 10 Sep 2015 18:24:54 +0200 Subject: [PATCH 10/10] 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/cache/cache_vcl.c | 5 +++-- bin/varnishd/mgt/mgt_child.c | 1 + bin/varnishd/mgt/mgt_vcl.c | 33 ++++++++++++++++++++------------- bin/varnishtest/tests/v00044.vtc | 6 +++++- 4 files changed, 29 insertions(+), 16 deletions(-) diff --git a/bin/varnishd/cache/cache_vcl.c b/bin/varnishd/cache/cache_vcl.c index a40388d..110d75e 100644 --- a/bin/varnishd/cache/cache_vcl.c +++ b/bin/varnishd/cache/cache_vcl.c @@ -495,6 +495,7 @@ 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); } @@ -504,6 +505,7 @@ VCL_Load(struct cli *cli, const char *name, const char *fn, const char *state) vcl = VCL_Open(fn, vsb); if (vcl == NULL) { + VCLI_SetResult(cli, CLIS_PARAM); AZ(VSB_finish(vsb)); VCLI_Out(cli, "%s", VSB_data(vsb)); VSB_delete(vsb); @@ -633,8 +635,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) diff --git a/bin/varnishd/mgt/mgt_child.c b/bin/varnishd/mgt/mgt_child.c index 0fdf199..68c1e6d 100644 --- a/bin/varnishd/mgt/mgt_child.c +++ b/bin/varnishd/mgt/mgt_child.c @@ -434,6 +434,7 @@ 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(cli, &u, &p)) { + VCLI_SetResult(cli, u); REPORT(LOG_ERR, "Pushing vcls failed:\n%s", p); free(p); child_state = CH_RUNNING; diff --git a/bin/varnishd/mgt/mgt_vcl.c b/bin/varnishd/mgt/mgt_vcl.c index d645df9..6d610f4 100644 --- a/bin/varnishd/mgt/mgt_vcl.c +++ b/bin/varnishd/mgt/mgt_vcl.c @@ -126,7 +126,7 @@ mgt_has_vcl(void) return (!VTAILQ_EMPTY(&vclhead)); } -static void +static int mgt_vcl_setstate(struct cli *cli, struct vclprog *vp, enum vclstate vs) { unsigned status, warm; @@ -146,7 +146,7 @@ mgt_vcl_setstate(struct cli *cli, struct vclprog *vp, enum vclstate vs) warm = vs == VCL_STATE_WARM ? 1 : 0; if (vp->warm == warm) - return; + return (0); vp->warm = warm; @@ -154,7 +154,7 @@ mgt_vcl_setstate(struct cli *cli, struct vclprog *vp, enum vclstate vs) vp->go_cold = 0; if (child_pid < 0) - return; + return (0); if (mgt_cli_askchild(&status, &p, "vcl.state %s %d%s\n", vp->name, vp->warm, vp->state)) { @@ -162,9 +162,12 @@ mgt_vcl_setstate(struct cli *cli, struct vclprog *vp, enum vclstate vs) AN(vp->warm); VCLI_SetResult(cli, status); VCLI_Out(cli, "%s", p); + free(p); + return (1); } free(p); + return (0); } /*--------------------------------------------------------------------*/ @@ -242,7 +245,10 @@ mgt_push_vcls_and_start(struct cli *cli, unsigned *status, char **p) struct vclprog *vp; AN(active_vcl); - mgt_vcl_setstate(cli, 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)) @@ -333,7 +339,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(NULL, vp, VCL_STATE_AUTO); + (void)mgt_vcl_setstate(NULL, vp, VCL_STATE_AUTO); } } else if (!strcmp(av[3], "cold")) { if (vp == active_vcl) { @@ -342,10 +348,10 @@ mcf_vcl_state(struct cli *cli, const char * const *av, void *priv) return; } bprintf(vp->state, "%s", "auto"); - mgt_vcl_setstate(NULL, vp, VCL_STATE_COLD); + (void)mgt_vcl_setstate(NULL, vp, VCL_STATE_COLD); } else if (!strcmp(av[3], "warm")) { - bprintf(vp->state, "%s", av[3]); - mgt_vcl_setstate(cli, 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); @@ -365,20 +371,21 @@ mcf_vcl_use(struct cli *cli, const char * const *av, void *priv) return; if (vp == active_vcl) return; - mgt_vcl_setstate(cli, 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(NULL, vp, VCL_STATE_AUTO); + (void)mgt_vcl_setstate(NULL, 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(NULL, vp2, VCL_STATE_AUTO); + (void)mgt_vcl_setstate(NULL, vp2, VCL_STATE_AUTO); } } free(p); @@ -400,7 +407,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(NULL, vp, VCL_STATE_COLD); + (void)mgt_vcl_setstate(NULL, vp, VCL_STATE_COLD); if (child_pid >= 0) { /* XXX If this fails the child is crashing, figure that later */ (void)mgt_cli_askchild(&status, &p, "vcl.discard %s\n", av[2]); @@ -446,7 +453,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(NULL, 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 e9eeecc..9f5a9c1 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 warmup can fail +# A VMOD's warm-up can fail varnish v1 -cliok "param.set max_esi_depth 42" varnish v1 -clierr 300 "vcl.state vcl1 warm" + +# A warm-up fail can also fail a child start +varnish v1 -cliok stop +varnish v1 -clierr 300 start -- 2.4.3
_______________________________________________ varnish-dev mailing list [email protected] https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev
