[master] 17ffd4690 vrb: Reduce storage pressure from req.body

Dridi Boukelmoune dridi.boukelmoune at gmail.com
Thu Jun 8 05:50:09 UTC 2023


commit 17ffd469089744da5bc501621aab0a73d22be8d6
Author: Dridi Boukelmoune <dridi.boukelmoune at gmail.com>
Date:   Mon Jun 5 08:09:21 2023 +0200

    vrb: Reduce storage pressure from req.body
    
    Instead of trying to allocate storage for the entire request body when
    the content length is known, let it go in fetch_chunksize steps. This
    may prevent spurious cache evictions for large request bodies for pass
    transactions when we are merely streaming the body.
    
    On the other hand, when the content length is known and the goal is to
    cache the request body, we may still attempt a single allocation and all
    that entails.

diff --git a/bin/varnishd/cache/cache_req_body.c b/bin/varnishd/cache/cache_req_body.c
index 5ffd08b77..bca871765 100644
--- a/bin/varnishd/cache/cache_req_body.c
+++ b/bin/varnishd/cache/cache_req_body.c
@@ -60,6 +60,7 @@ vrb_pull(struct req *req, ssize_t maxsize, objiterate_f *func, void *priv)
 	enum vfp_status vfps = VFP_ERROR;
 	const struct stevedore *stv;
 	ssize_t req_bodybytes = 0;
+	unsigned hint;
 
 	CHECK_OBJ_NOTNULL(req, REQ_MAGIC);
 
@@ -77,7 +78,8 @@ vrb_pull(struct req *req, ssize_t maxsize, objiterate_f *func, void *priv)
 
 	req->storage = NULL;
 
-	if (STV_NewObject(req->wrk, req->body_oc, stv, 8) == 0) {
+	hint = maxsize > 0 ? maxsize : 1;
+	if (STV_NewObject(req->wrk, req->body_oc, stv, hint) == 0) {
 		req->req_body_status = BS_ERROR;
 		HSH_DerefBoc(req->wrk, req->body_oc);
 		AZ(HSH_DerefObjCore(req->wrk, &req->body_oc, 0));
@@ -111,7 +113,8 @@ vrb_pull(struct req *req, ssize_t maxsize, objiterate_f *func, void *priv)
 			(void)VFP_Error(vfc, "Request body too big to cache");
 			break;
 		}
-		l = yet;
+		/* NB: only attempt a full allocation when caching. */
+		l = maxsize > 0 ? yet : 0;
 		if (VFP_GetStorage(vfc, &l, &ptr) != VFP_OK)
 			break;
 		AZ(vfc->failed);
@@ -122,6 +125,8 @@ vrb_pull(struct req *req, ssize_t maxsize, objiterate_f *func, void *priv)
 			req_bodybytes += l;
 			if (yet >= l)
 				yet -= l;
+			else if (yet > 0)
+				yet = 0;
 			if (func != NULL) {
 				r = func(priv, 1, ptr, l);
 				if (r)
diff --git a/bin/varnishtest/tests/c00123.vtc b/bin/varnishtest/tests/c00123.vtc
new file mode 100644
index 000000000..c72c55267
--- /dev/null
+++ b/bin/varnishtest/tests/c00123.vtc
@@ -0,0 +1,75 @@
+varnishtest "Low req.body streaming pressure on storage"
+
+server s0 {
+	rxreq
+	txresp -status 200
+	expect req.bodylen == 100000
+} -dispatch
+
+varnish v1 -vcl+backend {
+	import std;
+
+	sub vcl_recv {
+		set req.storage = storage.s0;
+		if (req.http.cache) {
+			std.cache_req_body(100000b);
+		}
+	}
+} -start
+
+# explicit setting to be robust against changes to the default value
+varnish v1 -cliok "param.set fetch_chunksize 16k"
+
+# chunked req.body streaming uses approximately one fetch_chunksize'd chunk
+client c1 {
+	txreq -req PUT -hdr "Transfer-encoding: chunked"
+	chunkedlen 100000
+	chunkedlen 0
+	rxresp
+	expect resp.status == 200
+} -run
+
+# in practice a little over fetch_chunksize is allocated
+varnish v1 -expect SM?.s0.c_bytes < 20000
+
+# reset s0 counters
+varnish v1 -cliok stop
+varnish v1 -cliok start
+varnish v1 -expect SM?.s0.c_bytes == 0
+
+# content-length req.body streaming also needs one chunk
+client c2 {
+	txreq -req PUT -bodylen 100000
+	rxresp
+	expect resp.status == 200
+} -run
+
+varnish v1 -expect SM?.s0.c_bytes < 20000
+
+# reset s0 counters
+varnish v1 -cliok stop
+varnish v1 -cliok start
+
+# chunked req.body caching allocates storage for the entire body
+client c3 {
+	txreq -req PUT -hdr "cache: body" -hdr "Transfer-encoding: chunked"
+	chunkedlen 100000
+	chunkedlen 0
+	rxresp
+	expect resp.status == 200
+} -run
+
+varnish v1 -expect SM?.s0.c_bytes > 100000
+
+# reset s0 counters
+varnish v1 -cliok stop
+varnish v1 -cliok start
+
+# content-length req.body caching allocates storage for the entire body
+client c4 {
+	txreq -req PUT -hdr "cache: body" -bodylen 100000
+	rxresp
+	expect resp.status == 200
+} -run
+
+varnish v1 -expect SM?.s0.c_bytes > 100000


More information about the varnish-commit mailing list