Mailing List Archive

commit c9fc384202b418faed3f1768c0caa60ec26555e0
Author: Dridi Boukelmoune <dridi.boukelmoune@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)
_______________________________________________
varnish-commit mailing list
varnish-commit@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-commit