[master] c9fc38420 varnishtest: Tweak default h2 window parameters

Mon Oct 19 17:55:06 UTC 2020

commit c9fc384202b418faed3f1768c0caa60ec26555e0
Author: Dridi Boukelmoune <dridi.boukelmoune at gmail.com>
Date:   Mon Oct 19 19:29:23 2020 +0200

    varnishtest: Tweak default h2 window parameters
    
    This change increases the initial size and reduces the low watermark.
    
    RFC7540 says this:
    
    > Flow-controlled frames from the sender and WINDOW_UPDATE frames from
    > the receiver are completely asynchronous with respect to each other.
    > This property allows a receiver to aggressively update the window
    > size kept by the sender to prevent streams from stalling.
    
    The default parameters are very much on the low-latency aggressive
    updates end of the spectrum, which increases asynchronicity at the
    expense of determinism in test cases.
    
    The tweaks made by varnishtest allows basic tests to send a few request
    bodies before being bothered by window update race conditions. Test
    cases that cover h2 flow control or anything else related to window
    updates may reset parameters or pick other specific values. This frees
    us from a bunch of barriers where the purpose of mitigating this race
    was rarely even documented.
    
    This successfully passed the following test locally:
    
        git grep -Fl +http2 -- '*.vtc' |
        xargs bin/varnishtest/varnishtest -i -n100 -j32
    
    We can hope that h2 test cases will be overall more stable from now on.
    
    Refs #3442

diff --git a/bin/varnishtest/tests/r02305.vtc b/bin/varnishtest/tests/r02305.vtc
index 7da2d486e..dfd60398a 100644
--- a/bin/varnishtest/tests/r02305.vtc
+++ b/bin/varnishtest/tests/r02305.vtc
@@ -1,7 +1,6 @@
 varnishtest "#2305: h/2 reembark with a request body"
 
 barrier b1 cond 2
-barrier b2 sock 2
 
 server s1 {
 	rxreq
@@ -16,15 +15,9 @@ varnish v1 -cliok "param.set debug +syncvsl"
 varnish v1 -cliok "param.set debug +waitinglist"
 
 varnish v1 -vcl+backend {
-	import vtc;
 	sub vcl_recv {
 		return (hash);
 	}
-	sub vcl_deliver {
-		if (req.http.sync) {
-			vtc.barrier_sync("${b2_sock}");
-		}
-	}
 } -start
 
 client c1 {
@@ -35,11 +28,7 @@ client c1 {
 	} -start
 	stream 3 {
 		barrier b1 sync
-		txreq -req POST -hdr sync 1 -body "foo"
-		rxwinup
-		# barrier b2 is here to make HEADERS vs WINDOW_UPDATE
-		# less racy
-		barrier b2 sync
+		txreq -req POST -body "foo"
 		rxresp
 		expect resp.status == 200
 	} -run
diff --git a/bin/varnishtest/tests/r02679.vtc b/bin/varnishtest/tests/r02679.vtc
index c0029782a..d1284e2f5 100644
--- a/bin/varnishtest/tests/r02679.vtc
+++ b/bin/varnishtest/tests/r02679.vtc
@@ -16,6 +16,7 @@ varnish v1 -vcl+backend {
 
 varnish v1 -cliok "param.set feature +http2"
 varnish v1 -cliok "param.set h2_rx_window_low_water 65535"
+varnish v1 -cliok "param.reset h2_initial_window_size"
 
 client c1 {
 	stream 1 {
diff --git a/bin/varnishtest/tests/t02000.vtc b/bin/varnishtest/tests/t02000.vtc
index b3cbc1a22..789b5d822 100644
--- a/bin/varnishtest/tests/t02000.vtc
+++ b/bin/varnishtest/tests/t02000.vtc
@@ -33,6 +33,7 @@ client c1 {
 } -run
 
 varnish v1 -cliok "param.set feature +http2"
+varnish v1 -cliok "param.reset h2_initial_window_size"
 
 client c1 {
 	stream 1 {
@@ -67,8 +68,10 @@ varnish v1 -expect MEMPOOL.sess1.live == 0
 
 process p1 -stop
 # shell {cat ${tmpdir}/vlog}
-shell -match {1001 H2TxHdr        c \[000006040000000000\]} \
-	{cat ${tmpdir}/vlog}
+# SETTINGS with default initial window size
+shell -match {1001 H2TxHdr        c \[000006040000000000\]} {
+	cat ${tmpdir}/vlog
+}
 
 # While we're here, test sess.xid over H2 as well
 
diff --git a/bin/varnishtest/tests/t02005.vtc b/bin/varnishtest/tests/t02005.vtc
index 2bcd1da24..03c9a85ae 100644
--- a/bin/varnishtest/tests/t02005.vtc
+++ b/bin/varnishtest/tests/t02005.vtc
@@ -1,7 +1,5 @@
 varnishtest "H2 POST"
 
-barrier b1 cond 2
-
 barrier b2 sock 2
 barrier b3 sock 2
 
@@ -9,7 +7,6 @@ server s1 {
 	rxreq
 	expect req.http.content-length == 7
 	expect req.http.transfer-encoding == <undef>
-	barrier b1 sync
 	txresp -hdr "Content-Type: text/plain" -body response
 
 	rxreq
@@ -30,7 +27,7 @@ varnish v1 -cliok "param.set debug +syncvsl"
 
 logexpect l1 -v v1 -g raw {
 	expect	* 1001 ReqAcct	"80 7 87 106 8 114"
-	expect	* 1000 ReqAcct	"45 8 53 72 22 94"
+	expect	* 1000 ReqAcct	"45 8 53 54 20 74"
 } -start
 
 client c1 {
@@ -38,14 +35,8 @@ client c1 {
 		txping
 		rxping
 	} -run
-	stream 0 {
-		rxwinup
-	} -start
 	stream 1 {
 		txreq -req POST -hdr content-type text/plain -hdr content-length 7 -body request
-
-		rxwinup
-		barrier b1 sync
 		# First, HTTP checks
 		rxresp
 		expect resp.http.content-Type == "text/plain"
@@ -53,21 +44,18 @@ client c1 {
 		# Then, payload checks
 		expect resp.body == response
 	} -run
-	stream 0 -wait
 } -run
 
-client c1 {
+client c2 {
 	stream 0 {
 		barrier b2 sync
 		delay 1
 		barrier b3 sync
-		rxwinup
 	} -start
 	stream 1 {
 		txreq -url "/a" -req POST -nostrend
 		txdata -datalen 100
 		rxresp
-		rxwinup
 		expect resp.status == 503
 	} -run
 	stream 3 {
diff --git a/bin/varnishtest/tests/t02006.vtc b/bin/varnishtest/tests/t02006.vtc
index 30e4ac805..9056682d3 100644
--- a/bin/varnishtest/tests/t02006.vtc
+++ b/bin/varnishtest/tests/t02006.vtc
@@ -1,13 +1,10 @@
 varnishtest "H2 POST w/ 100 Continue"
 
-barrier b1 cond 2
-
 server s1 {
 	rxreq
 	expect req.http.content-length == <undef>
 	expect req.http.transfer-encoding == chunked
 	expect req.proto == HTTP/1.1
-	barrier b1 sync
 	txresp -hdr "Content-Type: text/plain" -body response
 } -start
 
@@ -31,8 +28,6 @@ client c1 {
 		txdata \
 			-data request
 
-		rxwinup
-		barrier b1 sync
 		rxresp
 		expect resp.status == 200
 		expect resp.http.content-Type == "text/plain"
diff --git a/bin/varnishtest/tests/t02007.vtc b/bin/varnishtest/tests/t02007.vtc
index b3fd521d5..f0b1f1abd 100644
--- a/bin/varnishtest/tests/t02007.vtc
+++ b/bin/varnishtest/tests/t02007.vtc
@@ -1,7 +1,5 @@
 varnishtest "H2 Huge response headers"
 
-barrier b1 sock 2
-
 server s1 {
 	rxreq
 	expect req.proto == HTTP/1.1
@@ -25,14 +23,7 @@ varnish v1 -cliok "param.set feature +http2"
 varnish v1 -cliok "param.set debug +syncvsl"
 varnish v1 -cliok "param.set debug +h2_nocheck"
 
-varnish v1 -vcl+backend {
-	import vtc;
-	sub vcl_deliver {
-		if (req.url == "/1") {
-			vtc.barrier_sync("${b1_sock}");
-		}
-	}
-} -start
+varnish v1 -vcl+backend {} -start
 
 client c1 {
 	stream 0 {
@@ -61,8 +52,6 @@ client c1 {
 		txdata \
 			-data request
 
-		rxwinup
-		barrier b1 sync
 		rxresp
 		expect resp.status == 200
 		expect resp.http.content-Type == "text/plain"
diff --git a/bin/varnishtest/tests/t02014.vtc b/bin/varnishtest/tests/t02014.vtc
index 17489d362..e73477311 100644
--- a/bin/varnishtest/tests/t02014.vtc
+++ b/bin/varnishtest/tests/t02014.vtc
@@ -17,6 +17,7 @@ varnish v1 -vcl+backend {
 
 varnish v1 -cliok "param.set debug +syncvsl"
 varnish v1 -cliok "param.set feature +http2"
+varnish v1 -cliok "param.reset h2_initial_window_size"
 
 client c1 {
 	stream 0 {
diff --git a/bin/varnishtest/vtc_varnish.c b/bin/varnishtest/vtc_varnish.c
index 5476806bc..1df5dd93d 100644
--- a/bin/varnishtest/vtc_varnish.c
+++ b/bin/varnishtest/vtc_varnish.c
@@ -427,6 +427,8 @@ varnish_launch(struct varnish *v)
 	VSB_cat(vsb, " -p thread_pool_min=10");
 	VSB_cat(vsb, " -p debug=+vtc_mode");
 	VSB_cat(vsb, " -p vsl_mask=+Debug");
+	VSB_cat(vsb, " -p h2_initial_window_size=1m");
+	VSB_cat(vsb, " -p h2_rx_window_low_water=64k");
 	if (!v->has_a_arg) {
 		VSB_printf(vsb, " -a '%s'", "127.0.0.1:0");
 		if (v->proto != NULL)
