[master] 706330867 fetch: Backends are in charge of printing headers

Dridi Boukelmoune dridi.boukelmoune at gmail.com
Wed Feb 1 17:12:07 UTC 2023


commit 7063308676415010da5f7b70f571a22a62c66c4b
Author: Guillaume Quintard <guillaume.quintard at gmail.com>
Date:   Sat Dec 24 23:13:35 2022 -0800

    fetch: Backends are in charge of printing headers
    
    The current backend implementation reads the headers all at once, as a
    big buffer, then manually chops them up, and later on, in the startfetch
    step, Varnish loops through all the headers and prints them.
    
    This is inconvenient for custom backends that are most likely going to
    use http_SetH() (directly or via http_SetHeader(), http_PrinfHeader() or
    others), which also prints the headers being added. As a result, those
    implementations end up logging the header twice.
    
    To work around the issue we can push the burden of logging the beresp
    headers onto the backend implementation. It does change one test, as
    now the Timestamp:Beresp log record appears after the headers instead
    of before.

diff --git a/bin/varnishd/cache/cache_backend.c b/bin/varnishd/cache/cache_backend.c
index 7ac117cc4..4802c05fc 100644
--- a/bin/varnishd/cache/cache_backend.c
+++ b/bin/varnishd/cache/cache_backend.c
@@ -273,6 +273,7 @@ vbe_dir_gethdrs(VRT_CTX, VCL_BACKEND d)
 	CHECK_OBJ_NOTNULL(d, DIRECTOR_MAGIC);
 	bo = ctx->bo;
 	CHECK_OBJ_NOTNULL(bo, BUSYOBJ_MAGIC);
+	CHECK_OBJ_NOTNULL(bo->bereq, HTTP_MAGIC);
 	if (bo->htc != NULL)
 		CHECK_OBJ_NOTNULL(bo->htc->doclose, STREAM_CLOSE_MAGIC);
 	wrk = ctx->bo->wrk;
@@ -317,6 +318,7 @@ vbe_dir_gethdrs(VRT_CTX, VCL_BACKEND d)
 				i = V1F_FetchRespHdr(bo);
 			if (i == 0) {
 				AN(bo->htc->priv);
+				http_VSL_log(bo->beresp);
 				return (0);
 			}
 		}
diff --git a/bin/varnishd/cache/cache_fetch.c b/bin/varnishd/cache/cache_fetch.c
index 343a28102..0af5c0d54 100644
--- a/bin/varnishd/cache/cache_fetch.c
+++ b/bin/varnishd/cache/cache_fetch.c
@@ -442,8 +442,6 @@ vbf_stp_startfetch(struct worker *wrk, struct busyobj *bo)
 		return (F_STP_ERROR);
 	}
 
-	http_VSL_log(bo->beresp);
-
 	if (bo->htc != NULL && bo->htc->body_status == BS_ERROR) {
 		bo->htc->doclose = SC_RX_BODY;
 		vbf_cleanup(bo);
diff --git a/bin/varnishtest/tests/c00036.vtc b/bin/varnishtest/tests/c00036.vtc
index adec31ab0..514fe08a9 100644
--- a/bin/varnishtest/tests/c00036.vtc
+++ b/bin/varnishtest/tests/c00036.vtc
@@ -33,11 +33,11 @@ logexpect l1 -v v1 -q "vxid == 1004" {
 	expect 0 1004 BackendOpen     {^\d+ s1}
 
 	expect 0 1004 Timestamp       {^Bereq:}
-	expect 0 1004 Timestamp       {^Beresp:}
 	expect 0 1004 BerespProtocol  {^HTTP/1.1}
 	expect 0 1004 BerespStatus    {^200}
 	expect 0 1004 BerespReason    {^OK}
 	expect 0 1004 BerespHeader    {^Content-Length: 6}
+	expect 0 1004 Timestamp       {^Beresp:}
 } -start
 
 client c1 {
diff --git a/doc/changes.rst b/doc/changes.rst
index 979857f08..4d0012a5d 100644
--- a/doc/changes.rst
+++ b/doc/changes.rst
@@ -75,6 +75,8 @@ Varnish Cache NEXT (2023-03-15)
 * The ``debug.xid`` CLI command now sets the next XID to be used,
   rather than "one less than the next XID to be used"
 
+* Backend implementations are in charge of logging their headers.
+
 ================================
 Varnish Cache 7.2.0 (2022-09-15)
 ================================
diff --git a/doc/sphinx/reference/directors.rst b/doc/sphinx/reference/directors.rst
index 66c0f406c..450515df7 100644
--- a/doc/sphinx/reference/directors.rst
+++ b/doc/sphinx/reference/directors.rst
@@ -204,6 +204,11 @@ to implement the whole probing infrastructure from scratch.
 You may also consider making your custom backend compliant with regards to the
 VCL state (see :ref:`ref-vmod-event-functions`).
 
+If you are implementing the `gethdrs` method of your backend (i.e. your
+backend is able to generate a backend response to be manipulated in
+`vcl_backend_response`), you will want to log the response code, protocol and
+the various headers it'll create for easier debugging. For this, you can look
+at the `VSL*` family of functions, listed in `cache/cache.h`.
 
 Data structure considerations
 -----------------------------


More information about the varnish-commit mailing list