[master] 05085d7 New req_dropped counter similar to sess_dropped

Dridi Boukelmoune dridi.boukelmoune at gmail.com
Mon Sep 4 10:18:06 CEST 2017


commit 05085d757691cfef133839ee58c7ca592b504cae
Author: Dridi Boukelmoune <dridi.boukelmoune at gmail.com>
Date:   Tue Aug 22 18:25:38 2017 +0200

    New req_dropped counter similar to sess_dropped
    
    This one is for h2 streams, so the thread pools needed to learn about
    "stream" tasks. To properly test this counter, thread_pool_timeout is
    now disregarded when the vtc_mode debug flag is set. A minimum value
    of 10s is unrealistic for the test suite, it is now hard-coded to 0.5s
    when ran through varnishtest.

diff --git a/bin/varnishd/cache/cache.h b/bin/varnishd/cache/cache.h
index d8e5dcb..3f517e4 100644
--- a/bin/varnishd/cache/cache.h
+++ b/bin/varnishd/cache/cache.h
@@ -294,10 +294,14 @@ enum task_prio {
 	TASK_QUEUE_BO,
 #define TASK_QUEUE_RESERVE	TASK_QUEUE_BO
 	TASK_QUEUE_REQ,
+	TASK_QUEUE_STR,
 	TASK_QUEUE_VCA,
 	TASK_QUEUE_END
 };
 
+#define TASK_QUEUE_CLIENT(prio) \
+	(prio == TASK_QUEUE_REQ || prio == TASK_QUEUE_STR)
+
 /*--------------------------------------------------------------------*/
 
 struct worker {
diff --git a/bin/varnishd/cache/cache_pool.h b/bin/varnishd/cache/cache_pool.h
index 98e43ca..4814c50 100644
--- a/bin/varnishd/cache/cache_pool.h
+++ b/bin/varnishd/cache/cache_pool.h
@@ -50,7 +50,8 @@ struct pool {
 	unsigned			nthr;
 	unsigned			dry;
 	unsigned			lqueue;
-	uintmax_t			ndropped;
+	uintmax_t			sdropped;
+	uintmax_t			rdropped;
 	uintmax_t			nqueued;
 	struct dstat			*a_stat;
 	struct dstat			*b_stat;
diff --git a/bin/varnishd/cache/cache_wrk.c b/bin/varnishd/cache/cache_wrk.c
index 88fb942..41d2849 100644
--- a/bin/varnishd/cache/cache_wrk.c
+++ b/bin/varnishd/cache/cache_wrk.c
@@ -268,14 +268,17 @@ Pool_Task(struct pool *pp, struct pool_task *task, enum task_prio prio)
 	 * queue limits only apply to client threads - all other
 	 * work is vital and needs do be done at the earliest
 	 */
-	if (prio != TASK_QUEUE_REQ ||
+	if (!TASK_QUEUE_CLIENT(prio) ||
 	    pp->lqueue + pp->nthr < cache_param->wthread_max +
 	    cache_param->wthread_queue_limit) {
 		pp->nqueued++;
 		pp->lqueue++;
 		VTAILQ_INSERT_TAIL(&pp->queues[prio], task, list);
 	} else {
-		pp->ndropped++;
+		if (prio == TASK_QUEUE_REQ)
+			pp->sdropped++;
+		else
+			pp->rdropped++;
 		retval = -1;
 	}
 	Lck_Unlock(&pp->mtx);
@@ -495,8 +498,9 @@ pool_herder(void *priv)
 			Lck_Lock(&pp->mtx);
 			/* XXX: unsafe counters */
 			VSC_C_main->sess_queued += pp->nqueued;
-			VSC_C_main->sess_dropped += pp->ndropped;
-			pp->nqueued = pp->ndropped = 0;
+			VSC_C_main->sess_dropped += pp->sdropped;
+			VSC_C_main->req_dropped += pp->rdropped;
+			pp->nqueued = pp->sdropped = pp->rdropped = 0;
 
 			wrk = NULL;
 			pt = VTAILQ_LAST(&pp->idle_queue, taskhead);
@@ -541,6 +545,8 @@ pool_herder(void *priv)
 		}
 		Lck_Lock(&pp->mtx);
 		if (!pp->dry) {
+			if (DO_DEBUG(DBG_VTC_MODE))
+				delay = 0.5;
 			(void)Lck_CondWait(&pp->herder_cond, &pp->mtx,
 				VTIM_real() + delay);
 		} else {
diff --git a/bin/varnishd/http2/cache_http2_proto.c b/bin/varnishd/http2/cache_http2_proto.c
index 122e61f..c5d9530 100644
--- a/bin/varnishd/http2/cache_http2_proto.c
+++ b/bin/varnishd/http2/cache_http2_proto.c
@@ -530,7 +530,7 @@ h2_end_headers(struct worker *wrk, const struct h2_sess *h2,
 	req->task.func = h2_do_req;
 	req->task.priv = req;
 	r2->scheduled = 1;
-	if (Pool_Task(wrk->pool, &req->task, TASK_QUEUE_REQ) != 0)
+	if (Pool_Task(wrk->pool, &req->task, TASK_QUEUE_STR) != 0)
 		return (H2SE_REFUSED_STREAM); //rfc7540,l,3326,3329
 	return (0);
 }
diff --git a/bin/varnishd/main.vsc b/bin/varnishd/main.vsc
index 726ee9e..b513ac4 100644
--- a/bin/varnishd/main.vsc
+++ b/bin/varnishd/main.vsc
@@ -240,8 +240,14 @@
 .. varnish_vsc:: sess_dropped
 	:oneliner:	Sessions dropped for thread
 
-	Number of times session was dropped because the queue were too long
-	already. See also parameter thread_queue_limit.
+	Number of times an HTTP/1 session was dropped because the queue was
+	too long already. See also parameter thread_queue_limit.
+
+.. varnish_vsc:: req_dropped
+	:oneliner:	Requests dropped
+
+	Number of times an HTTP/2 stream was refused because the queue was
+	too long already. See also parameter thread_queue_limit.
 
 .. varnish_vsc:: n_object
 	:type:	gauge
diff --git a/bin/varnishtest/tests/t02011.vtc b/bin/varnishtest/tests/t02011.vtc
index c906e92..fd698ce 100644
--- a/bin/varnishtest/tests/t02011.vtc
+++ b/bin/varnishtest/tests/t02011.vtc
@@ -26,6 +26,7 @@ varnish v1 -cliok "param.set thread_pool_min 4"
 varnish v1 -cliok "param.set thread_pool_max 4"
 varnish v1 -cliok "param.set thread_pool_reserve 1"
 varnish v1 -cliok "param.set thread_queue_limit 0"
+varnish v1 -cliok "param.set thread_stats_rate 1"
 varnish v1 -cliok "param.set feature +http2"
 varnish v1 -cliok "param.set debug +syncvsl"
 
@@ -66,3 +67,10 @@ client c1 {
 
 	barrier b2 sync
 } -run
+
+# trigger an update of the stats
+varnish v1 -cliok "param.set thread_pool_min 3"
+delay 1
+varnish v1 -expect sess_drop == 0
+varnish v1 -expect sess_dropped == 0
+varnish v1 -expect req_dropped == 1



More information about the varnish-commit mailing list