[master] f463c682d http2: Improve timestamps

Nils Goroll nils.goroll at uplex.de
Mon Mar 3 15:34:05 UTC 2025


commit f463c682d557ffb9faf5a0a5f4e6d4466df24ca1
Author: Nils Goroll <nils.goroll at uplex.de>
Date:   Mon Feb 24 22:18:26 2025 +0100

    http2: Improve timestamps
    
    A newly created request's t_first is now when the first (partial) read of the
    HEADERS frame is complete.
    
    t_req is now when all headers are complete.
    
    This should match http1 more closely
    
    Implements #4282

diff --git a/bin/varnishd/http2/cache_http2.h b/bin/varnishd/http2/cache_http2.h
index 8794db23d..ba036b84d 100644
--- a/bin/varnishd/http2/cache_http2.h
+++ b/bin/varnishd/http2/cache_http2.h
@@ -196,6 +196,7 @@ struct h2_sess {
 	struct h2_settings		local_settings;
 
 	struct req			*new_req;
+	vtim_real			t1;	// t_first for new_req
 	uint32_t			goaway_last_stream;
 
 	VTAILQ_HEAD(,h2_req)		txqueue;
diff --git a/bin/varnishd/http2/cache_http2_proto.c b/bin/varnishd/http2/cache_http2_proto.c
index 3ef036145..d7309913c 100644
--- a/bin/varnishd/http2/cache_http2_proto.c
+++ b/bin/varnishd/http2/cache_http2_proto.c
@@ -329,6 +329,7 @@ h2_rx_push_promise(struct worker *wrk, struct h2_sess *h2, struct h2_req *r2)
 	CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
 	ASSERT_RXTHR(h2);
 	CHECK_OBJ_ORNULL(r2, H2_REQ_MAGIC);
+
 	// rfc7540,l,2262,2267
 	H2S_Lock_VSLb(h2, SLT_SessError, "H2: rx push promise");
 	return (H2CE_PROTOCOL_ERROR);
@@ -654,6 +655,7 @@ h2_end_headers(struct worker *wrk, struct h2_sess *h2,
 		h2_del_req(wrk, r2);
 		return (h2e);
 	}
+	req->t_req = VTIM_real();
 	VSLb_ts_req(req, "Req", req->t_req);
 
 	// XXX: Smarter to do this already at HPACK time into tail end of
@@ -774,8 +776,7 @@ h2_rx_headers(struct worker *wrk, struct h2_sess *h2, struct h2_req *r2)
 	req->sp = h2->sess;
 	req->transport = &HTTP2_transport;
 
-	req->t_first = VTIM_real();
-	req->t_req = VTIM_real();
+	req->t_first = h2->t1;
 	req->t_prev = req->t_first;
 	VSLb_ts_req(req, "Start", req->t_first);
 	req->acct.req_hdrbytes += h2->rxf_len;
@@ -1479,8 +1480,9 @@ h2_rxframe(struct worker *wrk, struct h2_sess *h2)
 	if (h2->goaway && h2->open_streams == 0)
 		return (0);
 
+	h2->t1 = NAN;
 	VTCP_blocking(*h2->htc->rfd);
-	hs = HTC_RxStuff(h2->htc, h2_frame_complete, NULL, NULL, NAN,
+	hs = HTC_RxStuff(h2->htc, h2_frame_complete, &h2->t1, NULL, NAN,
 	    VTIM_real() + 0.5, NAN, h2->local_settings.max_frame_size + 9);
 
 	h2e = NULL;
diff --git a/doc/changes.rst b/doc/changes.rst
index a132c45cf..0df080469 100644
--- a/doc/changes.rst
+++ b/doc/changes.rst
@@ -41,6 +41,26 @@ Varnish Cache NEXT (2025-03-15)
 .. PLEASE keep this roughly in commit order as shown by git-log / tig
    (new to old)
 
+* (Add a prominent note to announcement) Timestamps for http/2 requests have
+  been corrected and made similar to how they are taken for http/1.
+
+  For http/1, the start time, internally called "t_first", is taken as soon as
+  any part of the request (headers) is received. Previously, http/2 took it
+  later, possibly much later if long header lines were involved. http/2 now
+  takes it the same way as http/1 when the first bit of the first HEADERS frame
+  of the request arrives.
+
+  Timing behavior for http/1 and http/2 is different and can not be directly
+  compared. But with this change, the ``Timestamp`` VSL records for http/2 now
+  at least reflect reality better.
+
+  NOTE that after upgrading Varnish-Cache, processing and response times for
+  http/2 will now be reported as worse than before the upgrade, potentially
+  *much* worse. This is **NOT** a performance regression, but rather due to the
+  corrected timestamps, which arguably were wrong for http/2.
+
+.. MISSING commits
+
 * The ``hitmiss`` and ``hitpass`` handling indicators have been added to the
   ``Varnish:handling`` format of ``varnishncsa``.
 


More information about the varnish-commit mailing list