[master] 3c8ecb4cc Incremental h2 request accounting

Nils Goroll nils.goroll at uplex.de
Wed Apr 24 11:41:08 UTC 2019


commit 3c8ecb4cc0447e0149715e2fbb773a8fa9f433ce
Author: Dridi Boukelmoune <dridi.boukelmoune at gmail.com>
Date:   Wed Apr 17 19:00:39 2019 +0200

    Incremental h2 request accounting
    
    The dummy_acct is only here to avoid repeating null checks in the send
    loop below. This doesn't change the end result if the transaction
    completes with no problem. As such, only header and body bytes are
    accounted for, as before, ignoring h2 framing overhead and in general
    other kinds of frames that belong to the stream.
    
    In other words, the only improvement is that ReqAcct doesn't show a full
    delivery when the client hangs up before the end of the transaction.
    
    The split of H2_Send will allow handling of error conditions from the
    multiple return statements although at this point there is no change in
    this area.

diff --git a/bin/varnishd/http2/cache_http2.h b/bin/varnishd/http2/cache_http2.h
index 5399f2f98..83bf5460c 100644
--- a/bin/varnishd/http2/cache_http2.h
+++ b/bin/varnishd/http2/cache_http2.h
@@ -223,8 +223,8 @@ void H2_Send_Frame(struct worker *, struct h2_sess *,
     h2_frame type, uint8_t flags, uint32_t len, uint32_t stream,
     const void *);
 
-void H2_Send(struct worker *, struct h2_req *,
-    h2_frame type, uint8_t flags, uint32_t len, const void *);
+void H2_Send(struct worker *, struct h2_req *, h2_frame type, uint8_t flags,
+    uint32_t len, const void *, uint64_t *acct);
 
 /* cache_http2_proto.c */
 struct h2_req * h2_new_req(const struct worker *, struct h2_sess *,
diff --git a/bin/varnishd/http2/cache_http2_deliver.c b/bin/varnishd/http2/cache_http2_deliver.c
index d5febbf38..025d431d2 100644
--- a/bin/varnishd/http2/cache_http2_deliver.c
+++ b/bin/varnishd/http2/cache_http2_deliver.c
@@ -88,7 +88,7 @@ h2_fini(struct req *req, void **priv)
 	CHECK_OBJ_NOTNULL(req, REQ_MAGIC);
 	TAKE_OBJ_NOTNULL(r2, priv, H2_REQ_MAGIC);
 	H2_Send_Get(req->wrk, r2->h2sess, r2);
-	H2_Send(req->wrk, r2, H2_F_DATA, H2FF_DATA_END_STREAM, 0, "");
+	H2_Send(req->wrk, r2, H2_F_DATA, H2FF_DATA_END_STREAM, 0, "", NULL);
 	H2_Send_Rel(r2->h2sess, r2);
 	return (0);
 }
@@ -106,9 +106,9 @@ h2_bytes(struct req *req, enum vdp_action act, void **priv,
 	if ((r2->h2sess->error || r2->error))
 		return (-1);
 	H2_Send_Get(req->wrk, r2->h2sess, r2);
-	H2_Send(req->wrk, r2, H2_F_DATA, H2FF_NONE, len, ptr);
+	H2_Send(req->wrk, r2, H2_F_DATA, H2FF_NONE, len, ptr,
+	    &req->acct.resp_bodybytes);
 	H2_Send_Rel(r2->h2sess, r2);
-	req->acct.resp_bodybytes += len;
 	return (0);
 }
 
@@ -173,7 +173,7 @@ h2_minimal_response(struct req *req, uint16_t status)
 	    H2_F_HEADERS,
 	    H2FF_HEADERS_END_HEADERS |
 		(status < 200 ? 0 : H2FF_HEADERS_END_STREAM),
-	    l, buf);
+	    l, buf, NULL);
 	H2_Send_Rel(r2->h2sess, r2);
 	return (0);
 }
@@ -317,9 +317,8 @@ h2_deliver(struct req *req, struct boc *boc, int sendbody)
 	H2_Send_Get(req->wrk, r2->h2sess, r2);
 	H2_Send(req->wrk, r2, H2_F_HEADERS,
 	    (sendbody ? 0 : H2FF_HEADERS_END_STREAM) | H2FF_HEADERS_END_HEADERS,
-	    sz, r);
+	    sz, r, &req->acct.resp_hdrbytes);
 	H2_Send_Rel(r2->h2sess, r2);
-	req->acct.resp_hdrbytes += sz;
 
 	WS_Release(req->ws, 0);
 
diff --git a/bin/varnishd/http2/cache_http2_send.c b/bin/varnishd/http2/cache_http2_send.c
index dd20d1051..a0d8600cd 100644
--- a/bin/varnishd/http2/cache_http2_send.c
+++ b/bin/varnishd/http2/cache_http2_send.c
@@ -256,9 +256,9 @@ h2_do_window(struct worker *wrk, struct h2_req *r2,
  * XXX: priority
  */
 
-void
-H2_Send(struct worker *wrk, struct h2_req *r2,
-    h2_frame ftyp, uint8_t flags, uint32_t len, const void *ptr)
+static void
+h2_send(struct worker *wrk, struct h2_req *r2, h2_frame ftyp, uint8_t flags,
+    uint32_t len, const void *ptr, uint64_t *acct)
 {
 	struct h2_sess *h2;
 	uint32_t mfs, tf;
@@ -270,6 +270,7 @@ H2_Send(struct worker *wrk, struct h2_req *r2,
 	h2 = r2->h2sess;
 	CHECK_OBJ_NOTNULL(h2, H2_SESS_MAGIC);
 	assert(len == 0 || ptr != NULL);
+	AN(acct);
 
 	assert(VTAILQ_FIRST(&h2->txqueue) == r2);
 
@@ -297,8 +298,7 @@ H2_Send(struct worker *wrk, struct h2_req *r2,
 	Lck_Unlock(&h2->sess->mtx);
 
 	if (ftyp->respect_window) {
-		tf = h2_do_window(wrk, r2, h2,
-				  (len > mfs) ? mfs : len);
+		tf = h2_do_window(wrk, r2, h2, (len > mfs) ? mfs : len);
 		if (h2_errcheck(r2, h2))
 			return;
 		assert(VTAILQ_FIRST(&h2->txqueue) == r2);
@@ -307,6 +307,7 @@ H2_Send(struct worker *wrk, struct h2_req *r2,
 
 	if (len <= tf) {
 		H2_Send_Frame(wrk, h2, ftyp, flags, len, r2->stream, ptr);
+		*acct += len;
 	} else {
 		AN(ptr);
 		p = ptr;
@@ -318,7 +319,7 @@ H2_Send(struct worker *wrk, struct h2_req *r2,
 				tf = mfs;
 			if (ftyp->respect_window && p != ptr) {
 				tf = h2_do_window(wrk, r2, h2,
-						  (len > mfs) ? mfs : len);
+				    (len > mfs) ? mfs : len);
 				if (h2_errcheck(r2, h2))
 					return;
 				assert(VTAILQ_FIRST(&h2->txqueue) == r2);
@@ -330,15 +331,28 @@ H2_Send(struct worker *wrk, struct h2_req *r2,
 				if (ftyp->respect_window)
 					assert(tf == len);
 				tf = len;
-				H2_Send_Frame(wrk, h2, ftyp,
-				    final_flags, tf, r2->stream, p);
+				H2_Send_Frame(wrk, h2, ftyp, final_flags, tf,
+				    r2->stream, p);
 				flags = 0;
 			}
 			p += tf;
 			len -= tf;
+			*acct += tf;
 			ftyp = ftyp->continuation;
 			flags &= ftyp->flags;
 			final_flags &= ftyp->flags;
 		} while (!h2->error && len > 0);
 	}
 }
+
+void
+H2_Send(struct worker *wrk, struct h2_req *r2, h2_frame ftyp, uint8_t flags,
+    uint32_t len, const void *ptr, uint64_t *acct)
+{
+	uint64_t dummy_acct;
+
+	if (acct == NULL)
+		acct = &dummy_acct;
+
+	h2_send(wrk, r2, ftyp, flags, len, ptr, acct);
+}
diff --git a/bin/varnishtest/tests/t02015.vtc b/bin/varnishtest/tests/t02015.vtc
new file mode 100644
index 000000000..3da5c24d4
--- /dev/null
+++ b/bin/varnishtest/tests/t02015.vtc
@@ -0,0 +1,49 @@
+varnishtest "h2 ReqAcct"
+
+server s1 {
+	rxreq
+	txresp -bodylen 12345
+} -start
+
+varnish v1 -cliok "param.set feature +http2"
+varnish v1 -vcl+backend "" -start
+
+client c1 {
+	txpri
+
+	stream 0 {
+		rxsettings
+		expect settings.ack == false
+		txsettings -ack
+		txsettings -winsize 1000
+		rxsettings
+		expect settings.ack == true
+	} -run
+
+	stream 1 {
+		txreq -hdr stream 1
+		rxhdrs
+		rxdata
+		txwinup -size 11345
+		rxdata
+	} -run
+
+	stream 3 {
+		txreq -hdr stream 3
+		rxhdrs
+		rxdata
+		txrst
+	} -run
+} -run
+
+shell {
+	varnishncsa -n ${v1_name} -F '%{VSL:ReqAcct[5]}x' -d \
+		-q 'ReqHeader:stream == 1' |
+	grep 12345
+}
+
+shell {
+	varnishncsa -n ${v1_name} -F '%{VSL:ReqAcct[5]}x' -d \
+		-q 'ReqHeader:stream == 3' |
+	grep 1000
+}


More information about the varnish-commit mailing list