[4.1] 8dd463a Catch a vcl.state failure on the manager side

Lasse Karstensen lkarsten at varnish-software.com
Fri Jan 22 16:46:58 CET 2016


commit 8dd463a32e3cb9c17762c44148dcdafc2ec39441
Author: Dridi Boukelmoune <dridi.boukelmoune at gmail.com>
Date:   Mon Dec 7 12:07:27 2015 +0100

    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.

diff --git a/bin/varnishd/mgt/mgt.h b/bin/varnishd/mgt/mgt.h
index 1aeba55..fe96360 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_startup(struct cli *, const char *b_arg, const char *f_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 11578ce..4c5ffc3 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 b651f83..858c5ec 100644
--- a/bin/varnishd/mgt/mgt_vcl.c
+++ b/bin/varnishd/mgt/mgt_vcl.c
@@ -122,12 +122,13 @@ mgt_has_vcl(void)
 	return (!VTAILQ_EMPTY(&vclhead));
 }
 
-static void
-mgt_vcl_setstate(struct vclprog *vp, const char *vs)
+static int
+mgt_vcl_setstate(struct cli *cli, struct vclprog *vp, const char *vs)
 {
 	unsigned status, warm;
 	double now;
 	char *p;
+	int i;
 
 	if (vs == VCL_STATE_AUTO) {
 		assert(vp != active_vcl);
@@ -142,7 +143,7 @@ mgt_vcl_setstate(struct vclprog *vp, const char *vs)
 	warm = vs == VCL_STATE_WARM ? 1 : 0;
 
 	if (vp->warm == warm)
-		return;
+		return (0);
 
 	vp->warm = warm;
 
@@ -150,15 +151,19 @@ mgt_vcl_setstate(struct vclprog *vp, const char *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);
 }
 
 /*--------------------------------------------------------------------*/
@@ -239,12 +244,15 @@ mgt_vcc_startup(struct cli *cli, const char *b_arg, const char *f_arg,
 /*--------------------------------------------------------------------*/
 
 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))
@@ -326,7 +334,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) {
@@ -335,10 +343,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);
@@ -358,20 +366,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);
@@ -393,9 +402,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);
 	}
@@ -439,7 +448,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 aa79e6a..c056d6c 100644
--- a/bin/varnishtest/tests/v00044.vtc
+++ b/bin/varnishtest/tests/v00044.vtc
@@ -83,3 +83,11 @@ 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
+varnish v1 -cliok "param.set max_esi_depth 42"
+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



More information about the varnish-commit mailing list