[master] fb046f6 Sort backend fetches (more) into protocol-agnostic bits, and protocol- implementation bits. Still not there, but getting closer...

Poul-Henning Kamp phk at FreeBSD.org
Wed Sep 24 14:21:52 CEST 2014


commit fb046f6ead21ec0ec5044e9223e0c41a7b4de5b1
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date:   Wed Sep 24 12:21:03 2014 +0000

    Sort backend fetches (more) into protocol-agnostic bits, and protocol-
    implementation bits.  Still not there, but getting closer...

diff --git a/bin/varnishd/cache/cache.h b/bin/varnishd/cache/cache.h
index 142fcef..238baa1 100644
--- a/bin/varnishd/cache/cache.h
+++ b/bin/varnishd/cache/cache.h
@@ -457,6 +457,12 @@ enum busyobj_state_e {
 	BOS_FAILED,		/* something went wrong */
 };
 
+enum director_state_e {
+	DIR_S_NULL = 0,
+	DIR_S_HDRS = 1,
+	DIR_S_BODY = 2,
+};
+
 struct busyobj {
 	unsigned		magic;
 #define BUSYOBJ_MAGIC		0x23b95567
@@ -513,6 +519,7 @@ struct busyobj {
 	const char		*storage_hint;
 	const struct director	*director_req;
 	const struct director	*director_resp;
+	enum director_state_e	director_state;
 	struct VCL_conf		*vcl;
 
 	struct vsl_log		vsl[1];
diff --git a/bin/varnishd/cache/cache_backend.c b/bin/varnishd/cache/cache_backend.c
index ba692fb..b8a2a79 100644
--- a/bin/varnishd/cache/cache_backend.c
+++ b/bin/varnishd/cache/cache_backend.c
@@ -414,6 +414,7 @@ vdi_simple_gethdrs(const struct director *d, struct worker *wrk,
 	 * Do a single retry in that case.
 	 */
 	if (i == 1) {
+		AZ(bo->vbc);
 		VSC_C_main->backend_retry++;
 		bo->vbc = VDI_GetFd(d, wrk, bo);
 		if (bo->vbc == NULL) {
@@ -422,6 +423,10 @@ vdi_simple_gethdrs(const struct director *d, struct worker *wrk,
 		}
 		i = V1F_fetch_hdr(wrk, bo);
 	}
+	if (i != 0)
+		AZ(bo->vbc);
+	else
+		AN(bo->vbc);
 	return (i);
 }
 
@@ -438,6 +443,23 @@ vdi_simple_getbody(const struct director *d, struct worker *wrk,
 	return (0);
 }
 
+static void __match_proto__(vdi_finish_f)
+vdi_simple_finish(const struct director *d, struct worker *wrk,
+    struct busyobj *bo)
+{
+	CHECK_OBJ_NOTNULL(d, DIRECTOR_MAGIC);
+	CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
+	CHECK_OBJ_NOTNULL(bo, BUSYOBJ_MAGIC);
+
+	if (bo->vbc != NULL) {
+		if (bo->doclose != SC_NULL)
+			VDI_CloseFd(&bo->vbc, &bo->acct);
+		else
+			VDI_RecycleFd(&bo->vbc, &bo->acct);
+	}
+}
+
+
 /*--------------------------------------------------------------------*/
 
 void
@@ -477,6 +499,7 @@ VRT_init_dir(struct cli *cli, struct director **bp, int idx, const void *priv)
 	vs->dir.healthy = vdi_simple_healthy;
 	vs->dir.gethdrs = vdi_simple_gethdrs;
 	vs->dir.getbody = vdi_simple_getbody;
+	vs->dir.finish = vdi_simple_finish;
 
 	vs->vrt = t;
 
diff --git a/bin/varnishd/cache/cache_backend.h b/bin/varnishd/cache/cache_backend.h
index 6ff1351..c51d62f 100644
--- a/bin/varnishd/cache/cache_backend.h
+++ b/bin/varnishd/cache/cache_backend.h
@@ -61,6 +61,8 @@ typedef int vdi_gethdrs_f(const struct director *, struct worker *,
     struct busyobj *);
 typedef int vdi_getbody_f(const struct director *, struct worker *,
     struct busyobj *);
+typedef void vdi_finish_f(const struct director *, struct worker *,
+    struct busyobj *);
 
 struct director {
 	unsigned		magic;
@@ -72,6 +74,7 @@ struct director {
 	vdi_resolve_f		*resolve;
 	vdi_gethdrs_f		*gethdrs;
 	vdi_getbody_f		*getbody;
+	vdi_finish_f		*finish;
 	void			*priv;
 };
 
@@ -159,6 +162,8 @@ void VBP_Summary(struct cli *cli, const struct vbp_target *vt);
 int VDI_GetHdr(struct worker *wrk, struct busyobj *bo);
 int VDI_GetBody(const struct director *d, struct worker *wrk,
     struct busyobj *bo);
+void VDI_Finish(const struct director *d, struct worker *wrk,
+    struct busyobj *bo);
 struct vbc *VDI_GetFd(const struct director *d, struct worker *wrk,
     struct busyobj *);
 int VDI_Healthy(const struct director *);
diff --git a/bin/varnishd/cache/cache_busyobj.c b/bin/varnishd/cache/cache_busyobj.c
index 9c47154..4256400 100644
--- a/bin/varnishd/cache/cache_busyobj.c
+++ b/bin/varnishd/cache/cache_busyobj.c
@@ -197,6 +197,8 @@ VBO_DerefBusyObj(struct worker *wrk, struct busyobj **pbo)
 
 	VSL_End(bo->vsl);
 
+	AZ(bo->vbc);
+
 	if (bo->fetch_objcore != NULL) {
 		AN(wrk);
 		(void)HSH_DerefObjCore(wrk, &bo->fetch_objcore);
diff --git a/bin/varnishd/cache/cache_dir.c b/bin/varnishd/cache/cache_dir.c
index a9ad94e..0dd65b5 100644
--- a/bin/varnishd/cache/cache_dir.c
+++ b/bin/varnishd/cache/cache_dir.c
@@ -120,15 +120,21 @@ vdi_resolve(struct worker *wrk, struct busyobj *bo, const struct director *d)
 int
 VDI_GetHdr(struct worker *wrk, struct busyobj *bo)
 {
-
 	const struct director *d;
+	int i = -1;
 
-	d = vdi_resolve(wrk, bo, bo->director_req);
-	if (d == NULL)
-		return (-1);
+	CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
+	CHECK_OBJ_NOTNULL(bo, BUSYOBJ_MAGIC);
 
-	AN(d->gethdrs);
-	return (d->gethdrs(d, wrk, bo));
+	d = vdi_resolve(wrk, bo, bo->director_req);
+	if (d != NULL) {
+		AN(d->gethdrs);
+		bo->director_state = DIR_S_HDRS;
+		i = d->gethdrs(d, wrk, bo);
+	}
+	if (i)
+		bo->director_state = DIR_S_NULL;
+	return (i);
 }
 
 /* Setup body fetch --------------------------------------------------*/
@@ -137,16 +143,36 @@ int
 VDI_GetBody(const struct director *d, struct worker *wrk, struct busyobj *bo)
 {
 
+	CHECK_OBJ_NOTNULL(d, DIRECTOR_MAGIC);
 	CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
 	CHECK_OBJ_NOTNULL(bo, BUSYOBJ_MAGIC);
-	CHECK_OBJ_NOTNULL(d, DIRECTOR_MAGIC);
 
 	AZ(d->resolve);
 	AN(d->getbody);
 
+	bo->director_state = DIR_S_BODY;
 	return (d->getbody(d, wrk, bo));
 }
 
+/* Finish fetch ------------------------------------------------------*/
+
+void
+VDI_Finish(const struct director *d, struct worker *wrk, struct busyobj *bo)
+{
+
+	CHECK_OBJ_NOTNULL(d, DIRECTOR_MAGIC);
+	CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
+	CHECK_OBJ_NOTNULL(bo, BUSYOBJ_MAGIC);
+
+	AZ(d->resolve);
+	AN(d->finish);
+
+	assert(bo->director_state != DIR_S_NULL);
+	bo->director_state = DIR_S_NULL;
+
+	d->finish(d, wrk, bo);
+}
+
 /* Get a connection --------------------------------------------------*/
 
 struct vbc *
diff --git a/bin/varnishd/cache/cache_fetch.c b/bin/varnishd/cache/cache_fetch.c
index 7ba5852..f680f02 100644
--- a/bin/varnishd/cache/cache_fetch.c
+++ b/bin/varnishd/cache/cache_fetch.c
@@ -174,7 +174,6 @@ vbf_stp_mkbereq(const struct worker *wrk, struct busyobj *bo)
 	CHECK_OBJ_NOTNULL(bo->req, REQ_MAGIC);
 
 	assert(bo->state == BOS_INVALID);
-	AZ(bo->vbc);
 	assert(bo->doclose == SC_NULL);
 	AZ(bo->storage_hint);
 
@@ -245,7 +244,6 @@ vbf_stp_startfetch(struct worker *wrk, struct busyobj *bo)
 	CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
 	CHECK_OBJ_NOTNULL(bo, BUSYOBJ_MAGIC);
 
-	AZ(bo->vbc);
 	assert(bo->doclose == SC_NULL);
 	AZ(bo->storage_hint);
 
@@ -274,11 +272,10 @@ vbf_stp_startfetch(struct worker *wrk, struct busyobj *bo)
 	VSLb_ts_busyobj(bo, "Beresp", now);
 
 	if (i) {
-		AZ(bo->vbc);
+		assert(bo->director_state == DIR_S_NULL);
 		return (F_STP_ERROR);
 	}
 
-	AN(bo->vbc);
 	http_VSL_log(bo->beresp);
 
 	if (!http_GetHdr(bo->beresp, H_Date, NULL)) {
@@ -355,9 +352,10 @@ vbf_stp_startfetch(struct worker *wrk, struct busyobj *bo)
 	}
 
 	if (bo->htc->body_status == BS_ERROR) {
-		AN (bo->vbc);
-		VDI_CloseFd(&bo->vbc, &bo->acct);
+		bo->doclose = SC_RX_BODY;
+		VDI_Finish(bo->director_resp, bo->wrk, bo);
 		VSLb(bo->vsl, SLT_Error, "Body cannot be fetched");
+		assert(bo->director_state == DIR_S_NULL);
 		return (F_STP_ERROR);
 	}
 
@@ -398,18 +396,22 @@ vbf_stp_startfetch(struct worker *wrk, struct busyobj *bo)
 
 	VCL_backend_response_method(bo->vcl, wrk, NULL, bo, bo->beresp->ws);
 
-	if (wrk->handling == VCL_RET_ABANDON)
+	if (wrk->handling == VCL_RET_ABANDON) {
+		bo->doclose = SC_RESP_CLOSE;
+		VDI_Finish(bo->director_resp, bo->wrk, bo);
 		return (F_STP_FAIL);
+	}
 
 	if (wrk->handling == VCL_RET_RETRY) {
-		AN (bo->vbc);
-		VDI_CloseFd(&bo->vbc, &bo->acct);
+		bo->doclose = SC_RESP_CLOSE;
+		VDI_Finish(bo->director_resp, bo->wrk, bo);
 		bo->doclose = SC_NULL;
 		bo->retries++;
 		if (bo->retries <= cache_param->max_retries)
 			return (F_STP_RETRY);
 		VSLb(bo->vsl, SLT_VCL_Error,
 		    "Too many retries, delivering 503");
+		assert(bo->director_state == DIR_S_NULL);
 		return (F_STP_ERROR);
 	}
 
@@ -481,14 +483,14 @@ vbf_fetch_body_helper(struct busyobj *bo)
 		}
 	} while (vfps == VFP_OK);
 
+	VFP_Close(vfc);
+
 	if (vfps == VFP_ERROR) {
 		AN(vfc->failed);
 		(void)VFP_Error(vfc, "Fetch Pipeline failed to process");
 		bo->doclose = SC_RX_BODY;
 	}
 
-	VFP_Close(vfc);
-
 	if (!bo->do_stream)
 		ObjTrimStore(bo->wrk, vfc->oc);
 }
@@ -539,8 +541,6 @@ vbf_stp_fetch(struct worker *wrk, struct busyobj *bo)
 	if (bo->do_gzip && !bo->is_gunzip)
 		bo->do_gzip = 0;
 
-	AN(bo->vbc);
-
 	/* But we can't do both at the same time */
 	assert(bo->do_gzip == 0 || bo->do_gunzip == 0);
 
@@ -569,12 +569,14 @@ vbf_stp_fetch(struct worker *wrk, struct busyobj *bo)
 	if (VFP_Open(bo->vfc)) {
 		(void)VFP_Error(bo->vfc, "Fetch Pipeline failed to open");
 		bo->doclose = SC_RX_BODY;
+		VDI_Finish(bo->director_resp, bo->wrk, bo);
 		return (F_STP_ERROR);
 	}
 
 	if (vbf_beresp2obj(bo)) {
 		(void)VFP_Error(bo->vfc, "Could not get storage");
 		bo->doclose = SC_RX_BODY;
+		VDI_Finish(bo->director_resp, bo->wrk, bo);
 		return (F_STP_ERROR);
 	}
 
@@ -620,11 +622,15 @@ vbf_stp_fetch(struct worker *wrk, struct busyobj *bo)
 	if (bo->vfc->failed && !bo->do_stream) {
 		assert(bo->state < BOS_STREAM);
 		ObjFreeObj(bo->wrk, bo->fetch_objcore);
+		// XXX: doclose = ?
+		VDI_Finish(bo->director_resp, bo->wrk, bo);
 		return (F_STP_ERROR);
 	}
 
-	if (bo->vfc->failed)
+	if (bo->vfc->failed) {
+		VDI_Finish(bo->director_resp, bo->wrk, bo);
 		return (F_STP_FAIL);
+	}
 
 	if (bo->do_stream)
 		assert(bo->state == BOS_STREAM);
@@ -635,10 +641,7 @@ vbf_stp_fetch(struct worker *wrk, struct busyobj *bo)
 
 	/* Recycle the backend connection before setting BOS_FINISHED to
 	   give predictable backend reuse behavior for varnishtest */
-	if (bo->vbc != NULL && bo->doclose == SC_NULL) {
-		VDI_RecycleFd(&bo->vbc, &bo->acct);
-		AZ(bo->vbc);
-	}
+	VDI_Finish(bo->director_resp, bo->wrk, bo);
 
 	VBO_setstate(bo, BOS_FINISHED);
 	VSLb_ts_busyobj(bo, "BerespBody", W_TIM_real(wrk));
@@ -708,10 +711,7 @@ vbf_stp_condfetch(struct worker *wrk, struct busyobj *bo)
 
 	/* Recycle the backend connection before setting BOS_FINISHED to
 	   give predictable backend reuse behavior for varnishtest */
-	if (bo->vbc != NULL && bo->doclose == SC_NULL) {
-		VDI_RecycleFd(&bo->vbc, &bo->acct);
-		AZ(bo->vbc);
-	}
+	VDI_Finish(bo->director_resp, bo->wrk, bo);
 
 	VBO_setstate(bo, BOS_FINISHED);
 	VSLb_ts_busyobj(bo, "BerespBody", W_TIM_real(wrk));
@@ -732,6 +732,7 @@ vbf_stp_error(struct worker *wrk, struct busyobj *bo)
 
 	CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
 	CHECK_OBJ_NOTNULL(bo, BUSYOBJ_MAGIC);
+	assert(bo->director_state == DIR_S_NULL);
 
 	now = W_TIM_real(wrk);
 	VSLb_ts_busyobj(bo, "Error", now);
@@ -868,13 +869,7 @@ vbf_fetch_thread(struct worker *wrk, void *priv)
 	}
 	assert(WRW_IsReleased(wrk));
 
-	if (bo->vbc != NULL) {
-		if (bo->doclose != SC_NULL)
-			VDI_CloseFd(&bo->vbc, &bo->acct);
-		else
-			VDI_RecycleFd(&bo->vbc, &bo->acct);
-		AZ(bo->vbc);
-	}
+	assert(bo->director_state == DIR_S_NULL);
 
 	http_Teardown(bo->bereq);
 	http_Teardown(bo->beresp);
diff --git a/bin/varnishd/cache/cache_vrt_var.c b/bin/varnishd/cache/cache_vrt_var.c
index 78d1ea8..0679086 100644
--- a/bin/varnishd/cache/cache_vrt_var.c
+++ b/bin/varnishd/cache/cache_vrt_var.c
@@ -315,10 +315,6 @@ VRT_r_beresp_backend_name(const struct vrt_ctx *ctx)
 	CHECK_OBJ_NOTNULL(ctx->bo, BUSYOBJ_MAGIC);
 	if (ctx->bo->director_resp != NULL)
 		return (ctx->bo->director_resp->vcl_name);
-	if (ctx->bo->vbc != NULL) {
-		CHECK_OBJ_NOTNULL(ctx->bo->vbc, VBC_MAGIC);
-		return (ctx->bo->vbc->backend->vcl_name);
-	}
 	return (NULL);
 }
 



More information about the varnish-commit mailing list