[master] d5b75354d wrk: Give bgfetch tasks the lowest priority

[Dridi Boukelmoune]

commit d5b75354dcccd077a889bf1a84bfc83b4b575145
Author: Dridi Boukelmoune <dridi.boukelmoune at gmail.com>
Date:   Mon Jul 18 17:33:15 2022 +0200

    wrk: Give bgfetch tasks the lowest priority
    
    The goal is to prevent grace mode from adding load to a saturated
    Varnish server.
    
    A background fetch entering the queue will block the client task that
    triggerred it until it starts its execution, and reaches the point
    where it no longer needs to hold onto its req.
    
    On a saturated system this can result in significant client latency
    despite a grace hit. A stale object can be served until the end of its
    grace period, at which point a regular fetch would be attempted, and
    eligible to queuing if that is still necessary.
    
    This turns the Pool_Task() failure dead branch from VBF_Fetch() into a
    reachable one.

diff --git a/bin/varnishd/cache/cache.h b/bin/varnishd/cache/cache.h
index 805612d3e..424067abb 100644
--- a/bin/varnishd/cache/cache.h
+++ b/bin/varnishd/cache/cache.h
@@ -214,11 +214,12 @@ enum task_prio {
 	TASK_QUEUE_REQ,
 	TASK_QUEUE_STR,
 	TASK_QUEUE_VCA,
+	TASK_QUEUE_BG,
 	TASK_QUEUE__END
 };
 
 #define TASK_QUEUE_HIGHEST_PRIORITY TASK_QUEUE_BO
-#define TASK_QUEUE_RESERVE TASK_QUEUE__END
+#define TASK_QUEUE_RESERVE TASK_QUEUE_BG
 #define TASK_QUEUE_LIMITED(prio) \
 	(prio == TASK_QUEUE_REQ || prio == TASK_QUEUE_STR)
 
diff --git a/bin/varnishd/cache/cache_fetch.c b/bin/varnishd/cache/cache_fetch.c
index 1edb65576..251f1676a 100644
--- a/bin/varnishd/cache/cache_fetch.c
+++ b/bin/varnishd/cache/cache_fetch.c
@@ -1109,6 +1109,7 @@ VBF_Fetch(struct worker *wrk, struct req *req, struct objcore *oc,
 {
 	struct boc *boc;
 	struct busyobj *bo;
+	enum task_prio prio;
 	const char *how;
 
 	CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
@@ -1126,13 +1127,16 @@ VBF_Fetch(struct worker *wrk, struct req *req, struct objcore *oc,
 
 	switch (mode) {
 	case VBF_PASS:
+		prio = TASK_QUEUE_BO;
 		how = "pass";
 		bo->uncacheable = 1;
 		break;
 	case VBF_NORMAL:
+		prio = TASK_QUEUE_BO;
 		how = "fetch";
 		break;
 	case VBF_BACKGROUND:
+		prio = TASK_QUEUE_BG;
 		how = "bgfetch";
 		bo->is_bgfetch = 1;
 		break;
@@ -1172,7 +1176,7 @@ VBF_Fetch(struct worker *wrk, struct req *req, struct objcore *oc,
 	bo->fetch_task->priv = bo;
 	bo->fetch_task->func = vbf_fetch_thread;
 
-	if (Pool_Task(wrk->pool, bo->fetch_task, TASK_QUEUE_BO)) {
+	if (Pool_Task(wrk->pool, bo->fetch_task, prio)) {
 		wrk->stats->bgfetch_no_thread++;
 		(void)vbf_stp_fail(req->wrk, bo);
 		if (bo->stale_oc != NULL)
diff --git a/bin/varnishtest/tests/c00120.vtc b/bin/varnishtest/tests/c00120.vtc
new file mode 100644
index 000000000..b340a48a9
--- /dev/null
+++ b/bin/varnishtest/tests/c00120.vtc
@@ -0,0 +1,32 @@
+varnishtest "bgfetch_no_thread"
+
+server s1 {
+	rxreq
+	txresp
+} -start
+
+varnish v1 -cliok "param.set thread_pools 1"
+varnish v1 -cliok "param.set thread_pool_min 5"
+varnish v1 -cliok "param.set thread_pool_max 5"
+varnish v1 -vcl+backend {
+	sub vcl_backend_response {
+		set beresp.ttl = 1ms;
+		set beresp.grace = 1h;
+	}
+} -start
+
+client c1 {
+	txreq
+	rxresp
+	expect resp.status == 200
+
+	delay 0.1
+
+	# At this point the thread reserve is too low to
+	# allow a low-priority task like a bgfetch.
+	txreq
+	rxresp
+	expect resp.status == 200
+} -start
+
+varnish v1 -expect MAIN.bgfetch_no_thread == 1